[PATCH v2 0/3] Add GPADC support for A523

[PATCH v2 0/3] Add GPADC support for A523

[Michal Piekos]

Add support for Allwinner A523 GPADC in sun20i gpadc driver and describe
corresponding node in dts for A523 SoC.

A523 uses same model as existing driver except it has two clocks.

Added support to enable more than one clock in the driver, extended the
binding with new compatible and wired up dts node for A523 as its own
fallback compatible.

Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
Changes in v2:
- Handle scenario when 0 clocks has been defined
- Fix copy&paste sentence in 3rd patch commit message
- Moved status as last property in dts node
- Make A523 its own fallback compatible
- Removed redundant maxItems/minItems properties from binding
- Link to v1: https://patch.msgid.link/20260510-sunxi-a523-gpadc-v1-0-4f6b0f4000fb@mmpsystems.pl

To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Chen-Yu Tsai <wens@kernel.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Maksim Kiselev <bigunclemax@gmail.com>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-kernel@vger.kernel.org

---
Michal Piekos (3):
      dt-bindings: iio: adc: Add GPADC for Allwinner A523
      iio: adc: sun20i-gpadc: add A523 gpadc support
      arm64: dts: allwinner: a523: add gpadc node

 .../iio/adc/allwinner,sun20i-d1-gpadc.yaml         | 32 +++++++++++++++++++++-
 arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi     | 11 ++++++++
 drivers/iio/adc/sun20i-gpadc-iio.c                 | 11 +++++---
 3 files changed, 49 insertions(+), 5 deletions(-)
---
base-commit: 8ab992f815d6736b5c7a6f5fd7bfe7bc106bb3dc
change-id: 20260507-sunxi-a523-gpadc-1879aa5df754

Best regards,
--  
Michal Piekos <michal.piekos@mmpsystems.pl>

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

[Michal Piekos]

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         | 32 +++++++++++++++++++++-
 1 file changed, 31 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..6467800d30e2 100644
--- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
@@ -14,6 +14,7 @@ properties:
     oneOf:
       - enum:
           - allwinner,sun20i-d1-gpadc
+          - allwinner,sun55i-a523-gpadc
       - items:
           - enum:
               - allwinner,sun50i-h616-gpadc
@@ -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,30 @@ properties:
   resets:
     maxItems: 1
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - allwinner,sun55i-a523-gpadc
+    then:
+      properties:
+        clocks:
+          items:
+            - description: Bus clock
+            - description: Module clock
+        clock-names:
+          items:
+            - const: bus
+            - const: mod
+      required:
+        - clock-names
+    else:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
+
 patternProperties:
   "^channel@[0-9a-f]+$":
     $ref: adc.yaml

-- 
2.43.0

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

On Wed, May 13, 2026 at 06:59:42AM +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>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Schema Conditional Logic Error: `if` block will silently fail to match A523 compatible string
--

commit f1e37c6efc76bc9bd352a9e8462984a80a03b87c
Author: Michal Piekos <michal.piekos@mmpsystems.pl>

dt-bindings: iio: adc: Add GPADC for Allwinner A523

This commit adds support for the GPADC on the Allwinner A523. It updates
the schema to allow two clocks for this new compatible string while
restricting previous generations to one clock.

> 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 da605a051b949..6467800d30e27 100644
> --- a/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/allwinner,sun20i-d1-gpadc.yaml
[ ... ]
> @@ -40,6 +46,30 @@ properties:
>    resets:
>      maxItems: 1
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun55i-a523-gpadc

Will this condition successfully match the A523 compatible string?

In dt-schema, the compatible property is parsed and evaluated as an array of
strings. By using enum directly without a contains: keyword, this condition
expects the property to be exactly a string.

Under JSON schema rules, this condition will evaluate to false because an
array cannot equal a string.

> +    then:
[ ... ]
> +    else:
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names: false

Does this result in valid A523 device tree nodes being incorrectly rejected?

Since the if block silently fails to match, the schema will fall through to
this else block, enforcing maxItems: 1 for clocks and rejecting clock-names.
This would erroneously reject A523 configurations that correctly provide the
two required clocks.

Could the condition be updated to use contains: enum: or contains: const: to
properly match the compatible array elements?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-sunxi-a523-gpadc-v2-0-d5efde151dac@mmpsystems.pl?part=1

[PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Michal Piekos]

A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
clock and module clock.

Change driver to enable all clocks.

Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
 drivers/iio/adc/sun20i-gpadc-iio.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
index 861c14da75ad..3f1f07b3a385 100644
--- a/drivers/iio/adc/sun20i-gpadc-iio.c
+++ b/drivers/iio/adc/sun20i-gpadc-iio.c
@@ -180,7 +180,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
 	struct iio_dev *indio_dev;
 	struct sun20i_gpadc_iio *info;
 	struct reset_control *rst;
-	struct clk *clk;
+	struct clk_bulk_data *clks;
 	int irq;
 	int ret;
 
@@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
 	if (IS_ERR(info->regs))
 		return PTR_ERR(info->regs);
 
-	clk = devm_clk_get_enabled(dev, NULL);
-	if (IS_ERR(clk))
-		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
+	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
+	if (ret <= 0)
+		return dev_err_probe(
+			dev, ret,
+			"failed to enable clocks or no clocks defined\n");
 
 	rst = devm_reset_control_get_exclusive(dev, NULL);
 	if (IS_ERR(rst))
@@ -243,6 +245,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
 
 static const struct of_device_id sun20i_gpadc_of_id[] = {
 	{ .compatible = "allwinner,sun20i-d1-gpadc" },
+	{ .compatible = "allwinner,sun55i-a523-gpadc" },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun20i_gpadc_of_id);

-- 
2.43.0

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Sanjay Chitroda]

On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:
>A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
>clock and module clock.
>
>Change driver to enable all clocks.
>
>Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
>---
> drivers/iio/adc/sun20i-gpadc-iio.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
>index 861c14da75ad..3f1f07b3a385 100644
>--- a/drivers/iio/adc/sun20i-gpadc-iio.c
>+++ b/drivers/iio/adc/sun20i-gpadc-iio.c
>@@ -180,7 +180,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> 	struct iio_dev *indio_dev;
> 	struct sun20i_gpadc_iio *info;
> 	struct reset_control *rst;
>-	struct clk *clk;
>+	struct clk_bulk_data *clks;
> 	int irq;
> 	int ret;
> 
>@@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> 	if (IS_ERR(info->regs))
> 		return PTR_ERR(info->regs);
> 
>-	clk = devm_clk_get_enabled(dev, NULL);
>-	if (IS_ERR(clk))
>-		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
>+	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
>+	if (ret <= 0)

Thank you Michal for the change.

Have you validated the changes ?
It looks while success ret would be 0 and it would give return error.

Thanks, Sanjay


>+		return dev_err_probe(
>+			dev, ret,
>+			"failed to enable clocks or no clocks defined\n");
> 
> 	rst = devm_reset_control_get_exclusive(dev, NULL);
> 	if (IS_ERR(rst))
>@@ -243,6 +245,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> 
> static const struct of_device_id sun20i_gpadc_of_id[] = {
> 	{ .compatible = "allwinner,sun20i-d1-gpadc" },
>+	{ .compatible = "allwinner,sun55i-a523-gpadc" },
> 	{ }
> };
> MODULE_DEVICE_TABLE(of, sun20i_gpadc_of_id);
>

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andre Przywara]

Hi Sanjay,

thanks for having a look!

On 5/13/26 13:44, Sanjay Chitroda wrote:
> 
> 
> On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:
>> A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
>> clock and module clock.
>>
>> Change driver to enable all clocks.
>>
>> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
>> ---
>> drivers/iio/adc/sun20i-gpadc-iio.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
>> index 861c14da75ad..3f1f07b3a385 100644
>> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
>> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
>> @@ -180,7 +180,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
>> 	struct iio_dev *indio_dev;
>> 	struct sun20i_gpadc_iio *info;
>> 	struct reset_control *rst;
>> -	struct clk *clk;
>> +	struct clk_bulk_data *clks;
>> 	int irq;
>> 	int ret;
>>
>> @@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
>> 	if (IS_ERR(info->regs))
>> 		return PTR_ERR(info->regs);
>>
>> -	clk = devm_clk_get_enabled(dev, NULL);
>> -	if (IS_ERR(clk))
>> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
>> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
>> +	if (ret <= 0)
> 
> Thank you Michal for the change.
> 
> Have you validated the changes ?
> It looks while success ret would be 0 and it would give return error.

