diff mbox series

[v2,3/6] dt-bindings: mfd: add max77759 binding

Message ID 20250226-max77759-mfd-v2-3-a65ebe2bc0a9@linaro.org (mailing list archive)
State Superseded
Headers show
Series Maxim Integrated MAX77759 PMIC MFD-based drivers | expand

Commit Message

André Draszik Feb. 26, 2025, 5:51 p.m. UTC
The Maxim MAX77759 is a companion PMIC for USB Type-C applications and
includes Battery Charger, Fuel Gauge, temperature sensors, USB Type-C
Port Controller (TCPC), NVMEM, and a GPIO expander.

This describes the top-level device.

Signed-off-by: André Draszik <andre.draszik@linaro.org>

---
v2:
* rename expected nvmem subdev nodename to 'nvmem-0'
  I'd have preferred just 'nvmem', but that matches nvmem-consumer.yaml
  and fails validation.

Note: MAINTAINERS doesn't need updating, the binding update for the
first leaf device (gpio) adds a wildcard matching all max77759 bindings
---
 .../devicetree/bindings/mfd/maxim,max77759.yaml    | 104 +++++++++++++++++++++
 1 file changed, 104 insertions(+)

Comments

Rob Herring (Arm) Feb. 27, 2025, 1:04 p.m. UTC | #1
On Wed, Feb 26, 2025 at 05:51:22PM +0000, André Draszik wrote:
> The Maxim MAX77759 is a companion PMIC for USB Type-C applications and
> includes Battery Charger, Fuel Gauge, temperature sensors, USB Type-C
> Port Controller (TCPC), NVMEM, and a GPIO expander.
> 
> This describes the top-level device.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> 
> ---
> v2:
> * rename expected nvmem subdev nodename to 'nvmem-0'
>   I'd have preferred just 'nvmem', but that matches nvmem-consumer.yaml
>   and fails validation.
> 
> Note: MAINTAINERS doesn't need updating, the binding update for the
> first leaf device (gpio) adds a wildcard matching all max77759 bindings
> ---
>  .../devicetree/bindings/mfd/maxim,max77759.yaml    | 104 +++++++++++++++++++++
>  1 file changed, 104 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..87e3737896a289998a18b67932dbccacfb8e3150
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/maxim,max77759.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim Integrated MAX77759 PMIC for USB Type-C applications
> +
> +maintainers:
> +  - André Draszik <andre.draszik@linaro.org>
> +
> +description: |
> +  This is a part of device tree bindings for the MAX77759 companion Power
> +  Management IC for USB Type-C applications.
> +
> +  The MAX77759 includes Battery Charger, Fuel Gauge, temperature sensors, USB
> +  Type-C Port Controller (TCPC), NVMEM, and a GPIO expander.
> +
> +properties:
> +  compatible:
> +    const: maxim,max77759
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  "#gpio-cells":
> +    const: 2

Why do you have GPIO properties here and in the child node? Either would 
be valid, but both probably not. Putting them here is actually 
preferred.

