| Message ID | 20220505184037.511295-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add RZ/G2UL support | expand |
Hi Biju, On Thu, May 5, 2022 at 8:40 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > on the RZ/G2L. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Started using generic compatible for RZ/G2UL and added SoC specific validation > for channels. Thanks for the update! > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > @@ -74,18 +75,48 @@ patternProperties: > Represents the external channels which are connected to the ADC. > > properties: > - reg: > - description: | > - The channel number. It can have up to 8 channels numbered from 0 to 7. > - items: > - - minimum: 0 > - maximum: 7 > - > + reg: true > required: > - reg > > additionalProperties: false > > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: renesas,r9a07g043-adc > + then: > + patternProperties: > + "^channel@[0-7]$": [0-1] > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 2 channels numbered from 0 to 1. > + items: > + - minimum: 0 > + maximum: 1 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - renesas,r9a07g044-adc > + - renesas,r9a07g054-adc > + then: > + patternProperties: > + "^channel@[0-7]$": > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 8 channels numbered from 0 to 7. > + items: > + - minimum: 0 > + maximum: 7 > + > additionalProperties: false > > examples: The rest LGTM, but I'm wondering if more of the channel subnodes description can be factored out to the common part? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > Hi Biju, > > On Thu, May 5, 2022 at 8:40 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > > on the RZ/G2L. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v1->v2: > > * Started using generic compatible for RZ/G2UL and added SoC specific > validation > > for channels. > > Thanks for the update! > > > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > @@ -74,18 +75,48 @@ patternProperties: > > Represents the external channels which are connected to the ADC. > > > > properties: > > - reg: > > - description: | > > - The channel number. It can have up to 8 channels numbered from > 0 to 7. > > - items: > > - - minimum: 0 > > - maximum: 7 > > - > > + reg: true > > required: > > - reg > > > > additionalProperties: false > > > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: renesas,r9a07g043-adc > > + then: > > + patternProperties: > > + "^channel@[0-7]$": > > [0-1] Looks like with this change, validation doesn't work as expected. Please see the logs Test 1:- Update the current example with renesas,r9a07g043-adc and run dt-binding check, We get proper results. biju@biju-VirtualBox:~/rzg2l-linux$ git diff diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml index 2da3538a3543..a349e307f11e 100644 --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml @@ -125,7 +125,7 @@ examples: #include <dt-bindings/interrupt-controller/arm-gic.h> adc: adc@10059000 { - compatible = "renesas,r9a07g044-adc", "renesas,rzg2l-adc"; + compatible = "renesas,r9a07g043-adc", "renesas,rzg2l-adc"; reg = <0x10059000 0x400>; interrupts = <GIC_SPI 347 IRQ_TYPE_EDGE_RISING>; clocks = <&cpg CPG_MOD R9A07G044_ADC_ADCLK>, biju@biju-VirtualBox:~/rzg2l-linux$ biju@biju-VirtualBox:~/rzg2l-linux$ make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dt_binding_check -j8 LINT Documentation/devicetree/bindings DTEX Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dts CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb CHECK Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@2:reg:0:0: 2 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@3:reg:0:0: 3 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@4:reg:0:0: 4 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@5:reg:0:0: 5 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@6:reg:0:0: 6 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb: adc@10059000: channel@7:reg:0:0: 7 is greater than the maximum of 1 From schema: /home/biju/rzg2l-linux/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml biju@biju-VirtualBox:~/rzg2l-linux$ Test2: Update the current example with [0,1] change and renesas,r9a07g043-adc and run dt-binding check. Checking is passing for channel[2,7]. The results are not expected one, I am not sure is it dt-schema related issue?? biju@biju-VirtualBox:~/rzg2l-linux$ git diff --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml @@ -89,7 +89,7 @@ allOf: const: renesas,r9a07g043-adc then: patternProperties: - "^channel@[0-7]$": + "^channel@[0-1]$": type: object properties: reg: @@ -125,7 +125,7 @@ examples: #include <dt-bindings/interrupt-controller/arm-gic.h> adc: adc@10059000 { - compatible = "renesas,r9a07g044-adc", "renesas,rzg2l-adc"; + compatible = "renesas,r9a07g043-adc", "renesas,rzg2l-adc"; reg = <0x10059000 0x400>; interrupts = <GIC_SPI 347 IRQ_TYPE_EDGE_RISING>; clocks = <&cpg CPG_MOD R9A07G044_ADC_ADCLK>, biju@biju-VirtualBox:~/rzg2l-linux$ biju@biju-VirtualBox:~/rzg2l-linux$ make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dt_binding_check -j8 LINT Documentation/devicetree/bindings DTEX Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dts CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb CHECK Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb biju@biju-VirtualBox:~/rzg2l-linux$ > > > + type: object > > + properties: > > + reg: > > + description: | > > + The channel number. It can have up to 2 channels > numbered from 0 to 1. > > + items: > > + - minimum: 0 > > + maximum: 1 > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - renesas,r9a07g044-adc > > + - renesas,r9a07g054-adc > > + then: > > + patternProperties: > > + "^channel@[0-7]$": > > + type: object > > + properties: > > + reg: > > + description: | > > + The channel number. It can have up to 8 channels > numbered from 0 to 7. > > + items: > > + - minimum: 0 > > + maximum: 7 > > + > > additionalProperties: false > > > > examples: > > The rest LGTM, but I'm wondering if more of the channel subnodes > description can be factored out to the common part? You mean above reg: true?? ie, add as part of the below description?? type: object description: | Represents the external channels which are connected to the ADC. Cheers, Biju > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. > But when I'm talking to journalists I just say "programmer" or something > like that. > -- Linus Torvalds
Hi Biju, Thank you for the patch. On Fri, May 6, 2022 at 2:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > on the RZ/G2L. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Started using generic compatible for RZ/G2UL and added SoC specific validation > for channels. > --- > .../bindings/iio/adc/renesas,rzg2l-adc.yaml | 45 ++++++++++++++++--- > 1 file changed, 38 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > index d66c24cae1e1..2da3538a3543 100644 > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > @@ -19,6 +19,7 @@ properties: > compatible: > items: > - enum: > + - renesas,r9a07g043-adc # RZ/G2UL > - renesas,r9a07g044-adc # RZ/G2L > - renesas,r9a07g054-adc # RZ/V2L > - const: renesas,rzg2l-adc > @@ -74,18 +75,48 @@ patternProperties: > Represents the external channels which are connected to the ADC. > > properties: > - reg: > - description: | > - The channel number. It can have up to 8 channels numbered from 0 to 7. > - items: > - - minimum: 0 > - maximum: 7 > - > + reg: true > required: > - reg > > additionalProperties: false > > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: renesas,r9a07g043-adc > + then: > + patternProperties: > + "^channel@[0-7]$": > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 2 channels numbered from 0 to 1. > + items: > + - minimum: 0 > + maximum: 1 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - renesas,r9a07g044-adc > + - renesas,r9a07g054-adc > + then: Can the above hunk be replaced by else instead? Cheers, Prabhakar > + patternProperties: > + "^channel@[0-7]$": > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 8 channels numbered from 0 to 7. > + items: > + - minimum: 0 > + maximum: 7 > + > additionalProperties: false > > examples: > -- > 2.25.1 >
On Thu, 5 May 2022 19:40:36 +0100 Biju Das <biju.das.jz@bp.renesas.com> wrote: > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > on the RZ/G2L. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v1->v2: > * Started using generic compatible for RZ/G2UL and added SoC specific validation > for channels. > --- > .../bindings/iio/adc/renesas,rzg2l-adc.yaml | 45 ++++++++++++++++--- > 1 file changed, 38 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > index d66c24cae1e1..2da3538a3543 100644 > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > @@ -19,6 +19,7 @@ properties: > compatible: > items: > - enum: > + - renesas,r9a07g043-adc # RZ/G2UL > - renesas,r9a07g044-adc # RZ/G2L > - renesas,r9a07g054-adc # RZ/V2L > - const: renesas,rzg2l-adc > @@ -74,18 +75,48 @@ patternProperties: > Represents the external channels which are connected to the ADC. > > properties: > - reg: > - description: | > - The channel number. It can have up to 8 channels numbered from 0 to 7. Leave description: | The channel number. here. The rest of this description is obvious from the below schema so doesn't need to also be in the descriptions. > - items: > - - minimum: 0 > - maximum: 7 > - > + reg: true > required: > - reg > > additionalProperties: false > > +allOf: > + - if: > + properties: > + compatible: > + contains: > + const: renesas,r9a07g043-adc > + then: > + patternProperties: > + "^channel@[0-7]$": > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 2 channels numbered from 0 to 1. > + items: > + - minimum: 0 > + maximum: 1 > + - if: > + properties: > + compatible: > + contains: > + enum: > + - renesas,r9a07g044-adc > + - renesas,r9a07g054-adc > + then: > + patternProperties: > + "^channel@[0-7]$": > + type: object > + properties: > + reg: > + description: | > + The channel number. It can have up to 8 channels numbered from 0 to 7. > + items: > + - minimum: 0 > + maximum: 7 > + > additionalProperties: false > > examples:
Hi Biju, On Fri, May 6, 2022 at 10:40 AM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > > On Thu, May 5, 2022 at 8:40 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > > > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > > > on the RZ/G2L. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > @@ -74,18 +75,48 @@ patternProperties: > > > Represents the external channels which are connected to the ADC. > > > > > > properties: > > > - reg: > > > - description: | > > > - The channel number. It can have up to 8 channels numbered from > > 0 to 7. > > > - items: > > > - - minimum: 0 > > > - maximum: 7 > > > - > > > + reg: true > > > required: > > > - reg > > > > > > additionalProperties: false > > > > > > +allOf: > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: renesas,r9a07g043-adc > > > + then: > > > + patternProperties: > > > + "^channel@[0-7]$": > > > > [0-1] > > Looks like with this change, validation doesn't work as expected. OK, keep it at [0-7]. > > > + type: object > > > + properties: > > > + reg: > > > + description: | > > > + The channel number. It can have up to 2 channels > > numbered from 0 to 1. > > > + items: > > > + - minimum: 0 > > > + maximum: 1 > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + enum: > > > + - renesas,r9a07g044-adc > > > + - renesas,r9a07g054-adc > > > + then: > > > + patternProperties: > > > + "^channel@[0-7]$": > > > + type: object > > > + properties: > > > + reg: > > > + description: | > > > + The channel number. It can have up to 8 channels > > numbered from 0 to 7. > > > + items: > > > + - minimum: 0 > > > + maximum: 7 > > > + > > > additionalProperties: false > > > > > > examples: > > > > The rest LGTM, but I'm wondering if more of the channel subnodes > > description can be factored out to the common part? > > You mean above reg: true?? ie, add as part of the below description?? > > type: object > description: | > Represents the external channels which are connected to the ADC. I think I've found a solution while converting the R-Car Gen2 USB PHY bindings. You can mark channels 3-7 false on RZ/G2UL, cfr. the second channel on RZ/G1C in "[PATCH] dt-bindings: phy: renesas,rcar-gen2-usb-phy: Convert to json-schema" https://lore.kernel.org/r/8e48edc5e7b65f8dfd8b76c583e0265b9b97e62b.1652099944.git.geert+renesas@glider.be/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > Hi Biju, > > On Fri, May 6, 2022 at 10:40 AM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas > > > RZ/G2UL On Thu, May 5, 2022 at 8:40 PM Biju Das > <biju.das.jz@bp.renesas.com> wrote: > > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost > > > > identical to RZ/G2L, but it has 2 analog input channels compared > > > > to 8 channels on the RZ/G2L. > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > --- > > > > a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc. > > > > +++ yaml > > > > @@ -74,18 +75,48 @@ patternProperties: > > > > Represents the external channels which are connected to the > ADC. > > > > > > > > properties: > > > > - reg: > > > > - description: | > > > > - The channel number. It can have up to 8 channels numbered > from > > > 0 to 7. > > > > - items: > > > > - - minimum: 0 > > > > - maximum: 7 > > > > - > > > > + reg: true > > > > required: > > > > - reg > > > > > > > > additionalProperties: false > > > > > > > > +allOf: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + const: renesas,r9a07g043-adc > > > > + then: > > > > + patternProperties: > > > > + "^channel@[0-7]$": > > > > > > [0-1] > > > > Looks like with this change, validation doesn't work as expected. > > OK, keep it at [0-7]. OK. > > > > > + type: object > > > > + properties: > > > > + reg: > > > > + description: | > > > > + The channel number. It can have up to 2 channels > > > numbered from 0 to 1. > > > > + items: > > > > + - minimum: 0 > > > > + maximum: 1 > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - renesas,r9a07g044-adc > > > > + - renesas,r9a07g054-adc > > > > + then: > > > > + patternProperties: > > > > + "^channel@[0-7]$": > > > > + type: object > > > > + properties: > > > > + reg: > > > > + description: | > > > > + The channel number. It can have up to 8 channels > > > numbered from 0 to 7. > > > > + items: > > > > + - minimum: 0 > > > > + maximum: 7 > > > > + > > > > additionalProperties: false > > > > > > > > examples: > > > > > > The rest LGTM, but I'm wondering if more of the channel subnodes > > > description can be factored out to the common part? > > > > You mean above reg: true?? ie, add as part of the below description?? > > > > type: object > > description: | > > Represents the external channels which are connected to the ADC. > > I think I've found a solution while converting the R-Car Gen2 USB PHY > bindings. You can mark channels 3-7 false on RZ/G2UL, cfr. the second > channel on RZ/G1C in "[PATCH] dt-bindings: I just added similar check for RZ/G2UL by making channel@2-7 false and But dt-binding checks is passing with channel@2-7 present in the example. Validation should fail, but it is passing. I will go with current implementation, as it does proper validation. +if: + properties: + compatible: + contains: + const: renesas,r9a07g043-adc + then: + properties: + channel@2: false + channel@3: false + channel@4: false + channel@5: false + channel@6: false + channel@7: false additionalProperties: false @@ -125,7 +105,7 @@ examples: #include <dt-bindings/interrupt-controller/arm-gic.h> adc: adc@10059000 { - compatible = "renesas,r9a07g044-adc", "renesas,rzg2l-adc"; + compatible = "renesas,r9a07g043-adc", "renesas,rzg2l-adc"; $ make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dt_binding_check -j8 LINT Documentation/devicetree/bindings DTEX Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dts CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTC Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb CHECK Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb Cheers, Biju
Hi Jonathan, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > On Thu, 5 May 2022 19:40:36 +0100 > Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical > > to RZ/G2L, but it has 2 analog input channels compared to 8 channels > > on the RZ/G2L. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v1->v2: > > * Started using generic compatible for RZ/G2UL and added SoC specific > validation > > for channels. > > --- > > .../bindings/iio/adc/renesas,rzg2l-adc.yaml | 45 ++++++++++++++++--- > > 1 file changed, 38 insertions(+), 7 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > index d66c24cae1e1..2da3538a3543 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > @@ -19,6 +19,7 @@ properties: > > compatible: > > items: > > - enum: > > + - renesas,r9a07g043-adc # RZ/G2UL > > - renesas,r9a07g044-adc # RZ/G2L > > - renesas,r9a07g054-adc # RZ/V2L > > - const: renesas,rzg2l-adc > > @@ -74,18 +75,48 @@ patternProperties: > > Represents the external channels which are connected to the ADC. > > > > properties: > > - reg: > > - description: | > > - The channel number. It can have up to 8 channels numbered from > 0 to 7. > Leave > description: | > The channel number. > here. The rest of this description is obvious from the below schema so > doesn't need to also be in the descriptions. OK will do the changes in next revision. Cheers, Biju > > > - items: > > - - minimum: 0 > > - maximum: 7 > > - > > + reg: true > > required: > > - reg > > > > additionalProperties: false > > > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: renesas,r9a07g043-adc > > + then: > > + patternProperties: > > + "^channel@[0-7]$": > > + type: object > > + properties: > > + reg: > > + description: | > > + The channel number. It can have up to 2 channels > numbered from 0 to 1. > > + items: > > + - minimum: 0 > > + maximum: 1 > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - renesas,r9a07g044-adc > > + - renesas,r9a07g054-adc > > + then: > > + patternProperties: > > + "^channel@[0-7]$": > > + type: object > > + properties: > > + reg: > > + description: | > > + The channel number. It can have up to 8 channels > numbered from 0 to 7. > > + items: > > + - minimum: 0 > > + maximum: 7 > > + > > additionalProperties: false > > > > examples:
On Tue, May 10, 2022 at 05:54:59AM +0000, Biju Das wrote: > Hi Geert, > > Thanks for the feedback. > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > > ADC > > > > Hi Biju, > > > > On Fri, May 6, 2022 at 10:40 AM Biju Das <biju.das.jz@bp.renesas.com> > > wrote: > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas > > > > RZ/G2UL On Thu, May 5, 2022 at 8:40 PM Biju Das > > <biju.das.jz@bp.renesas.com> wrote: > > > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost > > > > > identical to RZ/G2L, but it has 2 analog input channels compared > > > > > to 8 channels on the RZ/G2L. > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > --- > > > > > a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc. > > > > > +++ yaml > > > > > @@ -74,18 +75,48 @@ patternProperties: > > > > > Represents the external channels which are connected to the > > ADC. > > > > > > > > > > properties: > > > > > - reg: > > > > > - description: | > > > > > - The channel number. It can have up to 8 channels numbered > > from > > > > 0 to 7. > > > > > - items: > > > > > - - minimum: 0 > > > > > - maximum: 7 > > > > > - > > > > > + reg: true > > > > > required: > > > > > - reg > > > > > > > > > > additionalProperties: false > > > > > > > > > > +allOf: > > > > > + - if: > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + const: renesas,r9a07g043-adc > > > > > + then: > > > > > + patternProperties: > > > > > + "^channel@[0-7]$": > > > > > > > > [0-1] > > > > > > Looks like with this change, validation doesn't work as expected. > > > > OK, keep it at [0-7]. > > OK. > > > > > > > > + type: object > > > > > + properties: > > > > > + reg: > > > > > + description: | > > > > > + The channel number. It can have up to 2 channels > > > > numbered from 0 to 1. > > > > > + items: > > > > > + - minimum: 0 > > > > > + maximum: 1 > > > > > + - if: > > > > > + properties: > > > > > + compatible: > > > > > + contains: > > > > > + enum: > > > > > + - renesas,r9a07g044-adc > > > > > + - renesas,r9a07g054-adc > > > > > + then: > > > > > + patternProperties: > > > > > + "^channel@[0-7]$": > > > > > + type: object > > > > > + properties: > > > > > + reg: > > > > > + description: | > > > > > + The channel number. It can have up to 8 channels > > > > numbered from 0 to 7. > > > > > + items: > > > > > + - minimum: 0 > > > > > + maximum: 7 > > > > > + > > > > > additionalProperties: false > > > > > > > > > > examples: > > > > > > > > The rest LGTM, but I'm wondering if more of the channel subnodes > > > > description can be factored out to the common part? > > > > > > You mean above reg: true?? ie, add as part of the below description?? > > > > > > type: object > > > description: | > > > Represents the external channels which are connected to the ADC. > > > > I think I've found a solution while converting the R-Car Gen2 USB PHY > > bindings. You can mark channels 3-7 false on RZ/G2UL, cfr. the second > > channel on RZ/G1C in "[PATCH] dt-bindings: > > I just added similar check for RZ/G2UL by making channel@2-7 false and But dt-binding checks is passing > with channel@2-7 present in the example. Validation should fail, but it is passing. I will go with current implementation, as it does proper validation. > > +if: > + properties: > + compatible: > + contains: > + const: renesas,r9a07g043-adc > + then: > + properties: > + channel@2: false > + channel@3: false > + channel@4: false > + channel@5: false > + channel@6: false > + channel@7: false patternProperties: '^channel@[2-7]$': false > > additionalProperties: false > > @@ -125,7 +105,7 @@ examples: > #include <dt-bindings/interrupt-controller/arm-gic.h> > > adc: adc@10059000 { > - compatible = "renesas,r9a07g044-adc", "renesas,rzg2l-adc"; > + compatible = "renesas,r9a07g043-adc", "renesas,rzg2l-adc"; > > $ make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml CROSS_COMPILE=~/gcc-arm-10.3-2021.07-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu- dt_binding_check -j8 > LINT Documentation/devicetree/bindings > DTEX Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dts > CHKDT Documentation/devicetree/bindings/processed-schema.json > SCHEMA Documentation/devicetree/bindings/processed-schema.json > DTC Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb > CHECK Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.example.dtb > > Cheers, > Biju > >
Hi Rob, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > On Tue, May 10, 2022 at 05:54:59AM +0000, Biju Das wrote: > > Hi Geert, > > > > Thanks for the feedback. > > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas > > > RZ/G2UL ADC > > > > > > Hi Biju, > > > > > > On Fri, May 6, 2022 at 10:40 AM Biju Das > > > <biju.das.jz@bp.renesas.com> > > > wrote: > > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document > > > > > Renesas RZ/G2UL On Thu, May 5, 2022 at 8:40 PM Biju Das > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost > > > > > > identical to RZ/G2L, but it has 2 analog input channels > > > > > > compared to 8 channels on the RZ/G2L. > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > > --- > > > > > > a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc. > > > > > > yaml > > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l- > adc. > > > > > > +++ yaml > > > > > > @@ -74,18 +75,48 @@ patternProperties: > > > > > > Represents the external channels which are connected to > > > > > > the > > > ADC. > > > > > > > > > > > > properties: > > > > > > - reg: > > > > > > - description: | > > > > > > - The channel number. It can have up to 8 channels > numbered > > > from > > > > > 0 to 7. > > > > > > - items: > > > > > > - - minimum: 0 > > > > > > - maximum: 7 > > > > > > - > > > > > > + reg: true > > > > > > required: > > > > > > - reg > > > > > > > > > > > > additionalProperties: false > > > > > > > > > > > > +allOf: > > > > > > + - if: > > > > > > + properties: > > > > > > + compatible: > > > > > > + contains: > > > > > > + const: renesas,r9a07g043-adc > > > > > > + then: > > > > > > + patternProperties: > > > > > > + "^channel@[0-7]$": > > > > > > > > > > [0-1] > > > > > > > > Looks like with this change, validation doesn't work as expected. > > > > > > OK, keep it at [0-7]. > > > > OK. > > > > > > > > > > > + type: object > > > > > > + properties: > > > > > > + reg: > > > > > > + description: | > > > > > > + The channel number. It can have up to 2 > > > > > > + channels > > > > > numbered from 0 to 1. > > > > > > + items: > > > > > > + - minimum: 0 > > > > > > + maximum: 1 > > > > > > + - if: > > > > > > + properties: > > > > > > + compatible: > > > > > > + contains: > > > > > > + enum: > > > > > > + - renesas,r9a07g044-adc > > > > > > + - renesas,r9a07g054-adc > > > > > > + then: > > > > > > + patternProperties: > > > > > > + "^channel@[0-7]$": > > > > > > + type: object > > > > > > + properties: > > > > > > + reg: > > > > > > + description: | > > > > > > + The channel number. It can have up to 8 > > > > > > + channels > > > > > numbered from 0 to 7. > > > > > > + items: > > > > > > + - minimum: 0 > > > > > > + maximum: 7 > > > > > > + > > > > > > additionalProperties: false > > > > > > > > > > > > examples: > > > > > > > > > > The rest LGTM, but I'm wondering if more of the channel subnodes > > > > > description can be factored out to the common part? > > > > > > > > You mean above reg: true?? ie, add as part of the below description?? > > > > > > > > type: object > > > > description: | > > > > Represents the external channels which are connected to the > ADC. > > > > > > I think I've found a solution while converting the R-Car Gen2 USB > > > PHY bindings. You can mark channels 3-7 false on RZ/G2UL, cfr. the > > > second channel on RZ/G1C in "[PATCH] dt-bindings: > > > > I just added similar check for RZ/G2UL by making channel@2-7 false and > > But dt-binding checks is passing with channel@2-7 present in the example. > Validation should fail, but it is passing. I will go with current > implementation, as it does proper validation. > > > > +if: > > + properties: > > + compatible: > > + contains: > > + const: renesas,r9a07g043-adc > > + then: > > + properties: > > + channel@2: false > > + channel@3: false > > + channel@4: false > > + channel@5: false > > + channel@6: false > > + channel@7: false > > patternProperties: > '^channel@[2-7]$': false This fixes the validation for channels. But how do we restrict reg index between 0-1 ? The below example is passing instead of failing. channel@0 { reg = <5>; }; The Current implementation restricts both channel and reg to [0-1] for RZ/G2UL. Regards, BIju
Hi Rob, > -----Original Message----- > From: Biju Das <biju.das.jz@bp.renesas.com> > Sent: 10 May 2022 19:31 > To: Rob Herring <robh@kernel.org> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>; Jonathan Cameron > <jic23@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Prabhakar Mahadev Lad > <prabhakar.mahadev-lad.rj@bp.renesas.com>; Lars-Peter Clausen > <lars@metafoo.de>; linux-iio@vger.kernel.org; Linux-Renesas <linux-renesas- > soc@vger.kernel.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE > BINDINGS <devicetree@vger.kernel.org>; Geert Uytterhoeven > <geert+renesas@glider.be>; Chris Paterson <Chris.Paterson2@renesas.com>; > Biju Das <biju.das@bp.renesas.com> > Subject: RE: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > Hi Rob, > > Thanks for the feedback. > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas > > RZ/G2UL ADC > > > > On Tue, May 10, 2022 at 05:54:59AM +0000, Biju Das wrote: > > > Hi Geert, > > > > > > Thanks for the feedback. > > > > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document > > > > Renesas RZ/G2UL ADC > > > > > > > > Hi Biju, > > > > > > > > On Fri, May 6, 2022 at 10:40 AM Biju Das > > > > <biju.das.jz@bp.renesas.com> > > > > wrote: > > > > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document > > > > > > Renesas RZ/G2UL On Thu, May 5, 2022 at 8:40 PM Biju Das > > > > <biju.das.jz@bp.renesas.com> wrote: > > > > > > > Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost > > > > > > > identical to RZ/G2L, but it has 2 analog input channels > > > > > > > compared to 8 channels on the RZ/G2L. > > > > > > > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > > > > > > > > --- > > > > > > > a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc. > > > > > > > yaml > > > > > > > +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2 > > > > > > > +++ l- > > adc. > > > > > > > +++ yaml > > > > > > > @@ -74,18 +75,48 @@ patternProperties: > > > > > > > Represents the external channels which are connected > > > > > > > to the > > > > ADC. > > > > > > > > > > > > > > properties: > > > > > > > - reg: > > > > > > > - description: | > > > > > > > - The channel number. It can have up to 8 channels > > numbered > > > > from > > > > > > 0 to 7. > > > > > > > - items: > > > > > > > - - minimum: 0 > > > > > > > - maximum: 7 > > > > > > > - > > > > > > > + reg: true > > > > > > > required: > > > > > > > - reg > > > > > > > > > > > > > > additionalProperties: false > > > > > > > > > > > > > > +allOf: > > > > > > > + - if: > > > > > > > + properties: > > > > > > > + compatible: > > > > > > > + contains: > > > > > > > + const: renesas,r9a07g043-adc > > > > > > > + then: > > > > > > > + patternProperties: > > > > > > > + "^channel@[0-7]$": > > > > > > > > > > > > [0-1] > > > > > > > > > > Looks like with this change, validation doesn't work as expected. > > > > > > > > OK, keep it at [0-7]. > > > > > > OK. > > > > > > > > > > > > > > + type: object > > > > > > > + properties: > > > > > > > + reg: > > > > > > > + description: | > > > > > > > + The channel number. It can have up to 2 > > > > > > > + channels > > > > > > numbered from 0 to 1. > > > > > > > + items: > > > > > > > + - minimum: 0 > > > > > > > + maximum: 1 > > > > > > > + - if: > > > > > > > + properties: > > > > > > > + compatible: > > > > > > > + contains: > > > > > > > + enum: > > > > > > > + - renesas,r9a07g044-adc > > > > > > > + - renesas,r9a07g054-adc > > > > > > > + then: > > > > > > > + patternProperties: > > > > > > > + "^channel@[0-7]$": > > > > > > > + type: object > > > > > > > + properties: > > > > > > > + reg: > > > > > > > + description: | > > > > > > > + The channel number. It can have up to 8 > > > > > > > + channels > > > > > > numbered from 0 to 7. > > > > > > > + items: > > > > > > > + - minimum: 0 > > > > > > > + maximum: 7 > > > > > > > + > > > > > > > additionalProperties: false > > > > > > > > > > > > > > examples: > > > > > > > > > > > > The rest LGTM, but I'm wondering if more of the channel > > > > > > subnodes description can be factored out to the common part? > > > > > > > > > > You mean above reg: true?? ie, add as part of the below > description?? > > > > > > > > > > type: object > > > > > description: | > > > > > Represents the external channels which are connected to > > > > > the > > ADC. > > > > > > > > I think I've found a solution while converting the R-Car Gen2 USB > > > > PHY bindings. You can mark channels 3-7 false on RZ/G2UL, cfr. > > > > the second channel on RZ/G1C in "[PATCH] dt-bindings: > > > > > > I just added similar check for RZ/G2UL by making channel@2-7 false > > > and But dt-binding checks is passing with channel@2-7 present in the > example. > > Validation should fail, but it is passing. I will go with current > > implementation, as it does proper validation. > > > > > > +if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: renesas,r9a07g043-adc > > > + then: > > > + properties: > > > + channel@2: false > > > + channel@3: false > > > + channel@4: false > > > + channel@5: false > > > + channel@6: false > > > + channel@7: false > > > > patternProperties: > > '^channel@[2-7]$': false > > This fixes the validation for channels. But how do we restrict reg index > between 0-1 ? > The below example is passing instead of failing. > channel@0 { > reg = <5>; > }; > > The Current implementation restricts both channel and reg to [0-1] for > RZ/G2UL. Looks like the below change is restricting both channel and reg to [0-1] for RZ/G2UL. I will send next version based on this. + "^channel@[2-7]$": false + "^channel@[0-1]$": Cheers, Biju
Hi Biju, On Tue, May 10, 2022 at 8:31 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > > ADC > > patternProperties: > > '^channel@[2-7]$': false > > This fixes the validation for channels. But how do we restrict reg index between 0-1 ? > The below example is passing instead of failing. > channel@0 { > reg = <5>; > }; I expect that to be flagged by the generic unit-address vs. reg check? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for the feedback. > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > ADC > > Hi Biju, > > On Tue, May 10, 2022 at 8:31 PM Biju Das <biju.das.jz@bp.renesas.com> > wrote: > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas > > > RZ/G2UL ADC > > > patternProperties: > > > '^channel@[2-7]$': false > > > > This fixes the validation for channels. But how do we restrict reg index > between 0-1 ? > > The below example is passing instead of failing. > > channel@0 { > > reg = <5>; > > }; > > I expect that to be flagged by the generic unit-address vs. reg check? You mean this should be take care by dt-schema automatically, right? Currently it is not the case. It needs additional check for taking care [1] [1] https://lore.kernel.org/linux-renesas-soc/CAMuHMdWNP_H9FNcygXZp0Ysw=wGXzV71Q_U7Hs=wH_Vctnz1pg@mail.gmail.com/T/#u Cheers, Biju
On Wed, May 11, 2022 at 08:53:53AM +0200, Geert Uytterhoeven wrote: > Hi Biju, > > On Tue, May 10, 2022 at 8:31 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL > > > ADC > > > patternProperties: > > > '^channel@[2-7]$': false > > > > This fixes the validation for channels. But how do we restrict reg index between 0-1 ? > > The below example is passing instead of failing. > > channel@0 { > > reg = <5>; > > }; > > I expect that to be flagged by the generic unit-address vs. reg check? No such thing. All the unit-address vs. reg value checks are bus specific (and in dtc). David G was against any generic/default check... Rob
diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml index d66c24cae1e1..2da3538a3543 100644 --- a/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml +++ b/Documentation/devicetree/bindings/iio/adc/renesas,rzg2l-adc.yaml @@ -19,6 +19,7 @@ properties: compatible: items: - enum: + - renesas,r9a07g043-adc # RZ/G2UL - renesas,r9a07g044-adc # RZ/G2L - renesas,r9a07g054-adc # RZ/V2L - const: renesas,rzg2l-adc @@ -74,18 +75,48 @@ patternProperties: Represents the external channels which are connected to the ADC. properties: - reg: - description: | - The channel number. It can have up to 8 channels numbered from 0 to 7. - items: - - minimum: 0 - maximum: 7 - + reg: true required: - reg additionalProperties: false +allOf: + - if: + properties: + compatible: + contains: + const: renesas,r9a07g043-adc + then: + patternProperties: + "^channel@[0-7]$": + type: object + properties: + reg: + description: | + The channel number. It can have up to 2 channels numbered from 0 to 1. + items: + - minimum: 0 + maximum: 1 + - if: + properties: + compatible: + contains: + enum: + - renesas,r9a07g044-adc + - renesas,r9a07g054-adc + then: + patternProperties: + "^channel@[0-7]$": + type: object + properties: + reg: + description: | + The channel number. It can have up to 8 channels numbered from 0 to 7. + items: + - minimum: 0 + maximum: 7 + additionalProperties: false examples:
Document Renesas RZ/G2UL ADC bindings. RZ/G2UL ADC is almost identical to RZ/G2L, but it has 2 analog input channels compared to 8 channels on the RZ/G2L. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v1->v2: * Started using generic compatible for RZ/G2UL and added SoC specific validation for channels. --- .../bindings/iio/adc/renesas,rzg2l-adc.yaml | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-)