Message ID | 20230501091552.847240-3-patrick.rudolph@9elements.com |
---|---|
State | Superseded |
Headers | show |
Series | Add support for Maxim MAX735x/MAX736x variants | expand |
Hi! 2023-05-01 at 11:15, Patrick Rudolph wrote: > Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x > chips. The functionality will be provided by the existing pca954x driver. > > For chips that are powered off by default add a regulator called vdd-supply. > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > .../bindings/i2c/i2c-mux-pca954x.yaml | 22 +++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > index e5c1070903ef..6fed6eae9c9b 100644 > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > @@ -4,18 +4,29 @@ > $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 <laurent.pinchart@ideasonboard.com> > > description: > - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices. > + The NXP PCA954x and compatible devices are I2C bus > + multiplexer/switches that share the same functionality > + and register layout. > + The devices usually have 4 or 8 child buses, which are > + attached to the parent bus by using the SMBus "Send Byte" > + command. > > properties: > compatible: > oneOf: > - enum: > + - maxim,max7356 > + - maxim,max7357 > + - maxim,max7358 > + - maxim,max7367 > + - maxim,max7368 > + - maxim,max7369 > - nxp,pca9540 > - nxp,pca9542 > - nxp,pca9543 > @@ -56,6 +67,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. > + The pca9846-9 chips do not have one VDD, they have separate supplies for "low level" (VDD1), and "core logic" (VDD2). I don't know how such a situation is normally reflected in bindings, but could it not cause headaches later if use of unqualified VDD is spreading for those chips? Possibly with different semantics depending on if vdd-supply is tied to VDD1, VDD2 or both? Cheers, Peter > required: > - compatible > - reg > @@ -68,6 +82,8 @@ allOf: > compatible: > contains: > enum: > + - maxim,max7367 > + - maxim,max7369 > - nxp,pca9542 > - nxp,pca9543 > - nxp,pca9544 > @@ -94,6 +110,8 @@ examples: > #size-cells = <0>; > reg = <0x74>; > > + vdd-supply = <&p3v3>; > + > interrupt-parent = <&ipic>; > interrupts = <17 IRQ_TYPE_LEVEL_LOW>; > interrupt-controller;
Hi Peter, it could indeed cause problems when VDD1 != VDD2 and at both needs to be enabled. The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add an optional "vdd2" regulator to the binding and driver. Please let me know if that's what you had in mind. Regards, Patrick On Tue, May 2, 2023 at 8:03 AM Peter Rosin <peda@axentia.se> wrote: > > Hi! > > 2023-05-01 at 11:15, Patrick Rudolph wrote: > > Update the pca954x bindings to add support for the Maxim MAX735x/MAX736x > > chips. The functionality will be provided by the existing pca954x driver. > > > > For chips that are powered off by default add a regulator called vdd-supply. > > > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > --- > > .../bindings/i2c/i2c-mux-pca954x.yaml | 22 +++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > > index e5c1070903ef..6fed6eae9c9b 100644 > > --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > > +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml > > @@ -4,18 +4,29 @@ > > $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 <laurent.pinchart@ideasonboard.com> > > > > description: > > - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices. > > + The NXP PCA954x and compatible devices are I2C bus > > + multiplexer/switches that share the same functionality > > + and register layout. > > + The devices usually have 4 or 8 child buses, which are > > + attached to the parent bus by using the SMBus "Send Byte" > > + command. > > > > properties: > > compatible: > > oneOf: > > - enum: > > + - maxim,max7356 > > + - maxim,max7357 > > + - maxim,max7358 > > + - maxim,max7367 > > + - maxim,max7368 > > + - maxim,max7369 > > - nxp,pca9540 > > - nxp,pca9542 > > - nxp,pca9543 > > @@ -56,6 +67,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. > > + > > The pca9846-9 chips do not have one VDD, they have separate supplies for > "low level" (VDD1), and "core logic" (VDD2). I don't know how such a > situation is normally reflected in bindings, but could it not cause > headaches later if use of unqualified VDD is spreading for those chips? > Possibly with different semantics depending on if vdd-supply is tied to > VDD1, VDD2 or both? > > Cheers, > Peter > > > required: > > - compatible > > - reg > > @@ -68,6 +82,8 @@ allOf: > > compatible: > > contains: > > enum: > > + - maxim,max7367 > > + - maxim,max7369 > > - nxp,pca9542 > > - nxp,pca9543 > > - nxp,pca9544 > > @@ -94,6 +110,8 @@ examples: > > #size-cells = <0>; > > reg = <0x74>; > > > > + vdd-supply = <&p3v3>; > > + > > interrupt-parent = <&ipic>; > > interrupts = <17 IRQ_TYPE_LEVEL_LOW>; > > interrupt-controller;
On 02/05/2023 08:52, Patrick Rudolph wrote: > Hi Peter, > it could indeed cause problems when VDD1 != VDD2 and at both needs to > be enabled. > The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add > an optional "vdd2" regulator to the binding and driver. > > Please let me know if that's what you had in mind. Don't top post. In such case vdd-supply should not be used for VDD2. Best regards, Krzysztof
Hi! 2023-05-02 at 10:46, Krzysztof Kozlowski wrote: > On 02/05/2023 08:52, Patrick Rudolph wrote: >> Hi Peter, >> it could indeed cause problems when VDD1 != VDD2 and at both needs to >> be enabled. >> The pca9846 datasheet seems to refer to VDD1 as VDD. Thus I could add >> an optional "vdd2" regulator to the binding and driver. >> >> Please let me know if that's what you had in mind. > > Don't top post. > > In such case vdd-supply should not be used for VDD2. When reading the data sheet [1], I get the feeling that the instances of VDD are either copy-paste errors from data sheets from chip with a single VDD, or a reference to either of VDD1 or VDD2. It is thus not super clear to me that VDD should be the same thing as VDD1. Sure, there is section 6.5 "Power-on reset", which mentions VDD and VDD2 (but not VDD1), but that seems like a simply typo and that it should really have been VDD1 instead of an unqualified VDD. There are also various timings "glitch supply voltage difference" (delta VDD(gl)) and "supply voltage glitch pulse width" (t w(gl)VDD) with notes that refer to VDD2, which *could* indicate that the glitch in VDD is about a glitch VDD1. But it could also mean glitches on any of VDD1 and VDD2? The general description of the chip indicates that VDD1 is there mainly to allow different bus voltages on each of the channels. Which is not at all the function of VDD on the other chips. Meanwhile VDD2 "is the core logic supply from which most of the PCA9846 circuitry runs", and seems like it is a better match for plain VDD? Maybe one can find out more by reading the spec more carefully, but as I said, it is not clear to me that either of VDD1 or VDD2 can be matched to VDD. Perhaps it is best to not mix things at all? [1] https://www.nxp.com/docs/en/data-sheet/PCA9846.pdf
diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml index e5c1070903ef..6fed6eae9c9b 100644 --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml @@ -4,18 +4,29 @@ $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 <laurent.pinchart@ideasonboard.com> description: - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices. + The NXP PCA954x and compatible devices are I2C bus + multiplexer/switches that share the same functionality + and register layout. + The devices usually have 4 or 8 child buses, which are + attached to the parent bus by using the SMBus "Send Byte" + command. properties: compatible: oneOf: - enum: + - maxim,max7356 + - maxim,max7357 + - maxim,max7358 + - maxim,max7367 + - maxim,max7368 + - maxim,max7369 - nxp,pca9540 - nxp,pca9542 - nxp,pca9543 @@ -56,6 +67,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 @@ -68,6 +82,8 @@ allOf: compatible: contains: enum: + - maxim,max7367 + - maxim,max7369 - nxp,pca9542 - nxp,pca9543 - nxp,pca9544 @@ -94,6 +110,8 @@ examples: #size-cells = <0>; reg = <0x74>; + vdd-supply = <&p3v3>; + interrupt-parent = <&ipic>; interrupts = <17 IRQ_TYPE_LEVEL_LOW>; interrupt-controller;