> +
> +  gpio:
> +    $ref: /schemas/gpio/maxim,max77759-gpio.yaml
André Draszik Feb. 27, 2025, 1:13 p.m. UTC | #2
On Thu, 2025-02-27 at 07:04 -0600, Rob Herring wrote:
> On Wed, Feb 26, 2025 at 05:51:22PM +0000, André Draszik wrote:
> > The Maxim MAX77759 is a companion PMIC for USB Type-C applications and
> > includes Battery Charger, Fuel Gauge, temperature sensors, USB Type-C
> > Port Controller (TCPC), NVMEM, and a GPIO expander.
> > 
> > This describes the top-level device.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > 
> > ---
> > v2:
> > * rename expected nvmem subdev nodename to 'nvmem-0'
> >   I'd have preferred just 'nvmem', but that matches nvmem-consumer.yaml
> >   and fails validation.
> > 
> > Note: MAINTAINERS doesn't need updating, the binding update for the
> > first leaf device (gpio) adds a wildcard matching all max77759 bindings
> > ---
> >  .../devicetree/bindings/mfd/maxim,max77759.yaml    | 104 +++++++++++++++++++++
> >  1 file changed, 104 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..87e3737896a289998a18b67932dbccacfb8e3150
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/maxim,max77759.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Maxim Integrated MAX77759 PMIC for USB Type-C applications
> > +
> > +maintainers:
> > +  - André Draszik <andre.draszik@linaro.org>
> > +
> > +description: |
> > +  This is a part of device tree bindings for the MAX77759 companion Power
> > +  Management IC for USB Type-C applications.
> > +
> > +  The MAX77759 includes Battery Charger, Fuel Gauge, temperature sensors, USB
> > +  Type-C Port Controller (TCPC), NVMEM, and a GPIO expander.
> > +
> > +properties:
> > +  compatible:
> > +    const: maxim,max77759
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  interrupt-controller: true
> > +
> > +  "#interrupt-cells":
> > +    const: 2
> > +
> > +  gpio-controller: true
> > +
> > +  "#gpio-cells":
> > +    const: 2
> 
> Why do you have GPIO properties here and in the child node? Either would 
> be valid, but both probably not. Putting them here is actually 
> preferred.

That's an oversight, I meant to put them into the child only, not here,
since the child is the one providing the gpio functionality.

What's the reason to have it preferred inside this parent node?

At least some bindings do specify it in the child node, e.g.:
delta,tn48m-gpio.yaml
kontron,sl28cpld-gpio.yaml
xylon,logicvc-gpio.yaml

Cheers,
Andre'
Rob Herring (Arm) Feb. 28, 2025, 1:01 p.m. UTC | #3
On Thu, Feb 27, 2025 at 7:14 AM André Draszik <andre.draszik@linaro.org> wrote:
>
> On Thu, 2025-02-27 at 07:04 -0600, Rob Herring wrote:
> > On Wed, Feb 26, 2025 at 05:51:22PM +0000, André Draszik wrote:
> > > The Maxim MAX77759 is a companion PMIC for USB Type-C applications and
> > > includes Battery Charger, Fuel Gauge, temperature sensors, USB Type-C
> > > Port Controller (TCPC), NVMEM, and a GPIO expander.
> > >
> > > This describes the top-level device.
> > >
> > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > >
> > > ---
> > > v2:
> > > * rename expected nvmem subdev nodename to 'nvmem-0'
> > >   I'd have preferred just 'nvmem', but that matches nvmem-consumer.yaml
> > >   and fails validation.
> > >
> > > Note: MAINTAINERS doesn't need updating, the binding update for the
> > > first leaf device (gpio) adds a wildcard matching all max77759 bindings
> > > ---
> > >  .../devicetree/bindings/mfd/maxim,max77759.yaml    | 104 +++++++++++++++++++++
> > >  1 file changed, 104 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..87e3737896a289998a18b67932dbccacfb8e3150
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> > > @@ -0,0 +1,104 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/maxim,max77759.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Maxim Integrated MAX77759 PMIC for USB Type-C applications
> > > +
> > > +maintainers:
> > > +  - André Draszik <andre.draszik@linaro.org>
> > > +
> > > +description: |
> > > +  This is a part of device tree bindings for the MAX77759 companion Power
> > > +  Management IC for USB Type-C applications.
> > > +
> > > +  The MAX77759 includes Battery Charger, Fuel Gauge, temperature sensors, USB
> > > +  Type-C Port Controller (TCPC), NVMEM, and a GPIO expander.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: maxim,max77759
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  interrupt-controller: true
> > > +
> > > +  "#interrupt-cells":
> > > +    const: 2
> > > +
> > > +  gpio-controller: true
> > > +
> > > +  "#gpio-cells":
> > > +    const: 2
> >
> > Why do you have GPIO properties here and in the child node? Either would
> > be valid, but both probably not. Putting them here is actually
> > preferred.
>
> That's an oversight, I meant to put them into the child only, not here,
> since the child is the one providing the gpio functionality.
>
> What's the reason to have it preferred inside this parent node?

