On 15/04/2024 08:51, Matti Vaittinen wrote: > On 4/14/24 00:27, Krzysztof Kozlowski wrote: >> On 12/04/2024 13:21, Matti Vaittinen wrote: >>> ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce >>> DT bindings for the BD96801 regulators. >>> >>> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx> >>> --- >>> Revision history: >>> - No changes since RFCv1 >> >> Subject: missing "regulator" prefix, as first. >> >>> >>> .../regulator/rohm,bd96801-regulator.yaml | 69 +++++++++++++++++++ >>> 1 file changed, 69 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml >>> new file mode 100644 >>> index 000000000000..4015802a3d84 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml >>> @@ -0,0 +1,69 @@ >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/regulator/rohm,bd96801-regulator.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: ROHM BD96801 Power Management Integrated Circuit regulators >>> + >>> +maintainers: >>> + - Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> >>> + >>> +description: | >>> + This module is part of the ROHM BD96801 MFD device. For more details >>> + see Documentation/devicetree/bindings/mfd/rohm,bd96801-pmic.yaml. >>> + >>> + The regulator controller is represented as a sub-node of the PMIC node >>> + on the device tree. >>> + >>> + Regulator nodes should be named to BUCK_<number> and LDO_<number>. >>> + The valid names for BD96801 regulator nodes are >>> + BUCK1, BUCK2, BUCK3, BUCK4, LDO5, LDO6, LDO7 >>> + >>> +patternProperties: >>> + "^LDO[5-7]$": >> >> lowercase >> >>> + type: object >>> + description: >>> + Properties for single LDO regulator. >>> + $ref: regulator.yaml# >> >> Missing unevaluatedProperties: false >> >>> + >>> + properties: >>> + regulator-name: >>> + pattern: "^ldo[5-7]$" >>> + description: >>> + Name of the regulator. Should be "ldo5", ..., "ldo7" >> >> Why do you enforce the name? The name should match board schematics, not >> regulator datasheet. > > If my memory serves me right, the slightly peculiar thing with the > regulator core is it does matching of the regulators based on the names > of the nodes. There was the regulator-compatible property, but I think > it has been deprecated long ago. > > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/of_regulator.c#L380 > > Hence the regulators tend to have fixed names for the nodes. Unless > there has been some recent changes I am not aware of... Yes, names of the nodes, but not "regulator-name" property. > >>> + rohm,initial-voltage-microvolt: >>> + description: >>> + Initial voltage for regulator. Voltage can be tuned +/-150 mV from >>> + this value. NOTE, This can be modified via I2C only when PMIC is in >>> + STBY state. >>> + minimum: 300000 >>> + maximum: 3300000 >> >> Hm, regulator min/max microvolts properties don't work for you? The >> initial will be just middle? > > I had not even thought of this! > > I think this is a good idea. The problem I see is if the system where > the PMIC is used will need to have 'initial power level' at start-up, > which is near the one end of the allowed voltage area. (This because the > "tuning"-range is quite narrow after the initial voltage is set). Wide > allowed voltage range may be needed if the PMIC is reconfigured using > the PMIC STBY state during the runtime. > > Eg, sequence would look like: > > Bootup: > PMIC STBY: > - initial value 'A' from DT > => PMIC ACTIVE > - desired (early) voltages 'A' + 'tune' > > ... > > Voltage state differing more than the 'tune' needed due to some runtime > use-case: > => PMIC STBY > - initial value 'B' > => PMIC ACTIVE > - desired voltages 'B' + 'tune' > > Now, if the 'A' can be 'far' from the mid point of the 'allowed > voltages' -range. > > I have no idea how valid this use-case is though. Once again, I work for > a component vendor and don't get to see the forest from the trees... But > sure I would like to enable as many possible use-cases as, well, possible :) Still I think min/max microvolt solves your case. The property is board-specific and should match what is really on the board. Therefore when writing DTS, one must properly set min/max which in this particular meaning would choose the starting voltage. I am also fine with this property if somehow min/max create confusion or aren't solving the problem. Best regards, Krzysztof
