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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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