2022-04-06 05:14:38

by Patrick Rudolph

[permalink] [raw]
Subject: [v7 1/3] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
chips. The functionality will be provided by the exisintg pca954x driver.

While on it make the interrupts support conditionally as not all of the
existing chips have interrupts.

For chips that are powered off by default add an optional regulator
called vdd-supply.

Signed-off-by: Patrick Rudolph <[email protected]>
---
.../bindings/i2c/i2c-mux-pca954x.yaml | 44 ++++++++++++++-----
1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index 9f1726d0356b..132c3e54e7ab 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -4,21 +4,48 @@
$id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: NXP PCA954x I2C bus switch
+title: NXP PCA954x I2C and compatible bus switches

maintainers:
- Laurent Pinchart <[email protected]>

description:
- The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
+ The binding supports NXP PCA954x and PCA984x I2C mux/switch devices,
+ and the Maxim MAX735x and MAX736x I2C mux/switch devices.

allOf:
- $ref: /schemas/i2c/i2c-mux.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - maxim,max7367
+ - maxim,max7369
+ - nxp,pca9542
+ - nxp,pca9543
+ - nxp,pca9544
+ - nxp,pca9545
+ then:
+ properties:
+ interrupts:
+ maxItems: 1
+
+ "#interrupt-cells":
+ const: 2
+
+ interrupt-controller: true

properties:
compatible:
oneOf:
- enum:
+ - maxim,max7356
+ - maxim,max7357
+ - maxim,max7358
+ - maxim,max7367
+ - maxim,max7368
+ - maxim,max7369
- nxp,pca9540
- nxp,pca9542
- nxp,pca9543
@@ -38,14 +65,6 @@ properties:
reg:
maxItems: 1

- interrupts:
- maxItems: 1
-
- "#interrupt-cells":
- const: 2
-
- interrupt-controller: true
-
reset-gpios:
maxItems: 1

@@ -59,6 +78,9 @@ properties:
description: if present, overrides i2c-mux-idle-disconnect
$ref: /schemas/mux/mux-controller.yaml#/properties/idle-state

+ vdd-supply:
+ description: A voltage regulator supplying power to the chip.
+
required:
- compatible
- reg
@@ -79,6 +101,8 @@ examples:
#size-cells = <0>;
reg = <0x74>;

+ vdd-supply = <&p3v3>;
+
interrupt-parent = <&ipic>;
interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
--
2.35.1


2022-04-06 11:17:24

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [v7 1/3] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

Hi Patrick,

Thank you for the patch.

On Tue, Apr 05, 2022 at 02:05:49PM +0200, Patrick Rudolph wrote:
> Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
> chips. The functionality will be provided by the exisintg pca954x driver.
>
> While on it make the interrupts support conditionally as not all of the
> existing chips have interrupts.
>
> For chips that are powered off by default add an optional regulator
> called vdd-supply.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
> ---
> .../bindings/i2c/i2c-mux-pca954x.yaml | 44 ++++++++++++++-----
> 1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index 9f1726d0356b..132c3e54e7ab 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -4,21 +4,48 @@
> $id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: NXP PCA954x I2C bus switch
> +title: NXP PCA954x I2C and compatible bus switches
>
> maintainers:
> - Laurent Pinchart <[email protected]>
>
> description:
> - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> + The binding supports NXP PCA954x and PCA984x I2C mux/switch devices,
> + and the Maxim MAX735x and MAX736x I2C mux/switch devices.
>
> allOf:
> - $ref: /schemas/i2c/i2c-mux.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - maxim,max7367
> + - maxim,max7369
> + - nxp,pca9542
> + - nxp,pca9543
> + - nxp,pca9544
> + - nxp,pca9545
> + then:
> + properties:
> + interrupts:
> + maxItems: 1
> +
> + "#interrupt-cells":
> + const: 2
> +
> + interrupt-controller: true

It feels a bit out of place to have those properties listed before the
main "properties" property, but we can only have a sincel allOf. I
wonder if the i2c-mux schema could be selected automatically based on
node name, but that's out of scope for this patch.

