diff mbox series

[v2,1/2] dt-bindings: iio: adc: Document Renesas RZ/G2UL ADC

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

Commit Message

Biju Das May 5, 2022, 6:40 p.m. UTC
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(-)

Comments

Geert Uytterhoeven May 6, 2022, 7:17 a.m. UTC | #1
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
Biju Das May 6, 2022, 8:40 a.m. UTC | #2
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
Lad, Prabhakar May 7, 2022, 5:38 a.m. UTC | #3
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
>
Jonathan Cameron May 7, 2022, 2:30 p.m. UTC | #4
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:
Geert Uytterhoeven May 9, 2022, 12:48 p.m. UTC | #5
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
Biju Das May 10, 2022, 5:54 a.m. UTC | #6
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
Biju Das May 10, 2022, 6 a.m. UTC | #7
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:
Rob Herring May 10, 2022, 4:14 p.m. UTC | #8
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
> 
>
Biju Das May 10, 2022, 6:31 p.m. UTC | #9
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
Biju Das May 11, 2022, 6:03 a.m. UTC | #10
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
Geert Uytterhoeven May 11, 2022, 6:53 a.m. UTC | #11
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
Biju Das May 11, 2022, 7:30 a.m. UTC | #12
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
Rob Herring May 18, 2022, 1:50 a.m. UTC | #13
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 mbox series

Patch

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: