[1/3] dt-bindings: iio: adc: Add GPADC for Allwinner A523

Message ID 20260510-sunxi-a523-gpadc-v1-1-4f6b0f4000fb@mmpsystems.pl (mailing list archive)
State New
Headers
Series Add GPADC support for A523 |

Commit Message

Michal Piekos May 10, 2026, 12:57 p.m. UTC
Add support for the GPADC for the Allwinner A523. It differs from the
D1/T113s/R329/T507 by having two clocks.

Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
 .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 37 +++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)
  

Comments

Rob Herring (Arm) May 10, 2026, 2:29 p.m. UTC | #1
On Sun, 10 May 2026 14:57:22 +0200, Michal Piekos wrote:
> Add support for the GPADC for the Allwinner A523. It differs from the
> D1/T113s/R329/T507 by having two clocks.
> 
> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> ---
>  .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 37 +++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml: allOf:0:then:properties:clocks: {'minItems': 2, 'maxItems': 2, 'items': [{'description': 'Bus clock'}, {'description': 'Module clock'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml: allOf:0:then:properties:clocks: 'oneOf' conditional failed, one must be fixed:
	False schema does not allow 2
	[{'description': 'Bus clock'}, {'description': 'Module clock'}] is too long
	[{'description': 'Bus clock'}, {'description': 'Module clock'}] is too short
	1 was expected
	hint: "minItems" is only needed if less than the "items" list length
	from schema $id: http://devicetree.org/meta-schemas/items.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml: allOf:0:then:properties:clock-names: {'minItems': 2, 'maxItems': 2, 'items': [{'const': 'bus'}, {'const': 'mod'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml: allOf:0:then:properties:clock-names: 'oneOf' conditional failed, one must be fixed:
	False schema does not allow 2
	[{'const': 'bus'}, {'const': 'mod'}] is too long
	[{'const': 'bus'}, {'const': 'mod'}] is too short
	1 was expected
	hint: "minItems" is only needed if less than the "items" list length
	from schema $id: http://devicetree.org/meta-schemas/items.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260510-sunxi-a523-gpadc-v1-1-4f6b0f4000fb@mmpsystems.pl

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
  
Jernej Škrabec May 10, 2026, 2:40 p.m. UTC | #2
Dne nedelja, 10. maj 2026 ob 14:57:22 Srednjeevropski poletni čas je Michal Piekos napisal(a):
> Add support for the GPADC for the Allwinner A523. It differs from the
> D1/T113s/R329/T507 by having two clocks.
> 
> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> ---
>  .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 37 +++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> index da605a051b94..89da96cd705f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> @@ -17,6 +17,7 @@ properties:
>        - items:
>            - enum:
>                - allwinner,sun50i-h616-gpadc
> +              - allwinner,sun55i-a523-gpadc

It shouldn't be combined if it has different number of clocks.

Best regards,
Jernej

>            - const: allwinner,sun20i-d1-gpadc
>  
>    "#io-channel-cells":
> @@ -29,7 +30,12 @@ properties:
>      const: 0
>  
>    clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 2
>  
>    interrupts:
>      maxItems: 1
> @@ -40,6 +46,35 @@ properties:
>    resets:
>      maxItems: 1
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          items:
> +            - const: allwinner,sun55i-a523-gpadc
> +            - const: allwinner,sun20i-d1-gpadc
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - description: Bus clock
> +            - description: Module clock
> +        clock-names:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - const: bus
> +            - const: mod
> +      required:
> +        - clock-names
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names: false
> +
>  patternProperties:
>    "^channel@[0-9a-f]+$":
>      $ref: adc.yaml
> 
>
  
Andre Przywara May 11, 2026, 4:02 p.m. UTC | #3
Hi Michal,

thanks for adding this!

On 5/10/26 14:57, Michal Piekos wrote:
> Add support for the GPADC for the Allwinner A523. It differs from the
> D1/T113s/R329/T507 by having two clocks.
> 
> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> ---
>   .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 37 +++++++++++++++++++++-
>   1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> index da605a051b94..89da96cd705f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> @@ -17,6 +17,7 @@ properties:
>         - items:
>             - enum:
>                 - allwinner,sun50i-h616-gpadc
> +              - allwinner,sun55i-a523-gpadc
>             - const: allwinner,sun20i-d1-gpadc

As Jernej already mentioned, the A523 GPADC is not fully compatible, 
since it adds another clock. The question to ask is: Can a driver only 
knowing about the fallback device handle this new device? For which the 
answer here is: No, it misses a clock.
So add just a single entry for the A523 (plus adding it to the driver).

So looking at this I wonder if we should add some property to describe 
the number of supported channels, since they are slightly different 
between the SoCs:
- The D1 manual mentions 2 channels.
- The T113s manual (same die as the D1?) describes 1 channel only.
- The T507 manual (same die as the H616) reports 4 channels.
- The A733 has 6 channels.
- The A133 has 1 channel, but it's channel 1, not 0.

So all of this is somewhat covered as channels are described as child 
nodes, and have a reg property. Ideally non-existing channels just 
wouldn't be listed, but I don't know if we want to rely on that.

So I am wondering if we should introduce a limit, or rather a mask (to 
cover the A133 oddity)?
Either a DT property (channel-mask, as a single sell representing the 
bit mask), or derived in the driver from the compatible string.
The former would avoid introducing different compatible strings just 
because of that, though I think this type of property is somewhat 
discouraged?

Any thoughts?

>   
>     "#io-channel-cells":
> @@ -29,7 +30,12 @@ properties:
>       const: 0
>   
>     clocks:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 2
>   
>     interrupts:
>       maxItems: 1
> @@ -40,6 +46,35 @@ properties:
>     resets:
>       maxItems: 1
>   
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          items:
> +            - const: allwinner,sun55i-a523-gpadc
> +            - const: allwinner,sun20i-d1-gpadc
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - description: Bus clock
> +            - description: Module clock

I am not a YAML expert, but I think you can drop the min and max 
properties, if you just enumerate the cases. Same for the names.

Cheers,
Andre

> +        clock-names:
> +          minItems: 2
> +          maxItems: 2
> +          items:
> +            - const: bus
> +            - const: mod
> +      required:
> +        - clock-names
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names: false
> +
>   patternProperties:
>     "^channel@[0-9a-f]+$":
>       $ref: adc.yaml
>
  
Michal Piekos May 12, 2026, 3:56 a.m. UTC | #4
On Mon, May 11, 2026 at 06:02:16PM +0200, Andre Przywara wrote:
> Hi Michal,
> 
> thanks for adding this!
> 
> On 5/10/26 14:57, Michal Piekos wrote:
> > Add support for the GPADC for the Allwinner A523. It differs from the
> > D1/T113s/R329/T507 by having two clocks.
> > 
> > Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> > ---
> >   .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 37 +++++++++++++++++++++-
> >   1 file changed, 36 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> > index da605a051b94..89da96cd705f 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> > @@ -17,6 +17,7 @@ properties:
> >         - items:
> >             - enum:
> >                 - allwinner,sun50i-h616-gpadc
> > +              - allwinner,sun55i-a523-gpadc
> >             - const: allwinner,sun20i-d1-gpadc
> 
> As Jernej already mentioned, the A523 GPADC is not fully compatible, since
> it adds another clock. The question to ask is: Can a driver only knowing
> about the fallback device handle this new device? For which the answer here
> is: No, it misses a clock.
> So add just a single entry for the A523 (plus adding it to the driver).
> 
> So looking at this I wonder if we should add some property to describe the
> number of supported channels, since they are slightly different between the
> SoCs:
> - The D1 manual mentions 2 channels.
> - The T113s manual (same die as the D1?) describes 1 channel only.
> - The T507 manual (same die as the H616) reports 4 channels.
> - The A733 has 6 channels.
> - The A133 has 1 channel, but it's channel 1, not 0.
> 
> So all of this is somewhat covered as channels are described as child nodes,
> and have a reg property. Ideally non-existing channels just wouldn't be
> listed, but I don't know if we want to rely on that.
> 
> So I am wondering if we should introduce a limit, or rather a mask (to cover
> the A133 oddity)?
> Either a DT property (channel-mask, as a single sell representing the bit
> mask), or derived in the driver from the compatible string.
> The former would avoid introducing different compatible strings just because
> of that, though I think this type of property is somewhat discouraged?
> 
> Any thoughts?

Thanks. I have missed that sparse channels are also possible. I think in
current implementation it will cause issue with lookup through
<&gpadc 1> as sun20i_gpadc_alloc_channels would effectively create
channels[0].channel = 1 in such scenario. I will try to verify if my
logic is correct and provide separate fix if needed.

Regarding DT property masking channels there is already precedence:
atmel,adc-channels-used is a bitmask of muxed and enabled channels.
Not sure if preferable though...

Michal
> 
> >     "#io-channel-cells":
> > @@ -29,7 +30,12 @@ properties:
> >       const: 0
> >     clocks:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 2
> >     interrupts:
> >       maxItems: 1
> > @@ -40,6 +46,35 @@ properties:
> >     resets:
> >       maxItems: 1
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          items:
> > +            - const: allwinner,sun55i-a523-gpadc
> > +            - const: allwinner,sun20i-d1-gpadc
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 2
> > +          maxItems: 2
> > +          items:
> > +            - description: Bus clock
> > +            - description: Module clock
> 
> I am not a YAML expert, but I think you can drop the min and max properties,
> if you just enumerate the cases. Same for the names.
> 
> Cheers,
> Andre

I will correct that in v2.
Michal.
> 
> > +        clock-names:
> > +          minItems: 2
> > +          maxItems: 2
> > +          items:
> > +            - const: bus
> > +            - const: mod
> > +      required:
> > +        - clock-names
> > +    else:
> > +      properties:
> > +        clocks:
> > +          maxItems: 1
> > +        clock-names: false
> > +
> >   patternProperties:
> >     "^channel@[0-9a-f]+$":
> >       $ref: adc.yaml
> > 
>
  

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
index da605a051b94..89da96cd705f 100644
--- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
@@ -17,6 +17,7 @@  properties:
       - items:
           - enum:
               - allwinner,sun50i-h616-gpadc
+              - allwinner,sun55i-a523-gpadc
           - const: allwinner,sun20i-d1-gpadc
 
   "#io-channel-cells":
@@ -29,7 +30,12 @@  properties:
     const: 0
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     maxItems: 1
@@ -40,6 +46,35 @@  properties:
   resets:
     maxItems: 1
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          items:
+            - const: allwinner,sun55i-a523-gpadc
+            - const: allwinner,sun20i-d1-gpadc
+    then:
+      properties:
+        clocks:
+          minItems: 2
+          maxItems: 2
+          items:
+            - description: Bus clock
+            - description: Module clock
+        clock-names:
+          minItems: 2
+          maxItems: 2
+          items:
+            - const: bus
+            - const: mod
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
+
 patternProperties:
   "^channel@[0-9a-f]+$":
     $ref: adc.yaml