Re: [PATCH v1 1/1] iio: adc: ad7191: Don't check for specific errors when parsing properties

From: Nuno Sá

Date: Tue Apr 14 2026 - 05:12:51 EST


On Sun, Apr 12, 2026 at 03:30:40PM +0100, Jonathan Cameron wrote:
> On Tue, 17 Mar 2026 12:42:07 +0200
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> > On Fri, Feb 20, 2026 at 12:33:28PM +0200, Andy Shevchenko wrote:
> > > On Fri, Feb 20, 2026 at 12:04 PM Nuno Sá <noname.nuno@xxxxxxxxx> wrote:
> > > > On Thu, 2026-02-19 at 15:39 +0100, Andy Shevchenko wrote:
> > > > > Instead of checking for the specific error codes (that can be considered
> > > > > a layering violation to some extent) check for the property existence first
> > > > > and then either parse it, or apply a default value.
> > > >
> > > > Not really sure how I feel about this one. Checking for specific errors is a very common
> > > > pattern and this change just makes it we check for the property presence twice. That said,
> > > > this makes it more "future proof" (though I find it very unlikely for ret value o change).
> > >
> > > I already have an answer to this:
> > > https://lore.kernel.org/r/aZcenabXYsOdBu84@xxxxxxxxxxxxxxxxxx
> >
> > Does it help?
> >
> > > > Anyways, even if we choose to go down this route, I don't see much benefit in starting
> > > > converting the drivers with the pattern below (which should be a considerable number).
> > >
> > > There is not a big number of them, so I prefer to have common patterns
> > > without exact error code checks.
> >
>
>
> I left this one for a while to see if the discussion would continue but seems not.
> I'm not sure it is always the case, but in this particular example I think the
> resulting code is a little nicer to read so applied.
>
> So for me, case by case basis for this sort of change.

Yeah, I missed this one! Anyways seems the way will be to explicitly
use device_property_present() to check property presence. Still don't
love the dual call thing so I might just stop checking for -EINVAL (was
not doing it religiously anyways).

Maybe we could have a set of optional variants of the API like
device_property_read_*_optional() kind of thing where we just return 0
if the property is not present. But might also be too noisy...

- Nuno Sá

>
> Thanks,
>
> Jonathan