Re: [PATCH v2 4/5] iio: adc: ti-adc161s626: log buffer setup failure in probe
From: Andy Shevchenko
Date: Thu Jun 25 2026 - 10:03:28 EST
On Thu, Jun 25, 2026 at 12:09:52PM +0100, Jonathan Cameron wrote:
> On Wed, 24 Jun 2026 14:26:52 +0300
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> > On Wed, Jun 24, 2026 at 03:43:44PM +0530, Prashant Rahul wrote:
> > > On Wed Jun 24, 2026 at 1:06 AM IST, Jonathan Cameron wrote:
...
> > > > Why this particular one but not the devm_iio_device_register() below it?
> > > > Both pretty much only fail on driver bugs. Or out of memory for
> > > > which dev_err_probe() doesn't print anything anyway (on basis that
> > > > is normally pretty noisy).
> > >
> > > My apologies, I wasnt aware that dev_err_probe simply ignores `-ENOMEM`.
> > > Looking at drivers/base/core.c:5096, it is pretty evident that it does.
> > >
> > > My goal with the patch series was to change dev_err into dev_err_probe,
> > > as my understanding of dev_err_probe was that it's a specialization of
> > > dev_err for probe contexts.
> >
> > Jonathan, actually the triggered buffer setup might return not -ENOMEM error
> > code for the surprise. After all this series does add not a dead code, but
> > a quite rare to be used one.
> >
> > https://elixir.bootlin.com/linux/v7.1.1/source/drivers/iio/buffer/industrialio-triggered-buffer.c#L57
>
> Agreed and I'm not even sure why that is there (lost to the mists of time
> / my failing memory ;) However I think it's an obvious driver bug case
> that should be trivial to see in basic tests so to me it's marginal on
> whether a print on these is useful.
>
> Maybe given that one is so rare we should just add a print in the core?
> I'm not particularly keen on mass patches to print errors from the core
> code but this one is perhaps a reasonable trade off vs series like this
> one adding prints to drivers for something we don't think realistically
> ever happens.
While the series ends up not adding a dead code, the rareness of the case
doesn't justify it, so I am still on the side of _not_ applying this series.
For the core printing, I think for devm_*() or other managed resource APIs
it makes a lot of sense (in general). Not sure about plain calls, though.
--
With Best Regards,
Andy Shevchenko