[PATCH v3 1/7] dt-bindings: mfd: brcm,bcm59056: Convert to YAML

Artur Weber posted 7 patches 1 year ago
There is a newer version of this series
[PATCH v3 1/7] dt-bindings: mfd: brcm,bcm59056: Convert to YAML
Posted by Artur Weber 1 year ago
Convert devicetree bindings for the Broadcom BCM59056 PMU MFD from
TXT to YAML format. This patch does not change any functionality;
the bindings remain the same.

The bindings have been split into two parts: the MFD binding and
a separate binding for the regulator node, to simplify the addition
of other models later (which have different regulators).

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Changes in v3:
- Moved regulator node to separate binding
- Removed quotes around compatibles/vbus property
- Style fixes for example
---
 .../devicetree/bindings/mfd/brcm,bcm59056.txt      | 39 ---------------
 .../devicetree/bindings/mfd/brcm,bcm59056.yaml     | 54 ++++++++++++++++++++
 .../bindings/regulator/brcm,bcm59056.yaml          | 58 ++++++++++++++++++++++
 3 files changed, 112 insertions(+), 39 deletions(-)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
deleted file mode 100644
index be51a15e05f926913b3a473648d977b25f1a2fbc..0000000000000000000000000000000000000000
--- a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.txt
+++ /dev/null
@@ -1,39 +0,0 @@
--------------------------------
-BCM590xx Power Management Units
--------------------------------
-
-Required properties:
-- compatible: "brcm,bcm59056"
-- reg: I2C slave address
-- interrupts: interrupt for the PMU. Generic interrupt client node bindings
-  are described in interrupt-controller/interrupts.txt
-
-------------------
-Voltage Regulators
-------------------
-
-Optional child nodes:
-- regulators: container node for regulators following the generic
-  regulator binding in regulator/regulator.txt
-
-  The valid regulator node names for BCM59056 are:
-  	rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
-	mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
-	csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
-	gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
-	vbus
-
-Example:
-	pmu: bcm59056@8 {
-		compatible = "brcm,bcm59056";
-		reg = <0x08>;
-		interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
-		regulators {
-			rfldo_reg: rfldo {
-				regulator-min-microvolt = <1200000>;
-				regulator-max-microvolt = <3300000>;
-			};
-
-			...
-		};
-	};
diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..3296799eb452fca2a4b03699fcb5aa27005a8e8d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml
@@ -0,0 +1,54 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/brcm,bcm59056.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM590xx Power Management IC
+
+maintainers:
+  - Artur Weber <aweber.kernel@gmail.com>
+
+properties:
+  compatible:
+    const: brcm,bcm59056
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  regulators:
+    type: object
+    description: Container node for regulators.
+    $ref: ../regulator/brcm,bcm59056.yaml
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic: bcm59056@8 {
+            compatible = "brcm,bcm59056";
+            reg = <0x08>;
+            interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
+
+            regulators {
+                rfldo_reg: rfldo {
+                    regulator-min-microvolt = <1200000>;
+                    regulator-max-microvolt = <3300000>;
+                };
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..8939004d5a3f079c05d313bed4a2f07fbc473bac
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/brcm,bcm59056.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM59056 Power Management IC regulators
+
+description: |
+  This is a part of device tree bindings for the BCM590XX family of power
+  management ICs.
+
+  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
+  additional information and example.
+
+maintainers:
+  - Artur Weber <aweber.kernel@gmail.com>
+
+# The valid regulator node names for BCM59056 are:
+#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
+#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
+#   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
+#   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
+#   vbus
+
+patternProperties:
+  "^(cam|sim|mmc)ldo[1-2]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+  "^(rf|sd|sdx|aud|mic|usb|vib)ldo$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+  "^(c|m|v)sr$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+  "^(io|sd)sr[1-2]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+  "^gpldo[1-6]$":
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+properties:
+  vbus:
+    type: object
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+additionalProperties: false

-- 
2.48.1
Re: [PATCH v3 1/7] dt-bindings: mfd: brcm,bcm59056: Convert to YAML
Posted by Krzysztof Kozlowski 1 year ago
On Fri, Jan 31, 2025 at 07:13:49PM +0100, Artur Weber wrote:
> +$id: http://devicetree.org/schemas/mfd/brcm,bcm59056.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM590xx Power Management IC
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: brcm,bcm59056
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +    description: Container node for regulators.
> +    $ref: ../regulator/brcm,bcm59056.yaml

Does this device have any other function? RTC? Charger? If not, maybe
just put everything in one binding in regulator dir.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: bcm59056@8 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> +            compatible = "brcm,bcm59056";
> +            reg = <0x08>;
> +            interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
> +
> +            regulators {
> +                rfldo_reg: rfldo {
> +                    regulator-min-microvolt = <1200000>;
> +                    regulator-max-microvolt = <3300000>;
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..8939004d5a3f079c05d313bed4a2f07fbc473bac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/brcm,bcm59056.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM59056 Power Management IC regulators
> +
> +description: |
> +  This is a part of device tree bindings for the BCM590XX family of power
> +  management ICs.
> +
> +  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
> +  additional information and example.
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +# The valid regulator node names for BCM59056 are:
> +#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
> +#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
> +#   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
> +#   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
> +#   vbus

patternProperties should define this. No need to repeat schema in
freeform text.

Best regards,
Krzysztof
Re: [PATCH v3 1/7] dt-bindings: mfd: brcm,bcm59056: Convert to YAML
Posted by Artur Weber 1 year ago
On 2.02.2025 14:27, Krzysztof Kozlowski wrote:
> On Fri, Jan 31, 2025 at 07:13:49PM +0100, Artur Weber wrote:
>> +$id: http://devicetree.org/schemas/mfd/brcm,bcm59056.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM590xx Power Management IC
>> +
>> +maintainers:
>> +  - Artur Weber <aweber.kernel@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: brcm,bcm59056
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  regulators:
>> +    type: object
>> +    description: Container node for regulators.
>> +    $ref: ../regulator/brcm,bcm59056.yaml
> 
> Does this device have any other function? RTC? Charger? If not, maybe
> just put everything in one binding in regulator dir.
> 

Yes, the BCM590xx PMICs have multiple other functions: power-on key,
RTC, charger/fuel gauge and some USB/audio related controls. I am
planning to extend the driver to support all of these features in the
near future; when the time comes, the DT bindings will have to be
updated as well.

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        pmic: bcm59056@8 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 

I will fix this in the next version.

>> +            compatible = "brcm,bcm59056";
>> +            reg = <0x08>;
>> +            interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +            regulators {
>> +                rfldo_reg: rfldo {
>> +                    regulator-min-microvolt = <1200000>;
>> +                    regulator-max-microvolt = <3300000>;
>> +                };
>> +            };
>> +        };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8939004d5a3f079c05d313bed4a2f07fbc473bac
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/regulator/brcm,bcm59056.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM59056 Power Management IC regulators
>> +
>> +description: |
>> +  This is a part of device tree bindings for the BCM590XX family of power
>> +  management ICs.
>> +
>> +  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
>> +  additional information and example.
>> +
>> +maintainers:
>> +  - Artur Weber <aweber.kernel@gmail.com>
>> +
>> +# The valid regulator node names for BCM59056 are:
>> +#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
>> +#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
>> +#   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
>> +#   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
>> +#   vbus
> 
> patternProperties should define this. No need to repeat schema in
> freeform text.

IMO a full list is a bit easier to read than the regexes (another
suggestion was to move the list to the description [1]), but I don't
mind dropping it. For the most part, it's a carryover from the TXT
binding.

Best regards
Artur

[1] https://lore.kernel.org/lkml/Z59BQB_cBgTDm4ie@standask-GA-A55M-S2HP/
Re: [PATCH v3 1/7] dt-bindings: mfd: brcm,bcm59056: Convert to YAML
Posted by Stanislav Jakubek 1 year ago
On Fri, Jan 31, 2025 at 07:13:49PM +0100, Artur Weber wrote:
> Convert devicetree bindings for the Broadcom BCM59056 PMU MFD from
> TXT to YAML format. This patch does not change any functionality;
> the bindings remain the same.
> 
> The bindings have been split into two parts: the MFD binding and
> a separate binding for the regulator node, to simplify the addition
> of other models later (which have different regulators).
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>

[snip]

> +  regulators:
> +    type: object
> +    description: Container node for regulators.
> +    $ref: ../regulator/brcm,bcm59056.yaml

Use the full path, so /schemas/regulator/brcm,bcm59056.yaml#
The description seems unnecessary, you can drop it.

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        pmic: bcm59056@8 {

Node names should be generic. And drop unused label.
Should just look like this:
	pmic@8 {

> +            compatible = "brcm,bcm59056";
> +            reg = <0x08>;
> +            interrupts = <GIC_SPI 215 IRQ_TYPE_LEVEL_HIGH>;

#include <dt-bindings/interrupt-controller/irq.h> ?
V2 seems to have it, not sure why you dropped this.

> +
> +            regulators {
> +                rfldo_reg: rfldo {

Unused label, drop.

> +                    regulator-min-microvolt = <1200000>;
> +                    regulator-max-microvolt = <3300000>;
> +                };
> +            };
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..8939004d5a3f079c05d313bed4a2f07fbc473bac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/brcm,bcm59056.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/brcm,bcm59056.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM59056 Power Management IC regulators
> +
> +description: |
> +  This is a part of device tree bindings for the BCM590XX family of power
> +  management ICs.

This doesn't really say anything. Also AFAIK these are only part of BCM59056,
not the entire BCM590XX family.
Maybe say here something like:
"The BCM59056 PMIC integrates X regulators, their valid names are
 lorem, ipsum, etc.
> +
> +  See also Documentation/devicetree/bindings/mfd/brcm,bcm59056.yaml for
> +  additional information and example.
> +
> +maintainers:
> +  - Artur Weber <aweber.kernel@gmail.com>
> +
> +# The valid regulator node names for BCM59056 are:
> +#   rfldo, camldo1, camldo2, simldo1, simldo2, sdldo, sdxldo,
> +#   mmcldo1, mmcldo2, audldo, micldo, usbldo, vibldo,
> +#   csr, iosr1, iosr2, msr, sdsr1, sdsr2, vsr,
> +#   gpldo1, gpldo2, gpldo3, gpldo4, gpldo5, gpldo6,
> +#   vbus

This should probably be a part of the description, not just a comment.
Could be argued to drop it since it's also described below in
patternProperties, but this is easier to read, so IMO would be better to keep.

Regards,
Stanislav