Re: [RFC PATCH v2 1/6] dt-bindings: ROHM BD96801 PMIC regulators

From: Matti Vaittinen
Date: Mon Apr 15 2024 - 02:52:04 EST


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...

+ 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 :)

Yours,
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~