[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 --]

[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

[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