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

[Andre Przywara <andre.przywara@arm.com>]

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