But devm_clk_bulk_get_all_enabled() returns the number of clocks found 
and enabled. And since we need at least one, I think this is correct, 
and the error message below reflects that.

To me that change looks good:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre


> 
> Thanks, Sanjay
> 
> 
>> +		return dev_err_probe(
>> +			dev, ret,
>> +			"failed to enable clocks or no clocks defined\n");
>>
>> 	rst = devm_reset_control_get_exclusive(dev, NULL);
>> 	if (IS_ERR(rst))
>> @@ -243,6 +245,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
>>
>> static const struct of_device_id sun20i_gpadc_of_id[] = {
>> 	{ .compatible = "allwinner,sun20i-d1-gpadc" },
>> +	{ .compatible = "allwinner,sun55i-a523-gpadc" },
>> 	{ }
>> };
>> MODULE_DEVICE_TABLE(of, sun20i_gpadc_of_id);
>>
> 

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Jonathan Cameron]

On Wed, 13 May 2026 13:53:49 +0200
Andre Przywara <andre.przywara@arm.com> wrote:

> Hi Sanjay,
> 
> thanks for having a look!
> 
> On 5/13/26 13:44, Sanjay Chitroda wrote:
> > 
> > 
> > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:  
> >> A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
> >> clock and module clock.
> >>
> >> Change driver to enable all clocks.
> >>
> >> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> >> ---
> >> drivers/iio/adc/sun20i-gpadc-iio.c | 11 +++++++----
> >> 1 file changed, 7 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
> >> index 861c14da75ad..3f1f07b3a385 100644
> >> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
> >> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
> >> @@ -180,7 +180,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> >> 	struct iio_dev *indio_dev;
> >> 	struct sun20i_gpadc_iio *info;
> >> 	struct reset_control *rst;
> >> -	struct clk *clk;
> >> +	struct clk_bulk_data *clks;
> >> 	int irq;
> >> 	int ret;
> >>
> >> @@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> >> 	if (IS_ERR(info->regs))
> >> 		return PTR_ERR(info->regs);
> >>
> >> -	clk = devm_clk_get_enabled(dev, NULL);
> >> -	if (IS_ERR(clk))
> >> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
> >> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> >> +	if (ret <= 0)  
> > 
> > Thank you Michal for the change.
> > 
> > Have you validated the changes ?
> > It looks while success ret would be 0 and it would give return error.  
> 
> But devm_clk_bulk_get_all_enabled() returns the number of clocks found 
> and enabled. And since we need at least one, I think this is correct, 
> and the error message below reflects that.

True but passing 0 to dev_err_probe() isn't going to do the right thing.

Though from this function, 0 is an error you need to return an error code
not 0 which to the caller looks like a success.



> 
> To me that change looks good:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre
> 
> 
> > 
> > Thanks, Sanjay
> > 
> >   
> >> +		return dev_err_probe(
> >> +			dev, ret,
> >> +			"failed to enable clocks or no clocks defined\n");
> >>
> >> 	rst = devm_reset_control_get_exclusive(dev, NULL);
> >> 	if (IS_ERR(rst))
> >> @@ -243,6 +245,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> >>
> >> static const struct of_device_id sun20i_gpadc_of_id[] = {
> >> 	{ .compatible = "allwinner,sun20i-d1-gpadc" },
> >> +	{ .compatible = "allwinner,sun55i-a523-gpadc" },
> >> 	{ }
> >> };
> >> MODULE_DEVICE_TABLE(of, sun20i_gpadc_of_id);
> >>  
> >   
> 

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andre Przywara]

On Wed, 13 May 2026 17:16:38 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

Hi,

> On Wed, 13 May 2026 13:53:49 +0200
> Andre Przywara <andre.przywara@arm.com> wrote:
> 
> > Hi Sanjay,
> > 
> > thanks for having a look!
> > 
> > On 5/13/26 13:44, Sanjay Chitroda wrote:  
> > > 
> > > 
> > > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:    
> > >> A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
> > >> clock and module clock.
> > >>
> > >> Change driver to enable all clocks.
> > >>
> > >> Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> > >> ---
> > >> drivers/iio/adc/sun20i-gpadc-iio.c | 11 +++++++----
> > >> 1 file changed, 7 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
> > >> index 861c14da75ad..3f1f07b3a385 100644
> > >> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
> > >> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
> > >> @@ -180,7 +180,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> > >> 	struct iio_dev *indio_dev;
> > >> 	struct sun20i_gpadc_iio *info;
> > >> 	struct reset_control *rst;
> > >> -	struct clk *clk;
> > >> +	struct clk_bulk_data *clks;
> > >> 	int irq;
> > >> 	int ret;
> > >>
> > >> @@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> > >> 	if (IS_ERR(info->regs))
> > >> 		return PTR_ERR(info->regs);
> > >>
> > >> -	clk = devm_clk_get_enabled(dev, NULL);
> > >> -	if (IS_ERR(clk))
> > >> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
> > >> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> > >> +	if (ret <= 0)    
> > > 
> > > Thank you Michal for the change.
> > > 
> > > Have you validated the changes ?
> > > It looks while success ret would be 0 and it would give return error.    
> > 
> > But devm_clk_bulk_get_all_enabled() returns the number of clocks found 
> > and enabled. And since we need at least one, I think this is correct, 
> > and the error message below reflects that.  
> 
> True but passing 0 to dev_err_probe() isn't going to do the right thing.
> 
> Though from this function, 0 is an error you need to return an error code
> not 0 which to the caller looks like a success.

Ah, that's true - should have read your email first before answering to
Andy ;-)

So yeah, that needs a split handling, for == 0, and for < 0.

Cheers,
Andre

> 
> 
> 
> > 
> > To me that change looks good:
> > 
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > Cheers,
> > Andre
> > 
> >   
> > > 
> > > Thanks, Sanjay
> > > 
> > >     
> > >> +		return dev_err_probe(
> > >> +			dev, ret,
> > >> +			"failed to enable clocks or no clocks defined\n");
> > >>
> > >> 	rst = devm_reset_control_get_exclusive(dev, NULL);
> > >> 	if (IS_ERR(rst))
> > >> @@ -243,6 +245,7 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
> > >>
> > >> static const struct of_device_id sun20i_gpadc_of_id[] = {
> > >> 	{ .compatible = "allwinner,sun20i-d1-gpadc" },
> > >> +	{ .compatible = "allwinner,sun55i-a523-gpadc" },
> > >> 	{ }
> > >> };
> > >> MODULE_DEVICE_TABLE(of, sun20i_gpadc_of_id);
> > >>    
> > >     
> >   
> 
> 

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andy Shevchenko]

On Wed, May 13, 2026 at 01:53:49PM +0200, Andre Przywara wrote:
> On 5/13/26 13:44, Sanjay Chitroda wrote:
> > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:

> > > +	if (ret <= 0)
> > 
> > Thank you Michal for the change.
> > 
> > Have you validated the changes ?
> > It looks while success ret would be 0 and it would give return error.

Good catch!

> But devm_clk_bulk_get_all_enabled() returns the number of clocks found and
> enabled. And since we need at least one, I think this is correct, and the
> error message below reflects that.
> 
> To me that change looks good:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

== 0 ???
Doesn't look like correct code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andre Przywara]

On Wed, 13 May 2026 23:12:05 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

Hi Andy,

> On Wed, May 13, 2026 at 01:53:49PM +0200, Andre Przywara wrote:
> > On 5/13/26 13:44, Sanjay Chitroda wrote:  
> > > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:  
> 
> > > > +	if (ret <= 0)  
> > > 
> > > Thank you Michal for the change.
> > > 
> > > Have you validated the changes ?
> > > It looks while success ret would be 0 and it would give return error.

No, it doesn't. Returning 0 means no clocks found:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-devres.c#n300

