On 08/10/2022 13:50, Serge Semin wrote: > On Fri, Oct 07, 2022 at 09:53:50AM +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 <patrick.rudolph@xxxxxxxxxxxxx> >> --- >> .../bindings/i2c/i2c-mux-pca954x.yaml | 39 ++++++++++++++++--- >> 1 file changed, 34 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml >> index 9f1726d0356b..efad0a95806f 100644 >> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml >> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.yaml >> @@ -4,21 +4,25 @@ >> $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@xxxxxxxxxxxxxxxx> >> >> description: >> - The binding supports NXP PCA954x and PCA984x I2C mux/switch devices. >> - > >> -allOf: >> - - $ref: /schemas/i2c/i2c-mux.yaml# > > Why do you move the allOf statement to the bottom of the schema? Because it goes with 'ifs' at the bottom of the schema... > >> + The binding supports NXP PCA954x and PCA984x I2C mux/switch devices, >> + and the Maxim MAX735x and MAX736x I2C mux/switch devices. > > What about combining the sentence: "The binding supports NXP > PCA954x/PCA984x and Maxim MAX735x/MAX736x I2C mux/switch devices." ? > Currently it does look a bit bulky. Drop "The binding supports". Instead describe the hardware. > >> >> properties: >> compatible: >> oneOf: >> - enum: >> + - maxim,max7356 >> + - maxim,max7357 >> + - maxim,max7358 >> + - maxim,max7367 >> + - maxim,max7368 >> + - maxim,max7369 >> - nxp,pca9540 >> - nxp,pca9542 >> - nxp,pca9543 >> @@ -59,10 +63,33 @@ 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 >> >> +allOf: >> + - $ref: /schemas/i2c/i2c-mux.yaml# >> + - 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'd suggest to add an opposite definition. Evaluate the properties for > the devices which expect them being evaluated instead of falsing their > existence for the devices which don't support the interrupts. The properties rather should be defined in top-level than in "if", so I am not sure how would you want to achieve opposite way. Best regards, Krzysztof