I thought it was more customary to define properties in the main
"properties" property, and then have

if:
not:
properties:
compatible:
contains:
enum:
- maxim,max7367
- maxim,max7369
- nxp,pca9542
- nxp,pca9543
- nxp,pca9544
- nxp,pca9545
then:
properties:
interrupts: false
"#interrupt-cells": false
interrupt-controller: false

I don't mind much either way though, but if one option is preferred over
the other, we may want to be consistent.

Reviewed-by: Laurent Pinchart <[email protected]>

> properties:
> compatible:
> oneOf:
> - enum:
> + - maxim,max7356
> + - maxim,max7357
> + - maxim,max7358
> + - maxim,max7367
> + - maxim,max7368
> + - maxim,max7369
> - nxp,pca9540
> - nxp,pca9542
> - nxp,pca9543
> @@ -38,14 +65,6 @@ properties:
> reg:
> maxItems: 1
>
> - interrupts:
> - maxItems: 1
> -
> - "#interrupt-cells":
> - const: 2
> -
> - interrupt-controller: true
> -
> reset-gpios:
> maxItems: 1
>
> @@ -59,6 +78,9 @@ properties:
> description: if present, overrides i2c-mux-idle-disconnect
> $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
>
> + vdd-supply:
> + description: A voltage regulator supplying power to the chip.
> +
> required:
> - compatible
> - reg
> @@ -79,6 +101,8 @@ examples:
> #size-cells = <0>;
> reg = <0x74>;
>
> + vdd-supply = <&p3v3>;
> +
> interrupt-parent = <&ipic>;
> interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;

--
Regards,

Laurent Pinchart

2022-04-06 21:09:57

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [v7 1/3] dt-bindings: i2c: Add Maxim MAX735x/MAX736x variants

On Tue, Apr 05, 2022 at 04:46:34PM +0300, Laurent Pinchart wrote:
> Hi Patrick,
>
> Thank you for the patch.
>
> On Tue, Apr 05, 2022 at 02:05:49PM +0200, Patrick Rudolph wrote:
> > Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x
> > chips. The functionality will be provided by the exisintg pca954x driver.
> >
> > While on it make the interrupts support conditionally as not all of the
> > existing chips have interrupts.
> >
> > For chips that are powered off by default add an optional regulator
> > called vdd-supply.
> >
> > Signed-off-by: Patrick Rudolph <[email protected]>
> > ---
> > .../bindings/i2c/i2c-mux-pca954x.yaml | 44 ++++++++++++++-----
> > 1 file changed, 34 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > index 9f1726d0356b..132c3e54e7ab 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > @@ -4,21 +4,48 @@
> > $id: http://devicetree.org/schemas/i2c/i2c-mux-pca954x.yaml#
> > $schema: http://devicetree.org/meta-schemas/core.yaml#
> >
> > -title: NXP PCA954x I2C bus switch
> > +title: NXP PCA954x I2C and compatible bus switches
> >
> > maintainers:
> > - Laurent Pinchart <[email protected]>
> >
> > description:
> > - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> > + The binding supports NXP PCA954x and PCA984x I2C mux/switch devices,
> > + and the Maxim MAX735x and MAX736x I2C mux/switch devices.
> >
> > allOf:
> > - $ref: /schemas/i2c/i2c-mux.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - maxim,max7367
> > + - maxim,max7369
> > + - nxp,pca9542
> > + - nxp,pca9543
> > + - nxp,pca9544
> > + - nxp,pca9545
> > + then:
> > + properties:
> > + interrupts:
> > + maxItems: 1
> > +
> > + "#interrupt-cells":
> > + const: 2
> > +
> > + interrupt-controller: true
>
> It feels a bit out of place to have those properties listed before the
> main "properties" property, but we can only have a sincel allOf. I
> wonder if the i2c-mux schema could be selected automatically based on
> node name, but that's out of scope for this patch.

Yes, just move the allOf below 'properties'

> I thought it was more customary to define properties in the main
> "properties" property, and then have

Yes, please do.

Rob