> 
> Good catch!
> 
> > But devm_clk_bulk_get_all_enabled() returns the number of clocks found and
> > enabled. And since we need at least one, I think this is correct, and the
> > error message below reflects that.
> > 
> > To me that change looks good:
> > 
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>  
> 
> == 0 ???
> Doesn't look like correct code.

Not sure I follow:
devm_clk_bulk_get_all_enabled() returns the number of clocks in that
node, or a negative error value. If it returns 0, that means no clocks
have been found, which is an error in our case, since we expect at
least one clock. This is what the second part of the error message
refers to.
So we want one or two as the return value, with the current bindings,
but really anything greater than 0 is fine, from the driver's
perspective, since we don't care about the clocks beyond them being
enabled.

So am I missing something?

Cheers,
Andre

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andy Shevchenko]

On Wed, May 13, 2026 at 11:19:01PM +0200, Andre Przywara wrote:
> On Wed, 13 May 2026 23:12:05 +0300
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Wed, May 13, 2026 at 01:53:49PM +0200, Andre Przywara wrote:
> > > On 5/13/26 13:44, Sanjay Chitroda wrote:  
> > > > On 13 May 2026 10:29:43 am IST, Michal Piekos <michal.piekos@mmpsystems.pl> wrote:  

...

> > > > > +	if (ret <= 0)  
> > > > 
> > > > Thank you Michal for the change.
> > > > 
> > > > Have you validated the changes ?
> > > > It looks while success ret would be 0 and it would give return error.
> 
> No, it doesn't. Returning 0 means no clocks found:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-devres.c#n300
> 
> > Good catch!
> > 
> > > But devm_clk_bulk_get_all_enabled() returns the number of clocks found and
> > > enabled. And since we need at least one, I think this is correct, and the
> > > error message below reflects that.
> > > 
> > > To me that change looks good:
> > > 
> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com>  
> > 
> > == 0 ???
> > Doesn't look like correct code.
> 
> Not sure I follow:
> devm_clk_bulk_get_all_enabled() returns the number of clocks in that
> node, or a negative error value. If it returns 0, that means no clocks
> have been found,

Not in this code. Here it will be resent to the caller as success.

> which is an error in our case, since we expect at
> least one clock. This is what the second part of the error message
> refers to.

But not the error code itself! There will be no error message, IIRC the
implementation of dev_err_probe().

> So we want one or two as the return value, with the current bindings,
> but really anything greater than 0 is fine, from the driver's
> perspective, since we don't care about the clocks beyond them being
> enabled.
> 
> So am I missing something?

Yes!

You returned that to the caller, meaning everything is fine. There is a success
that is returned. The code is buggy (okay, not that, it rather will behave not
as intended).

TL;DR:
You should have something like
	if (ret < 0)
		return dev_err_probe(ret);
	if (ret == 0)
		return dev_err_probe(-Exxx, "Needs at least one clock!\n");

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[Andy Shevchenko]

On Wed, May 13, 2026 at 06:59:43AM +0200, Michal Piekos wrote:
> A523 differs from existing sun20i-gpadc-iio by having two clocks; bus
> clock and module clock.
> 
> Change driver to enable all clocks.

...

>  	struct iio_dev *indio_dev;
>  	struct sun20i_gpadc_iio *info;
>  	struct reset_control *rst;
> -	struct clk *clk;
> +	struct clk_bulk_data *clks;

Try to follow reversed xmas tree order.

>  	int irq;
>  	int ret;

...

> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> +	if (ret <= 0)
> +		return dev_err_probe(
> +			dev, ret,
> +			"failed to enable clocks or no clocks defined\n");

Is this done by clang-format or so? Please, don't do wrapping on the open
parenthesis. Also note for more than 10 years checkpatch does not complain
on the trailing string literals that go over 80 characters.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] If `devm_clk_bulk_get_all_enabled()` returns 0 (meaning no clocks are defined in the device tree), the driver aborts the probe early but incorrectly returns 0 (success).
--

commit 6e1c58e982371b9be677bdb51d74117b3577cd6f
Author: Michal Piekos <michal.piekos@mmpsystems.pl>

iio: adc: sun20i-gpadc: add A523 gpadc support