It really depends whether the GPIO block is a separate sub-block which
is going to get reused or has its own resources or not. It's the same
thing in system controllers which are often just a collection of
leftover control bits.

We just don't want child nodes created just for the ease of
instantiating drivers in Linux. While it's nice if drivers and nodes
are 1 to 1, but that's specific to an OS.

You already need other child nodes here, so I don't care too much in this case.

Rob
André Draszik Feb. 28, 2025, 1:59 p.m. UTC | #4
On Fri, 2025-02-28 at 07:01 -0600, Rob Herring wrote:
> On Thu, Feb 27, 2025 at 7:14 AM André Draszik <andre.draszik@linaro.org> wrote:
> > 
> > On Thu, 2025-02-27 at 07:04 -0600, Rob Herring wrote:
> > >
> > > 
> > > Why do you have GPIO properties here and in the child node? Either would
> > > be valid, but both probably not. Putting them here is actually
> > > preferred.
> > 
> > That's an oversight, I meant to put them into the child only, not here,
> > since the child is the one providing the gpio functionality.
> > 
> > What's the reason to have it preferred inside this parent node?
> 
> It really depends whether the GPIO block is a separate sub-block which
> is going to get reused or has its own resources or not. It's the same
> thing in system controllers which are often just a collection of
> leftover control bits.
> 
> We just don't want child nodes created just for the ease of
> instantiating drivers in Linux. While it's nice if drivers and nodes
> are 1 to 1, but that's specific to an OS.
> 
> You already need other child nodes here, so I don't care too much in this case.

Thanks Rob for taking the time and for the explanation! I'll keep
that in mind for the future.

Cheers,
Andre'
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..87e3737896a289998a18b67932dbccacfb8e3150
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
@@ -0,0 +1,104 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/maxim,max77759.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated MAX77759 PMIC for USB Type-C applications
+
+maintainers:
+  - André Draszik <andre.draszik@linaro.org>
+
+description: |
+  This is a part of device tree bindings for the MAX77759 companion Power
+  Management IC for USB Type-C applications.
+
+  The MAX77759 includes Battery Charger, Fuel Gauge, temperature sensors, USB
+  Type-C Port Controller (TCPC), NVMEM, and a GPIO expander.
+
+properties:
+  compatible:
+    const: maxim,max77759
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
+  gpio:
+    $ref: /schemas/gpio/maxim,max77759-gpio.yaml
+
+  reg:
+    maxItems: 1
+
+  nvmem-0:
+    $ref: /schemas/nvmem/maxim,max77759-nvmem.yaml
+
+required:
+  - compatible
+  - interrupts
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pmic@66 {
+            compatible = "maxim,max77759";
+            reg = <0x66>;
+            interrupts-extended = <&gpa8 3 IRQ_TYPE_LEVEL_LOW>;
+
+            interrupt-controller;
+            #interrupt-cells = <2>;
+
+            gpio {
+                compatible = "maxim,max77759-gpio";
+
+                gpio-controller;
+                #gpio-cells = <2>;
+
+                interrupt-controller;
+                #interrupt-cells = <2>;
+            };
+
+            nvmem-0 {
+                compatible = "maxim,max77759-nvmem";
+
+                nvmem-layout {
+                    compatible = "fixed-layout";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    reboot-mode@0 {
+                        reg = <0x0 0x4>;
+                    };
+
+                    boot-reason@4 {
+                        reg = <0x4 0x4>;
+                    };
+
+                    shutdown-user-flag@8 {
+                        reg = <0x8 0x1>;
+                    };
+
+                    rsoc@10 {
+                        reg = <0xa 0x2>;
+                    };
+                };
+            };
+        };
+    };