[PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml

Joy Zou posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml
Posted by Joy Zou 2 months, 2 weeks ago
Add device binding doc for PF0900 PMIC driver.

Signed-off-by: Joy Zou <joy.zou@nxp.com>
---
Changes for v2:
1. modify the binding file name to match compatible string.
2. add one space for dt_binding_check warning.
3. remove unnecessary quotes from "VAON".
4. remove the unnecessary empty line.
5. move unevaluatedProperties after the $ref.
6. move additionalProperties after regulator type.
7. remove unnecessary regulator description
---
 .../devicetree/bindings/regulator/nxp,pf0900.yaml  | 169 +++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf0900.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf0900.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..55cfe9ad5c5c24b47d5a806985e092e279755064
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/nxp,pf0900.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/nxp,pf0900.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PF0900 Power Management Integrated Circuit regulators
+
+maintainers:
+  - Joy Zou <joy.zou@nxp.com>
+
+description:
+  The PF0900 is a power management integrated circuit (PMIC) optimized
+  for high performance i.MX9x based applications. It features five high
+  efficiency buck converters, three linear and one vaon regulators. It
+  provides low quiescent current in Standby and low power off Modes.
+
+properties:
+  compatible:
+    enum:
+      - nxp,pf0900
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    type: object
+    additionalProperties: false
+
+    properties:
+      VAON:
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+    patternProperties:
+      "^LDO[1-3]$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+      "^SW[1-5]$":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          nxp,dvs-run-microvolt:
+            minimum: 300000
+            maximum: 1350000
+            description:
+              PMIC default "RUN" state voltage in uV.
+
+          nxp,dvs-standby-microvolt:
+            minimum: 300000
+            maximum: 1350000
+            description:
+              PMIC default "STANDBY" state voltage in uV.
+
+  nxp,i2c-crc-enable:
+    type: boolean
+    description: If the PMIC OTP_I2C_CRC_EN is enable, you need to add this property.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@8 {
+            compatible = "nxp,pf0900";
+            reg = <0x08>;
+            interrupt-parent = <&pcal6524>;
+            interrupts = <89 IRQ_TYPE_LEVEL_LOW>;
+            nxp,i2c-crc-enable;
+
+            regulators {
+                VAON {
+                    regulator-name = "VAON";
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+
+                SW1 {
+                    regulator-name = "SW1";
+                    regulator-min-microvolt = <500000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                    regulator-ramp-delay = <1950>;
+                };
+
+                SW2 {
+                    regulator-name = "SW2";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                    regulator-ramp-delay = <1950>;
+                };
+
+                SW3 {
+                    regulator-name = "SW3";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                    regulator-ramp-delay = <1950>;
+                };
+
+                SW4 {
+                    regulator-name = "SW4";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                    regulator-ramp-delay = <1950>;
+                };
+
+                SW5 {
+                    regulator-name = "SW5";
+                    regulator-min-microvolt = <300000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                    regulator-ramp-delay = <1950>;
+                };
+
+                LDO1 {
+                    regulator-name = "LDO1";
+                    regulator-min-microvolt = <750000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+
+                LDO2 {
+                    regulator-name = "LDO2";
+                    regulator-min-microvolt = <650000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+
+                LDO3 {
+                    regulator-name = "LDO3";
+                    regulator-min-microvolt = <650000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                    regulator-always-on;
+                };
+            };
+        };
+     };

-- 
2.37.1
Re: [PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml
Posted by Krzysztof Kozlowski 2 months, 2 weeks ago
On 21/07/2025 09:11, Joy Zou wrote:

Subject: Reverse prefixes.

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> +
> +  regulators:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      VAON:

Lowercase names.

> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +
> +    patternProperties:
> +      "^LDO[1-3]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +
> +      "^SW[1-5]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          nxp,dvs-run-microvolt:
> +            minimum: 300000
> +            maximum: 1350000
> +            description:
> +              PMIC default "RUN" state voltage in uV.

Why existing properties are not suitable?

> +
> +          nxp,dvs-standby-microvolt:

Why existing standby state bindings are not suitable?

> +            minimum: 300000
> +            maximum: 1350000
> +            description:
> +              PMIC default "STANDBY" state voltage in uV.
> +
> +  nxp,i2c-crc-enable:
> +    type: boolean
> +    description: If the PMIC OTP_I2C_CRC_EN is enable, you need to add this property.


1. Why you cannot just read registers to check for this?
2. You need anyway proper description what is this about and then wrap
according to Linux coding style.



> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - regulators
> +
> +additionalProperties: false
> +



Best regards,
Krzysztof
Re: [PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml
Posted by Joy Zou 2 months, 2 weeks ago
On Mon, Jul 21, 2025 at 09:28:59AM +0200, Krzysztof Kozlowski wrote:
> On 21/07/2025 09:11, Joy Zou wrote:
> 
> Subject: Reverse prefixes.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> > +
> > +  regulators:
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      VAON:
> 
> Lowercase names.
> 
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        unevaluatedProperties: false
> > +
> > +    patternProperties:
> > +      "^LDO[1-3]$":
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        unevaluatedProperties: false
> > +
> > +      "^SW[1-5]$":
> > +        type: object
> > +        $ref: regulator.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          nxp,dvs-run-microvolt:
> > +            minimum: 300000
> > +            maximum: 1350000
> > +            description:
> > +              PMIC default "RUN" state voltage in uV.
> 
> Why existing properties are not suitable?
Have not found the property that can set run state voltage in regulator.yaml.
Can we add a property for run state such as regulator-suspend-microvolt?
> 
> > +
> > +          nxp,dvs-standby-microvolt:
> 
> Why existing standby state bindings are not suitable?
Have found regulator-suspend-microvolt property that can set standby state voltage.
But the regulator-suspend-microvolt property is now deprecated. Can we use it?
> 
> > +            minimum: 300000
> > +            maximum: 1350000
> > +            description:
> > +              PMIC default "STANDBY" state voltage in uV.
> > +
> > +  nxp,i2c-crc-enable:
> > +    type: boolean
> > +    description: If the PMIC OTP_I2C_CRC_EN is enable, you need to add this property.
> 
> 
> 1. Why you cannot just read registers to check for this?
> 2. You need anyway proper description what is this about and then wrap
> according to Linux coding style.
Controlled by customer unviewable fuse bits OTP_I2C_CRC_EN. Check chip part number.
So can not get the I2C_CRC_EN config by reading register.
BR
Joy Zou
> 
> 
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - regulators
> > +
> > +additionalProperties: false
> > +
> 
> 
> 
> Best regards,
> Krzysztof
Re: [PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml
Posted by Krzysztof Kozlowski 2 months, 1 week ago
On 24/07/2025 12:51, Joy Zou wrote:
>>> +        type: object
>>> +        $ref: regulator.yaml#
>>> +        unevaluatedProperties: false
>>> +
>>> +    patternProperties:
>>> +      "^LDO[1-3]$":
>>> +        type: object
>>> +        $ref: regulator.yaml#
>>> +        unevaluatedProperties: false
>>> +
>>> +      "^SW[1-5]$":
>>> +        type: object
>>> +        $ref: regulator.yaml#
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          nxp,dvs-run-microvolt:
>>> +            minimum: 300000
>>> +            maximum: 1350000
>>> +            description:
>>> +              PMIC default "RUN" state voltage in uV.
>>
>> Why existing properties are not suitable?
> Have not found the property that can set run state voltage in regulator.yaml.
> Can we add a property for run state such as regulator-suspend-microvolt?
>>
>>> +
>>> +          nxp,dvs-standby-microvolt:
>>
>> Why existing standby state bindings are not suitable?
> Have found regulator-suspend-microvolt property that can set standby state voltage.
> But the regulator-suspend-microvolt property is now deprecated. Can we use it?

You have regulator-min/max properties for that. Please check with
bindings and other existing code how this looks like. I feel like you
did not try enough to solve it with existing code.

Best regards,
Krzysztof
Re: [PATCH v2 1/2] dt-bindings: regulator: add PF0900 regulator yaml
Posted by Mark Brown 2 months, 2 weeks ago
On Mon, Jul 21, 2025 at 09:28:59AM +0200, Krzysztof Kozlowski wrote:

> > +  nxp,i2c-crc-enable:
> > +    type: boolean
> > +    description: If the PMIC OTP_I2C_CRC_EN is enable, you need to add this property.

> 1. Why you cannot just read registers to check for this?
> 2. You need anyway proper description what is this about and then wrap
> according to Linux coding style.

Looking at the driver code the CRCs are also done on reads as well.