2023-11-30 17:25:56

by Johan Hovold

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

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


2023-11-30 17:26:12

by Johan Hovold

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

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 <[email protected]>
---
.../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

2023-11-30 17:26:26

by Johan Hovold

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

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 <[email protected]>
---
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

2023-12-01 08:35:49

by Krzysztof Kozlowski

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

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

2023-12-01 08:36:56

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---

Acked-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-12-01 09:36:43

by Johan Hovold

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

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