diff mbox series

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

Message ID 27a1c489f62e46a80643fe86cca101fb5aadb7f4.1712920132.git.mazziesaccount@gmail.com
State Superseded
Headers show
Series Support ROHM BD96801 scalable PMIC | expand

Commit Message

Matti Vaittinen April 12, 2024, 11:21 a.m. UTC
ROHM BD96801 is a highly configurable automotive grade PMIC. Introduce
DT bindings for the BD96801 regulators.

Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
---
Revision history:
- No changes since RFCv1

 .../regulator/rohm,bd96801-regulator.yaml     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/rohm,bd96801-regulator.yaml

Comments

Krzysztof Kozlowski April 13, 2024, 9:27 p.m. UTC | #1
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@gmail.com>
> ---
> 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@fi.rohmeurope.com>
> +
> +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.

> +      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?

> +
> +  "^BUCK[1-4]$":

lowercase

> +    type: object
> +    description:
> +      Properties for single BUCK regulator.
> +    $ref: regulator.yaml#
> +
> +    properties:
> +      regulator-name:
> +        pattern: "^buck[1-4]$"
> +        description:
> +          should be "buck1", ..., "buck4"
> +      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: 500000
> +        maximum: 3300000

Missing blank line

> +      rohm,keep-on-stby:
> +        description:
> +          Keep the regulator powered when PMIC transitions to STBY state.
> +        type: boolean
> +
> +    required:
> +      - regulator-name
> +  additionalProperties: false

Blank line

> +additionalProperties: false

Best regards,
Krzysztof
Matti Vaittinen April 15, 2024, 6:51 a.m. UTC | #2
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@gmail.com>
>> ---
>> 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@fi.rohmeurope.com>
>> +
>> +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
Krzysztof Kozlowski April 15, 2024, 3:29 p.m. UTC | #3
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@gmail.com>
>>> ---
>>> 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@fi.rohmeurope.com>
>>> +
>>> +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
diff mbox series

Patch

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@fi.rohmeurope.com>
+
+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]$":
+    type: object
+    description:
+      Properties for single LDO regulator.
+    $ref: regulator.yaml#
+
+    properties:
+      regulator-name:
+        pattern: "^ldo[5-7]$"
+        description:
+          Name of the regulator. Should be "ldo5", ..., "ldo7"
+      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
+
+  "^BUCK[1-4]$":
+    type: object
+    description:
+      Properties for single BUCK regulator.
+    $ref: regulator.yaml#
+
+    properties:
+      regulator-name:
+        pattern: "^buck[1-4]$"
+        description:
+          should be "buck1", ..., "buck4"
+      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: 500000
+        maximum: 3300000
+      rohm,keep-on-stby:
+        description:
+          Keep the regulator powered when PMIC transitions to STBY state.
+        type: boolean
+
+    required:
+      - regulator-name
+  additionalProperties: false
+additionalProperties: false