[PATCH v2 2/3] iio: adc: sun20i-gpadc: add A523 gpadc support
Andy Shevchenko
andriy.shevchenko at intel.com
Wed May 13 14:34:32 PDT 2026
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 at 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 at 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 at 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
More information about the linux-arm-kernel
mailing list