diff mbox series

[RFC,1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs

Message ID 20231217131838.7569-2-karelb@gimli.ms.mff.cuni.cz
State New
Headers show
Series support for Marvell 88PM886 PMIC | expand

Commit Message

Karel Balej Dec. 17, 2023, 1:16 p.m. UTC
From: Karel Balej <balejk@matfyz.cz>

Marvell 88PM880 and 88PM886 are two similar PMICs with mostly matching
register mapping and subdevices such as onkey, regulators or battery and
charger. Both seem to come in two revisions which seem to be handled
slightly differently in some subdevice drivers.

Signed-off-by: Karel Balej <balejk@matfyz.cz>
---
 .../bindings/mfd/marvell,88pm88x.yaml         | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml

Comments

Rob Herring (Arm) Dec. 18, 2023, 3:17 p.m. UTC | #1
On Sun, Dec 17, 2023 at 02:16:59PM +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
> 
> Marvell 88PM880 and 88PM886 are two similar PMICs with mostly matching
> register mapping and subdevices such as onkey, regulators or battery and
> charger. Both seem to come in two revisions which seem to be handled
> slightly differently in some subdevice drivers.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>  .../bindings/mfd/marvell,88pm88x.yaml         | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> new file mode 100644
> index 000000000000..e075729c360f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell 88PM88X PMIC core MFD

Drop 'MFD'.

> +
> +maintainers:
> +  - Karel Balej <balejk@matfyz.cz>
> +
> +description: |

Don't need '|' as there is no formatting to preserve.

> +  Marvell 88PM880 and 88PM886 are two similar PMICs providing
> +  several functions such as onkey, regulators or battery and
> +  charger. Both seem to come in two revisions -- A0 and A1.
> +
> +properties:
> +  compatible:
> +    const: marvell,88pm886-a1

The description talks about 4 different devices, but only 1 here. 

Do you expect to need A0 support? Devices with these PMICs should be 
known and few, right? 

> +
> +  reg:
> +    description: I2C device address

Drop.

> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupt-controller
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic0: 88pm886@30 {

pmic@30

Drop the unused label.

> +        compatible = "marvell,88pm886-a1";
> +        reg = <0x30>;
> +        interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;

You need the header for this.

You'll find the input binding fails too. Please test your bindings 
before sending.

> +        interrupt-parent = <&gic>;
> +        interrupt-controller;
> +        #interrupt-cells = <1>;
> +      };
> +    };
> +...
> -- 
> 2.43.0
>
Karel Balej Dec. 22, 2023, 5:25 p.m. UTC | #2
Rob,

thank you very much for your feedback.

On Mon Dec 18, 2023 at 4:17 PM CET, Rob Herring wrote:
> > +  Marvell 88PM880 and 88PM886 are two similar PMICs providing
> > +  several functions such as onkey, regulators or battery and
> > +  charger. Both seem to come in two revisions -- A0 and A1.
> > +
> > +properties:
> > +  compatible:
> > +    const: marvell,88pm886-a1
>
> The description talks about 4 different devices, but only 1 here. 
>
> Do you expect to need A0 support? Devices with these PMICs should be 
> known and few, right? 

I know of three smartphones which have 88PM886 and all of them (at least
the revisions that have been tested) seem to use A1. So no, I don't know
of any device that would need A0, but I wanted have the driver ready in
case somebody needed to add it later. What change do you then suggest?

Thank you and kind regards,
K. B.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
new file mode 100644
index 000000000000..e075729c360f
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell 88PM88X PMIC core MFD
+
+maintainers:
+  - Karel Balej <balejk@matfyz.cz>
+
+description: |
+  Marvell 88PM880 and 88PM886 are two similar PMICs providing
+  several functions such as onkey, regulators or battery and
+  charger. Both seem to come in two revisions -- A0 and A1.
+
+properties:
+  compatible:
+    const: marvell,88pm886-a1
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  interrupt-controller: true
+
+  interrupts:
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 2
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic0: 88pm886@30 {
+        compatible = "marvell,88pm886-a1";
+        reg = <0x30>;
+        interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-parent = <&gic>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+      };
+    };
+...