* [PATCH 1/4] dt-bindings: i2c Update PCA954x
@ 2021-12-14 9:50 Patrick Rudolph
2021-12-14 9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Patrick Rudolph @ 2021-12-14 9:50 UTC (permalink / raw)
To: Peter Rosin, Rob Herring, Laurent Pinchart
Cc: Patrick Rudolph, linux-i2c, devicetree, linux-kernel
Add the Maxim MAX735x as supported chip to PCA954x and add an
example how to use it.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
.../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index 9f1726d0356b..bd794cb80c11 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -11,6 +11,7 @@ maintainers:
description:
The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
+ Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
allOf:
- $ref: /schemas/i2c/i2c-mux.yaml#
@@ -19,6 +20,9 @@ properties:
compatible:
oneOf:
- enum:
+ - maxim,max7356
+ - maxim,max7357
+ - maxim,max7358
- nxp,pca9540
- nxp,pca9542
- nxp,pca9543
@@ -40,6 +44,7 @@ properties:
interrupts:
maxItems: 1
+ description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
"#interrupt-cells":
const: 2
@@ -100,6 +105,41 @@ examples:
#size-cells = <0>;
reg = <4>;
+ rtc@51 {
+ compatible = "nxp,pcf8563";
+ reg = <0x51>;
+ };
+ };
+ };
+ };
+
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2c-mux@74 {
+ compatible = "maxim,max7357";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x74>;
+
+ i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+
+ eeprom@54 {
+ compatible = "atmel,24c08";
+ reg = <0x54>;
+ };
+ };
+
+ i2c@7 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <7>;
+
rtc@51 {
compatible = "nxp,pcf8563";
reg = <0x51>;
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x
2021-12-14 9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
@ 2021-12-14 9:50 ` Patrick Rudolph
2021-12-14 11:37 ` Laurent Pinchart
2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
2021-12-15 20:33 ` Rob Herring
2 siblings, 1 reply; 8+ messages in thread
From: Patrick Rudolph @ 2021-12-14 9:50 UTC (permalink / raw)
To: Peter Rosin, Rob Herring, Laurent Pinchart
Cc: Patrick Rudolph, linux-i2c, devicetree, linux-kernel
Add a regulator called vcc and update the example.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
index bd794cb80c11..5add7db02c0c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
@@ -64,6 +64,9 @@ properties:
description: if present, overrides i2c-mux-idle-disconnect
$ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
+ vcc-supply:
+ description: An optional voltage regulator supplying power to the chip.
+
required:
- compatible
- reg
@@ -84,6 +87,8 @@ examples:
#size-cells = <0>;
reg = <0x74>;
+ vcc-supply = <&p3v3>;
+
interrupt-parent = <&ipic>;
interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
interrupt-controller;
--
2.33.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
2021-12-14 9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
2021-12-14 9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
@ 2021-12-14 11:13 ` Laurent Pinchart
2021-12-15 12:42 ` Peter Rosin
2021-12-15 20:33 ` Rob Herring
2 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2021-12-14 11:13 UTC (permalink / raw)
To: Patrick Rudolph
Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel
Hi Patrick,
Thank you for the patch.
On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> Add the Maxim MAX735x as supported chip to PCA954x and add an
> example how to use it.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> .../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index 9f1726d0356b..bd794cb80c11 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -11,6 +11,7 @@ maintainers:
>
> description:
> The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> + Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>
> allOf:
> - $ref: /schemas/i2c/i2c-mux.yaml#
> @@ -19,6 +20,9 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - maxim,max7356
> + - maxim,max7357
> + - maxim,max7358
> - nxp,pca9540
> - nxp,pca9542
> - nxp,pca9543
> @@ -40,6 +44,7 @@ properties:
>
> interrupts:
> maxItems: 1
> + description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
Could this be modelled by a YAML schema instead ? Something like
allOf:
- if:
properties:
compatible:
contains:
enum:
- maxim,max7356
- maxim,max7357
- maxim,max7358
then:
properties:
interrupts: false
(untested, it would be nice to use a pattern check for the compatible
property if possible)
>
> "#interrupt-cells":
> const: 2
> @@ -100,6 +105,41 @@ examples:
> #size-cells = <0>;
> reg = <4>;
>
> + rtc@51 {
> + compatible = "nxp,pcf8563";
> + reg = <0x51>;
> + };
> + };
> + };
> + };
> +
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + i2c-mux@74 {
> + compatible = "maxim,max7357";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x74>;
> +
> + i2c@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + eeprom@54 {
> + compatible = "atmel,24c08";
> + reg = <0x54>;
> + };
> + };
> +
> + i2c@7 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <7>;
> +
> rtc@51 {
> compatible = "nxp,pcf8563";
> reg = <0x51>;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x
2021-12-14 9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
@ 2021-12-14 11:37 ` Laurent Pinchart
0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2021-12-14 11:37 UTC (permalink / raw)
To: Patrick Rudolph
Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel
Hi Patrick,
Thank you for the patch.
On Tue, Dec 14, 2021 at 10:50:20AM +0100, Patrick Rudolph wrote:
> Add a regulator called vcc and update the example.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index bd794cb80c11..5add7db02c0c 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -64,6 +64,9 @@ properties:
> description: if present, overrides i2c-mux-idle-disconnect
> $ref: /schemas/mux/mux-controller.yaml#/properties/idle-state
>
> + vcc-supply:
> + description: An optional voltage regulator supplying power to the chip.
The NXP datasheet names the supply VDD, could we use vdd-supply here ? I
also wouldn't call it ooptional (even if it effectively is from a DT
point of view as the property isn't listed as required), given that the
power supply isn't optional for the chip to function. How about the
following ?
vdd-supply:
description: The voltage regulator powering to the VDD supply.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> required:
> - compatible
> - reg
> @@ -84,6 +87,8 @@ examples:
> #size-cells = <0>;
> reg = <0x74>;
>
> + vcc-supply = <&p3v3>;
> +
> interrupt-parent = <&ipic>;
> interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> interrupt-controller;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
@ 2021-12-15 12:42 ` Peter Rosin
2021-12-15 14:19 ` Patrick Rudolph
0 siblings, 1 reply; 8+ messages in thread
From: Peter Rosin @ 2021-12-15 12:42 UTC (permalink / raw)
To: Laurent Pinchart, Patrick Rudolph
Cc: Rob Herring, linux-i2c, devicetree, linux-kernel
Hi!
On 2021-12-14 12:13, Laurent Pinchart wrote:
> Hi Patrick,
>
> Thank you for the patch.
>
> On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
>> Add the Maxim MAX735x as supported chip to PCA954x and add an
>> example how to use it.
>>
>> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
>> ---
>> .../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> index 9f1726d0356b..bd794cb80c11 100644
>> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
>> @@ -11,6 +11,7 @@ maintainers:
>>
>> description:
>> The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
>> + Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>>
>> allOf:
>> - $ref: /schemas/i2c/i2c-mux.yaml#
>> @@ -19,6 +20,9 @@ properties:
>> compatible:
>> oneOf:
>> - enum:
>> + - maxim,max7356
>> + - maxim,max7357
>> + - maxim,max7358
>> - nxp,pca9540
>> - nxp,pca9542
>> - nxp,pca9543
>> @@ -40,6 +44,7 @@ properties:
>>
>> interrupts:
>> maxItems: 1
>> + description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
>
> Could this be modelled by a YAML schema instead ? Something like
>
> allOf:
> - if:
> properties:
> compatible:
> contains:
> enum:
> - maxim,max7356
> - maxim,max7357
> - maxim,max7358
> then:
> properties:
> interrupts: false
>
> (untested, it would be nice to use a pattern check for the compatible
> property if possible)
Some of the existing NXP chips do not support interrupts; we should
probably treat these new chips the same as the older ones. Either by
disallowing interrupts on both kinds or by continuing to ignore the
situation.
That said, I'm slightly in favor of the latter, since these new chips
do have interrupts, just not the same flavor as the NXP chips. What
the Maxim chips do not have is support for being an
interrupt-controller;
At least that's how I read it...
I don't know how this situation is supposed to be described? Maybe this
new kind of interrupt should be indicated with a bus-error-interrupts
property (or bikeshed along those lines)? Maybe there should be two
entries in the existing interrupts property? Maybe these new chips
should be described in a new binding specific to maxim,max7356-7358
(could still be handled by the pca954x driver of course) to keep the
yaml simpler to read?
However, there is also maxim,max7367-7369 to consider. They seem to
have interrupts of the style described by the NXP binding (haven't
checked if the registers work the same, but since they reuse the
0x70 address-range the are in all likelihood also compatible).
Cheers,
Peter
>>
>> "#interrupt-cells":
>> const: 2
>> @@ -100,6 +105,41 @@ examples:
>> #size-cells = <0>;
>> reg = <4>;
>>
>> + rtc@51 {
>> + compatible = "nxp,pcf8563";
>> + reg = <0x51>;
>> + };
>> + };
>> + };
>> + };
>> +
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + i2c-mux@74 {
>> + compatible = "maxim,max7357";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0x74>;
>> +
>> + i2c@1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <1>;
>> +
>> + eeprom@54 {
>> + compatible = "atmel,24c08";
>> + reg = <0x54>;
>> + };
>> + };
>> +
>> + i2c@7 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <7>;
>> +
>> rtc@51 {
>> compatible = "nxp,pcf8563";
>> reg = <0x51>;
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
2021-12-15 12:42 ` Peter Rosin
@ 2021-12-15 14:19 ` Patrick Rudolph
2021-12-15 21:22 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Patrick Rudolph @ 2021-12-15 14:19 UTC (permalink / raw)
To: Peter Rosin
Cc: Laurent Pinchart, Rob Herring, linux-i2c, devicetree, linux-kernel
Hi Peter,
thanks for your feedback.
You are right, the added Maxim chips should not have set the
interrupt-controller; flag.
The reason I decided to not handle that interrupt is that I don't know
where to pass that bus error to.
It looks like only the I2C master can signal bus errors by returning
-EIO, however there's no API for I2C clients
to pass such errors to the master. However any attempt to access the
stuck and isolated bus will fail and
the address will be NACKed, so I don't think that this a big issue as
in the end the bus stall will be detected.
Is there a mapping between devicetree bindings and driver file names?
If not I'll use
maxim,max7356 as devicetree binding to make it easier to read and
mention that interrupts
are not supported for those maxim devices.
Regards,
Patrick
On Wed, Dec 15, 2021 at 1:42 PM Peter Rosin <peda@axentia.se> wrote:
>
> Hi!
>
> On 2021-12-14 12:13, Laurent Pinchart wrote:
> > Hi Patrick,
> >
> > Thank you for the patch.
> >
> > On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> >> Add the Maxim MAX735x as supported chip to PCA954x and add an
> >> example how to use it.
> >>
> >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> >> ---
> >> .../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
> >> 1 file changed, 40 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> index 9f1726d0356b..bd794cb80c11 100644
> >> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> >> @@ -11,6 +11,7 @@ maintainers:
> >>
> >> description:
> >> The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> >> + Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
> >>
> >> allOf:
> >> - $ref: /schemas/i2c/i2c-mux.yaml#
> >> @@ -19,6 +20,9 @@ properties:
> >> compatible:
> >> oneOf:
> >> - enum:
> >> + - maxim,max7356
> >> + - maxim,max7357
> >> + - maxim,max7358
> >> - nxp,pca9540
> >> - nxp,pca9542
> >> - nxp,pca9543
> >> @@ -40,6 +44,7 @@ properties:
> >>
> >> interrupts:
> >> maxItems: 1
> >> + description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> >
> > Could this be modelled by a YAML schema instead ? Something like
> >
> > allOf:
> > - if:
> > properties:
> > compatible:
> > contains:
> > enum:
> > - maxim,max7356
> > - maxim,max7357
> > - maxim,max7358
> > then:
> > properties:
> > interrupts: false
> >
> > (untested, it would be nice to use a pattern check for the compatible
> > property if possible)
>
> Some of the existing NXP chips do not support interrupts; we should
> probably treat these new chips the same as the older ones. Either by
> disallowing interrupts on both kinds or by continuing to ignore the
> situation.
>
> That said, I'm slightly in favor of the latter, since these new chips
> do have interrupts, just not the same flavor as the NXP chips. What
> the Maxim chips do not have is support for being an
> interrupt-controller;
> At least that's how I read it...
>
> I don't know how this situation is supposed to be described? Maybe this
> new kind of interrupt should be indicated with a bus-error-interrupts
> property (or bikeshed along those lines)? Maybe there should be two
> entries in the existing interrupts property? Maybe these new chips
> should be described in a new binding specific to maxim,max7356-7358
> (could still be handled by the pca954x driver of course) to keep the
> yaml simpler to read?
>
> However, there is also maxim,max7367-7369 to consider. They seem to
> have interrupts of the style described by the NXP binding (haven't
> checked if the registers work the same, but since they reuse the
> 0x70 address-range the are in all likelihood also compatible).
>
> Cheers,
> Peter
>
> >>
> >> "#interrupt-cells":
> >> const: 2
> >> @@ -100,6 +105,41 @@ examples:
> >> #size-cells = <0>;
> >> reg = <4>;
> >>
> >> + rtc@51 {
> >> + compatible = "nxp,pcf8563";
> >> + reg = <0x51>;
> >> + };
> >> + };
> >> + };
> >> + };
> >> +
> >> + - |
> >> + i2c {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + i2c-mux@74 {
> >> + compatible = "maxim,max7357";
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <0x74>;
> >> +
> >> + i2c@1 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <1>;
> >> +
> >> + eeprom@54 {
> >> + compatible = "atmel,24c08";
> >> + reg = <0x54>;
> >> + };
> >> + };
> >> +
> >> + i2c@7 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> + reg = <7>;
> >> +
> >> rtc@51 {
> >> compatible = "nxp,pcf8563";
> >> reg = <0x51>;
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
2021-12-14 9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
2021-12-14 9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
@ 2021-12-15 20:33 ` Rob Herring
2 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2021-12-15 20:33 UTC (permalink / raw)
To: Patrick Rudolph
Cc: Peter Rosin, Laurent Pinchart, linux-i2c, devicetree, linux-kernel
On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> Add the Maxim MAX735x as supported chip to PCA954x and add an
> example how to use it.
The subject needs some work. Every change is an 'update' and you should
say something about Maxim. 'Add Maxim MAX735x variants' or something.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> .../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> index 9f1726d0356b..bd794cb80c11 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> @@ -11,6 +11,7 @@ maintainers:
>
> description:
> The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> + Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
>
> allOf:
> - $ref: /schemas/i2c/i2c-mux.yaml#
> @@ -19,6 +20,9 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - maxim,max7356
> + - maxim,max7357
> + - maxim,max7358
> - nxp,pca9540
> - nxp,pca9542
> - nxp,pca9543
> @@ -40,6 +44,7 @@ properties:
>
> interrupts:
> maxItems: 1
> + description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
You can express that as an if/then schema.
Just 'interrupts: false' for maxim compatibles. There's lots of
examples in the tree.
>
> "#interrupt-cells":
> const: 2
> @@ -100,6 +105,41 @@ examples:
> #size-cells = <0>;
> reg = <4>;
>
> + rtc@51 {
> + compatible = "nxp,pcf8563";
> + reg = <0x51>;
> + };
Unrelated change.
> + };
> + };
> + };
> +
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
Really need another example?
> +
> + i2c-mux@74 {
> + compatible = "maxim,max7357";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x74>;
> +
> + i2c@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> +
> + eeprom@54 {
> + compatible = "atmel,24c08";
> + reg = <0x54>;
> + };
> + };
> +
> + i2c@7 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <7>;
> +
> rtc@51 {
> compatible = "nxp,pcf8563";
> reg = <0x51>;
> --
> 2.33.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] dt-bindings: i2c Update PCA954x
2021-12-15 14:19 ` Patrick Rudolph
@ 2021-12-15 21:22 ` Laurent Pinchart
0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2021-12-15 21:22 UTC (permalink / raw)
To: Patrick Rudolph
Cc: Peter Rosin, Rob Herring, linux-i2c, devicetree, linux-kernel
Hi Patrick,
On Wed, Dec 15, 2021 at 03:19:37PM +0100, Patrick Rudolph wrote:
> Hi Peter,
> thanks for your feedback.
> You are right, the added Maxim chips should not have set the interrupt-controller; flag.
>
> The reason I decided to not handle that interrupt is that I don't know where to pass that bus error to.
> It looks like only the I2C master can signal bus errors by returning -EIO, however there's no API for I2C clients
> to pass such errors to the master. However any attempt to access the stuck and isolated bus will fail and
> the address will be NACKed, so I don't think that this a big issue as in the end the bus stall will be detected.
You don't have to handle interrupts in the driver in order to declare
the interrupts property in the bindings. If the device has an interrupt
output that is meant to be connected to an interrupt input of the SoC,
then an interrupt property should describe how that signal is connected.
You can upstream support for those devices in the driver without
handlign the interrupt, it can be added later.
> Is there a mapping between devicetree bindings and driver file names? If not I'll use
> maxim,max7356 as devicetree binding to make it easier to read and mention that interrupts
> are not supported for those maxim devices.
The bindings and drivers file names are usually related, as they support
the same device, but there's no specific rule that requires that.
> On Wed, Dec 15, 2021 at 1:42 PM Peter Rosin <peda@axentia.se> wrote:
> > On 2021-12-14 12:13, Laurent Pinchart wrote:
> > > Hi Patrick,
> > >
> > > Thank you for the patch.
> > >
> > > On Tue, Dec 14, 2021 at 10:50:18AM +0100, Patrick Rudolph wrote:
> > >> Add the Maxim MAX735x as supported chip to PCA954x and add an
> > >> example how to use it.
> > >>
> > >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> > >> ---
> > >> .../bindings/i2c/i2c-mux-pca954x.yaml | 40 +++++++++++++++++++
> > >> 1 file changed, 40 insertions(+)
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> index 9f1726d0356b..bd794cb80c11 100644
> > >> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml
> > >> @@ -11,6 +11,7 @@ maintainers:
> > >>
> > >> description:
> > >> The binding supports NXP PCA954x and PCA984x I2C mux/switch devices.
> > >> + Compatible with Maxim MAX7356 - MAX7358 I2C mux/switch devices.
> > >>
> > >> allOf:
> > >> - $ref: /schemas/i2c/i2c-mux.yaml#
> > >> @@ -19,6 +20,9 @@ properties:
> > >> compatible:
> > >> oneOf:
> > >> - enum:
> > >> + - maxim,max7356
> > >> + - maxim,max7357
> > >> + - maxim,max7358
> > >> - nxp,pca9540
> > >> - nxp,pca9542
> > >> - nxp,pca9543
> > >> @@ -40,6 +44,7 @@ properties:
> > >>
> > >> interrupts:
> > >> maxItems: 1
> > >> + description: Only supported on NXP devices. Unsupported on Maxim MAX735x.
> > >
> > > Could this be modelled by a YAML schema instead ? Something like
> > >
> > > allOf:
> > > - if:
> > > properties:
> > > compatible:
> > > contains:
> > > enum:
> > > - maxim,max7356
> > > - maxim,max7357
> > > - maxim,max7358
> > > then:
> > > properties:
> > > interrupts: false
> > >
> > > (untested, it would be nice to use a pattern check for the compatible
> > > property if possible)
> >
> > Some of the existing NXP chips do not support interrupts; we should
> > probably treat these new chips the same as the older ones. Either by
> > disallowing interrupts on both kinds or by continuing to ignore the
> > situation.
> >
> > That said, I'm slightly in favor of the latter, since these new chips
> > do have interrupts, just not the same flavor as the NXP chips. What
> > the Maxim chips do not have is support for being an
> > interrupt-controller;
> > At least that's how I read it...
> >
> > I don't know how this situation is supposed to be described? Maybe this
> > new kind of interrupt should be indicated with a bus-error-interrupts
> > property (or bikeshed along those lines)? Maybe there should be two
> > entries in the existing interrupts property? Maybe these new chips
> > should be described in a new binding specific to maxim,max7356-7358
> > (could still be handled by the pca954x driver of course) to keep the
> > yaml simpler to read?
> >
> > However, there is also maxim,max7367-7369 to consider. They seem to
> > have interrupts of the style described by the NXP binding (haven't
> > checked if the registers work the same, but since they reuse the
> > 0x70 address-range the are in all likelihood also compatible).
> >
> > >> "#interrupt-cells":
> > >> const: 2
> > >> @@ -100,6 +105,41 @@ examples:
> > >> #size-cells = <0>;
> > >> reg = <4>;
> > >>
> > >> + rtc@51 {
> > >> + compatible = "nxp,pcf8563";
> > >> + reg = <0x51>;
> > >> + };
> > >> + };
> > >> + };
> > >> + };
> > >> +
> > >> + - |
> > >> + i2c {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> +
> > >> + i2c-mux@74 {
> > >> + compatible = "maxim,max7357";
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + reg = <0x74>;
> > >> +
> > >> + i2c@1 {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + reg = <1>;
> > >> +
> > >> + eeprom@54 {
> > >> + compatible = "atmel,24c08";
> > >> + reg = <0x54>;
> > >> + };
> > >> + };
> > >> +
> > >> + i2c@7 {
> > >> + #address-cells = <1>;
> > >> + #size-cells = <0>;
> > >> + reg = <7>;
> > >> +
> > >> rtc@51 {
> > >> compatible = "nxp,pcf8563";
> > >> reg = <0x51>;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-15 21:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-14 9:50 [PATCH 1/4] dt-bindings: i2c Update PCA954x Patrick Rudolph
2021-12-14 9:50 ` [PATCH 3/4] dt-bindings: i2c Add regulator to pca954x Patrick Rudolph
2021-12-14 11:37 ` Laurent Pinchart
2021-12-14 11:13 ` [PATCH 1/4] dt-bindings: i2c Update PCA954x Laurent Pinchart
2021-12-15 12:42 ` Peter Rosin
2021-12-15 14:19 ` Patrick Rudolph
2021-12-15 21:22 ` Laurent Pinchart
2021-12-15 20:33 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).