[PATCH 0/4] dt-bindings: mfd: fix up PMIC examples

[PATCH 0/4] dt-bindings: mfd: fix up PMIC examples

[Johan Hovold]

When reviewing the various SPMI PMIC bindings, I noticed that several
examples were incorrect and misleading and could also use some cleanup.

This series addresses the mfd ones.

[ The PM8008 actually sits on an i2c bus but it is related to the other
  Qualcomm SPMI PMICs. ]

Johan


Johan Hovold (4):
  dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix up binding reference
  dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator
    node
  dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: clean up example
  dt-bindings: mfd: pm8008: fix example node names

 .../mfd/hisilicon,hi6421-spmi-pmic.yaml       | 133 +++++++++---------
 .../devicetree/bindings/mfd/qcom,pm8008.yaml  |   5 +-
 2 files changed, 71 insertions(+), 67 deletions(-)

-- 
2.41.0

[PATCH 1/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix up binding reference

[Johan Hovold]

Fix up the SPMI PMIC binding document free text reference which
erroneously referred to itself rather than the parent SPMI controller
binding as intended.

Fixes: 9e5917288545 ("dt: document HiSilicon SPMI controller and mfd/regulator properties")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml     | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
index bdff5b653453..e36e5ce58136 100644
--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
@@ -17,7 +17,7 @@ description: |
   node.
 
   The SPMI controller part is provided by
-  Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
+  Documentation/devicetree/bindings/spmi/hisilicon,hisi-spmi-controller.yaml
 
 properties:
   $nodename:
-- 
2.41.0

[PATCH 2/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator node

[Johan Hovold]

The example regulator child nodes do not have unit addresses so drop the
incorrect '#address-cells' and '#size-cells' properties from the parent
node.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml    | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
index e36e5ce58136..60d5e6b3de33 100644
--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
@@ -77,9 +77,6 @@ examples:
       interrupts = <0 0>;
 
       regulators {
-        #address-cells = <1>;
-        #size-cells = <0>;
-
         ldo3: ldo3 {
           regulator-name = "ldo3";
           regulator-min-microvolt = <1500000>;
-- 
2.41.0

[PATCH 3/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: clean up example

[Johan Hovold]

The SPMI PMIC sits on an SPMI bus which and has two address cells with
no size.

Clean up the example by adding a parent SPMI bus node with proper
'#address-cells' and '#size-cells' properties, using a define for the
second register value, dropping the unnecessary label and increasing the
indentation to four spaces.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../mfd/hisilicon,hi6421-spmi-pmic.yaml       | 134 +++++++++---------
 1 file changed, 70 insertions(+), 64 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
index 60d5e6b3de33..560ec3367045 100644
--- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
@@ -66,69 +66,75 @@ additionalProperties: false
 
 examples:
   - |
-
-    pmic: pmic@0 {
-      compatible = "hisilicon,hi6421v600-spmi";
-      reg = <0 0>;
-
-      #interrupt-cells = <2>;
-      interrupt-controller;
-      interrupt-parent = <&gpio28>;
-      interrupts = <0 0>;
-
-      regulators {
-        ldo3: ldo3 {
-          regulator-name = "ldo3";
-          regulator-min-microvolt = <1500000>;
-          regulator-max-microvolt = <2000000>;
-          regulator-boot-on;
-        };
-
-        ldo4: ldo4 {
-          regulator-name = "ldo4";
-          regulator-min-microvolt = <1725000>;
-          regulator-max-microvolt = <1900000>;
-          regulator-boot-on;
-        };
-
-        ldo9: ldo9 {
-          regulator-name = "ldo9";
-          regulator-min-microvolt = <1750000>;
-          regulator-max-microvolt = <3300000>;
-          regulator-boot-on;
-        };
-
-        ldo15: ldo15 {
-          regulator-name = "ldo15";
-          regulator-min-microvolt = <1800000>;
-          regulator-max-microvolt = <3000000>;
-          regulator-always-on;
-        };
-
-        ldo16: ldo16 {
-          regulator-name = "ldo16";
-          regulator-min-microvolt = <1800000>;
-          regulator-max-microvolt = <3000000>;
-          regulator-boot-on;
-        };
-
-        ldo17: ldo17 {
-          regulator-name = "ldo17";
-          regulator-min-microvolt = <2500000>;
-          regulator-max-microvolt = <3300000>;
-        };
-
-        ldo33: ldo33 {
-          regulator-name = "ldo33";
-          regulator-min-microvolt = <2500000>;
-          regulator-max-microvolt = <3300000>;
-          regulator-boot-on;
-        };
-
-        ldo34: ldo34 {
-          regulator-name = "ldo34";
-          regulator-min-microvolt = <2600000>;
-          regulator-max-microvolt = <3300000>;
+    #include <dt-bindings/spmi/spmi.h>
+
+    spmi {
+        #address-cells = <2>;
+        #size-cells = <0>;
+
+        pmic@0 {
+            compatible = "hisilicon,hi6421v600-spmi";
+            reg = <0 SPMI_USID>;
+
+            #interrupt-cells = <2>;
+            interrupt-controller;
+            interrupt-parent = <&gpio28>;
+            interrupts = <0 0>;
+
+            regulators {
+                ldo3 {
+                    regulator-name = "ldo3";
+                    regulator-min-microvolt = <1500000>;
+                    regulator-max-microvolt = <2000000>;
+                    regulator-boot-on;
+                };
+
+                ldo4 {
+                    regulator-name = "ldo4";
+                    regulator-min-microvolt = <1725000>;
+                    regulator-max-microvolt = <1900000>;
+                    regulator-boot-on;
+                };
+
+                ldo9 {
+                    regulator-name = "ldo9";
+                    regulator-min-microvolt = <1750000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                };
+
+                ldo15 {
+                    regulator-name = "ldo15";
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <3000000>;
+                    regulator-always-on;
+                };
+
+                ldo16 {
+                    regulator-name = "ldo16";
+                    regulator-min-microvolt = <1800000>;
+                    regulator-max-microvolt = <3000000>;
+                    regulator-boot-on;
+                };
+
+                ldo17 {
+                    regulator-name = "ldo17";
+                    regulator-min-microvolt = <2500000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+
+                ldo33 {
+                    regulator-name = "ldo33";
+                    regulator-min-microvolt = <2500000>;
+                    regulator-max-microvolt = <3300000>;
+                    regulator-boot-on;
+                };
+
+                ldo34 {
+                    regulator-name = "ldo34";
+                    regulator-min-microvolt = <2600000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+            };
         };
-      };
     };
-- 
2.41.0

[PATCH 4/4] dt-bindings: mfd: pm8008: fix example node names

[Johan Hovold]

Devicetree node names should be generic; fix up the pm8008 binding
example accordingly.

Fixes: b0572a9b2397 ("dt-bindings: mfd: pm8008: Add bindings")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
index 24c6158f73ae..32ea898e3ca9 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8008.yaml
@@ -88,10 +88,11 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/irq.h>
     #include <dt-bindings/gpio/gpio.h>
-    qupv3_se13_i2c {
+    i2c {
       #address-cells = <1>;
       #size-cells = <0>;
-      pm8008i@8 {
+
+      pmic@8 {
         compatible = "qcom,pm8008";
         reg = <0x8>;
 
-- 
2.41.0

Re: [PATCH 4/4] dt-bindings: mfd: pm8008: fix example node names

[Krzysztof Kozlowski]

On 30/11/2023 18:25, Johan Hovold wrote:
> Devicetree node names should be generic; fix up the pm8008 binding
> example accordingly.
> 
> Fixes: b0572a9b2397 ("dt-bindings: mfd: pm8008: Add bindings")

No, there is no bug here. The generic node names rule is just a style
issue, not a bug. We have never marked these as bugs even in DTS where
they matter. Here, it is an example, so it would not matter anyway.

Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix up binding reference

[Krzysztof Kozlowski]

On 30/11/2023 18:25, Johan Hovold wrote:
> Fix up the SPMI PMIC binding document free text reference which
> erroneously referred to itself rather than the parent SPMI controller
> binding as intended.
> 
> Fixes: 9e5917288545 ("dt: document HiSilicon SPMI controller and mfd/regulator properties")
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator node

[Krzysztof Kozlowski]

On 30/11/2023 18:25, Johan Hovold wrote:
> The example regulator child nodes do not have unit addresses so drop the
> incorrect '#address-cells' and '#size-cells' properties from the parent
> node.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 3/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: clean up example

[Krzysztof Kozlowski]

On 30/11/2023 18:25, Johan Hovold wrote:
> The SPMI PMIC sits on an SPMI bus which and has two address cells with
> no size.
> 
> Clean up the example by adding a parent SPMI bus node with proper
> '#address-cells' and '#size-cells' properties, using a define for the
> second register value, dropping the unnecessary label and increasing the
> indentation to four spaces.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 4/4] dt-bindings: mfd: pm8008: fix example node names

[Johan Hovold]

On Fri, Dec 01, 2023 at 09:35:30AM +0100, Krzysztof Kozlowski wrote:
> On 30/11/2023 18:25, Johan Hovold wrote:
> > Devicetree node names should be generic; fix up the pm8008 binding
> > example accordingly.
> > 
> > Fixes: b0572a9b2397 ("dt-bindings: mfd: pm8008: Add bindings")
> 
> No, there is no bug here. The generic node names rule is just a style
> issue, not a bug. We have never marked these as bugs even in DTS where
> they matter. Here, it is an example, so it would not matter anyway.

Here I agree with you, this is just a cleanup.

Johan

Re: [PATCH 2/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator node

[Rob Herring]

On Thu, Nov 30, 2023 at 06:25:45PM +0100, Johan Hovold wrote:
> The example regulator child nodes do not have unit addresses so drop the
> incorrect '#address-cells' and '#size-cells' properties from the parent
> node.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml    | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
> index e36e5ce58136..60d5e6b3de33 100644
> --- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
> @@ -77,9 +77,6 @@ examples:
>        interrupts = <0 0>;
>  
>        regulators {
> -        #address-cells = <1>;
> -        #size-cells = <0>;
> -

It's the right fix, but I have to wonder why these are not schema 
errors? Seems like something in the schemas needs fixing.

>          ldo3: ldo3 {
>            regulator-name = "ldo3";
>            regulator-min-microvolt = <1500000>;
> -- 
> 2.41.0
> 

Re: [PATCH 2/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator node

[Krzysztof Kozlowski]

On 01/12/2023 15:40, Rob Herring wrote:
> On Thu, Nov 30, 2023 at 06:25:45PM +0100, Johan Hovold wrote:
>> The example regulator child nodes do not have unit addresses so drop the
>> incorrect '#address-cells' and '#size-cells' properties from the parent
>> node.
>>
>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> ---
>>  .../devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml    | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
>> index e36e5ce58136..60d5e6b3de33 100644
>> --- a/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/hisilicon,hi6421-spmi-pmic.yaml
>> @@ -77,9 +77,6 @@ examples:
>>        interrupts = <0 0>;
>>  
>>        regulators {
>> -        #address-cells = <1>;
>> -        #size-cells = <0>;
>> -
> 
> It's the right fix, but I have to wonder why these are not schema 
> errors? Seems like something in the schemas needs fixing.
> 

Good point. The fix is just not sufficient.  This binding explicitly
allow it, so it should be removed there as well.

I retract my Ack/Review. :)

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: mfd: hisilicon,hi6421-spmi-pmic: fix example regulator node

[Johan Hovold]

On Fri, Dec 01, 2023 at 03:42:38PM +0100, Krzysztof Kozlowski wrote:
> On 01/12/2023 15:40, Rob Herring wrote:
> > On Thu, Nov 30, 2023 at 06:25:45PM +0100, Johan Hovold wrote:
> >> The example regulator child nodes do not have unit addresses so drop the
> >> incorrect '#address-cells' and '#size-cells' properties from the parent
> >> node.
 
> >>        regulators {
> >> -        #address-cells = <1>;
> >> -        #size-cells = <0>;
> >> -
> > 
> > It's the right fix, but I have to wonder why these are not schema 
> > errors? Seems like something in the schemas needs fixing.
> 
> Good point. The fix is just not sufficient.  This binding explicitly
> allow it, so it should be removed there as well.

Ah, I missed that. Now fixed up in v2.

Johan