On Sun, Oct 09, 2022 at 05:25:22PM +0200, Krzysztof Kozlowski wrote: > 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... Is there a requirement to move the allOf array to the bottom of the schema if it contains the 'if' statement? If only there were some kernel doc with all such implicit conventions... > > > > >> + 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. With one more implicit convention like "preferably define the properties in the top-level than in if" of course I can't. Otherwise I thought something like this would work: +allOf: + - ... + - if: + properties: + compatible: + contains: + enum: [...] + then: + properties: + interrupts: ... + "#interrupt-cells": ... + interrupt-controller: ... ... - interrupts: - "#interrupt-cells": - interrupt-controller: ... With unevaluatedProperties set to false and evaluation performed for the particular compatibles such schema shall work with the same semantic. -Sergey > > > Best regards, > Krzysztof >