This commit adds support for the A523 gpadc by switching to the bulk clock
API to handle both bus and module clocks.

> diff --git a/drivers/iio/adc/sun20i-gpadc-iio.c b/drivers/iio/adc/sun20i-gpadc-iio.c
> index 861c14da75add..3f1f07b3a3856 100644
> --- a/drivers/iio/adc/sun20i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun20i-gpadc-iio.c
[ ... ]
> @@ -205,9 +205,11 @@ static int sun20i_gpadc_probe(struct platform_device *pdev)
>  	if (IS_ERR(info->regs))
>  		return PTR_ERR(info->regs);
>  
> -	clk = devm_clk_get_enabled(dev, NULL);
> -	if (IS_ERR(clk))
> -		return dev_err_probe(dev, PTR_ERR(clk), "failed to enable bus clock\n");
> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> +	if (ret <= 0)
> +		return dev_err_probe(
> +			dev, ret,
> +			"failed to enable clocks or no clocks defined\n");

When devm_clk_bulk_get_all_enabled() returns 0 (meaning no clocks are defined),
will this code return 0 from the probe function?

Since dev_err_probe() returns the error code passed to it, returning 0 here
would cause sun20i_gpadc_probe() to succeed while skipping all remaining
initialization like reset deassertion, IRQ registration, and IIO device
registration.

Should ret be explicitly set to a negative error code (like -ENODEV) before
calling dev_err_probe() if ret is 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-sunxi-a523-gpadc-v2-0-d5efde151dac@mmpsystems.pl?part=2

[PATCH v2 3/3] arm64: dts: allwinner: a523: add gpadc node

[Michal Piekos]

Describe GPADC block on Allwinner A523.

Tested on Radxa Cubie A5E:
- 2 connected channels are showing voltages in agreement with
  schematics.
        BOOT-SEL-ADC ~500mV
        BOM-ADC ~1800mV
- 3rd channel exposed on 40pin header is showing correct voltages when
  connected to known voltage source.

Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
 arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
index 5afa8d92acbf..4e793aeccf89 100644
--- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
@@ -340,6 +340,17 @@ ledc: led-controller@2008000 {
 			status = "disabled";
 		};
 
+		gpadc: adc@2009000 {
+			compatible = "allwinner,sun55i-a523-gpadc";
+			reg = <0x2009000 0x400>;
+			clocks = <&ccu CLK_BUS_GPADC0>, <&ccu CLK_GPADC0>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_GPADC0>;
+			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
+			#io-channel-cells = <1>;
+			status = "disabled";
+		};
+
 		wdt: watchdog@2050000 {
 			compatible = "allwinner,sun55i-a523-wdt";
 			reg = <0x2050000 0x20>;

-- 
2.43.0

Re: [PATCH v2 3/3] arm64: dts: allwinner: a523: add gpadc node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The GPADC node lacks the `#address-cells` and `#size-cells` properties, which are structurally necessary for its required child nodes.
--

commit df07d88a5b17a24a46e46a7d0f2fce1a78cc0218
Author: Michal Piekos <michal.piekos@mmpsystems.pl>

arm64: dts: allwinner: a523: add gpadc node

This commit adds the GPADC block description for the Allwinner A523 SoC to
the base device tree include file.

> diff --git a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> --- a/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun55i-a523.dtsi

[ ... ]

> +		gpadc: adc@2009000 {
> +			compatible = "allwinner,sun55i-a523-gpadc";
> +			reg = <0x2009000 0x400>;
> +			clocks = <&ccu CLK_BUS_GPADC0>, <&ccu CLK_GPADC0>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_GPADC0>;
> +			interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> +			#io-channel-cells = <1>;

Should this node also include #address-cells = <1>; and #size-cells = <0>;?

The driver for this hardware requires channel@X child nodes to probe
successfully. Since these child nodes use reg properties to specify the
channel, omitting the cell properties from the base dtsi forces every board
file to redundantly declare them.

Including them here would prevent DTC warnings or probe failures if a board
author forgets to add them when enabling the device.

> +			status = "disabled";
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513-sunxi-a523-gpadc-v2-0-d5efde151dac@mmpsystems.pl?part=3