| Message ID | 20231220082803.345153-1-naresh.solanki@9elements.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,v5,1/2] dt-bindings: i2c: pca954x: Add custom properties for MAX7357 | expand |
On Wed, Dec 20, 2023 at 01:58:01PM +0530, Naresh Solanki wrote: > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > Maxim Max7357 has a configuration register to enable additional > features. These features aren't enabled by default & its up to > board designer to enable the same as it may have unexpected side effects. > > These should be validated for proper functioning & detection of devices > in secondary bus as sometimes it can cause secondary bus being disabled. > > Add booleans for: > - maxim,isolate-stuck-channel > - maxim,send-flush-out-sequence > - maxim,preconnection-wiggle-test-enable > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > Reviewed-by: Rob Herring <robh@kernel.org> Rob, are you really OK with these bindings? They look more like configuration instead of HW description to me. > --- > Changes in V4: > - Drop max7358. > Changes in V3: > - Update commit message > Changes in V2: > - Update properties. > --- > .../bindings/i2c/i2c-mux-pca954x.yaml | 30 +++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > index 2d7bb998b0e9..9aa0585200c9 100644 > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > @@ -71,6 +71,23 @@ properties: > description: A voltage regulator supplying power to the chip. On PCA9846 > the regulator supplies power to VDD2 (core logic) and optionally to VDD1. > > + maxim,isolate-stuck-channel: > + type: boolean > + description: Allows to use non faulty channels while a stuck channel is > + isolated from the upstream bus. If not set all channels are isolated from > + the upstream bus until the fault is cleared. > + > + maxim,send-flush-out-sequence: > + type: boolean > + description: Send a flush-out sequence to stuck auxiliary buses > + automatically after a stuck channel is being detected. > + > + maxim,preconnection-wiggle-test-enable: > + type: boolean > + description: Send a STOP condition to the auxiliary buses when the switch > + register activates a channel to detect a stuck high fault. On fault the > + channel is isolated from the upstream bus. > + > required: > - compatible > - reg > @@ -95,6 +112,19 @@ allOf: > "#interrupt-cells": false > interrupt-controller: false > > + - if: > + not: > + properties: > + compatible: > + contains: > + enum: > + - maxim,max7357 > + then: > + properties: > + maxim,isolate-stuck-channel: false > + maxim,send-flush-out-sequence: false > + maxim,preconnection-wiggle-test-enable: false > + > unevaluatedProperties: false > > examples: > > base-commit: 76998e5bcdf155b36c7066808a0a65b2ee13cb2a > -- > 2.41.0 > >
On 20/12/2023 21:50, Wolfram Sang wrote: > On Wed, Dec 20, 2023 at 01:58:01PM +0530, Naresh Solanki wrote: >> From: Patrick Rudolph <patrick.rudolph@9elements.com> >> >> Maxim Max7357 has a configuration register to enable additional >> features. These features aren't enabled by default & its up to >> board designer to enable the same as it may have unexpected side effects. >> >> These should be validated for proper functioning & detection of devices >> in secondary bus as sometimes it can cause secondary bus being disabled. >> >> Add booleans for: >> - maxim,isolate-stuck-channel >> - maxim,send-flush-out-sequence >> - maxim,preconnection-wiggle-test-enable >> >> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> >> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> >> Reviewed-by: Rob Herring <robh@kernel.org> > > Rob, are you really OK with these bindings? They look more like > configuration instead of HW description to me. Some explanation was provided here: https://lore.kernel.org/all/CABqG17g8QOgU7cObe=4EMLbEC1PeZWxdPXt7zzFs35JGqpRbfg@mail.gmail.com/ AFAIU, these properties are board-design choice. Best regards, Krzysztof
> Some explanation was provided here: > https://lore.kernel.org/all/CABqG17g8QOgU7cObe=4EMLbEC1PeZWxdPXt7zzFs35JGqpRbfg@mail.gmail.com/ > > AFAIU, these properties are board-design choice. Hey, thanks for the heads up. I agree that these options should not be "on" by default. I am still not fully convinced they serve as hardware description, though. Need to think about it some more...
Hi Wolfram, On Fri, 22 Dec 2023 at 16:05, Wolfram Sang <wsa@kernel.org> wrote: > > > > Some explanation was provided here: > > https://lore.kernel.org/all/CABqG17g8QOgU7cObe=4EMLbEC1PeZWxdPXt7zzFs35JGqpRbfg@mail.gmail.com/ > > > > AFAIU, these properties are board-design choice. > > Hey, thanks for the heads up. I agree that these options should not be > "on" by default. I am still not fully convinced they serve as hardware > description, though. Need to think about it some more... Any update on this ? Let me know if I can help with additional information. Regards, Naresh >
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml index 2d7bb998b0e9..9aa0585200c9 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml @@ -71,6 +71,23 @@ properties: description: A voltage regulator supplying power to the chip. On PCA9846 the regulator supplies power to VDD2 (core logic) and optionally to VDD1. + maxim,isolate-stuck-channel: + type: boolean + description: Allows to use non faulty channels while a stuck channel is + isolated from the upstream bus. If not set all channels are isolated from + the upstream bus until the fault is cleared. + + maxim,send-flush-out-sequence: + type: boolean + description: Send a flush-out sequence to stuck auxiliary buses + automatically after a stuck channel is being detected. + + maxim,preconnection-wiggle-test-enable: + type: boolean + description: Send a STOP condition to the auxiliary buses when the switch + register activates a channel to detect a stuck high fault. On fault the + channel is isolated from the upstream bus. + required: - compatible - reg @@ -95,6 +112,19 @@ allOf: "#interrupt-cells": false interrupt-controller: false + - if: + not: + properties: + compatible: + contains: + enum: + - maxim,max7357 + then: + properties: + maxim,isolate-stuck-channel: false + maxim,send-flush-out-sequence: false + maxim,preconnection-wiggle-test-enable: false + unevaluatedProperties: false examples: