Re: [PATCH v3 01/12] iio:dac:ad5755: Switch to generic firmware properties and drop pdata

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Sun, 5 Dec 2021 22:09:36 +0200
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Dec 5, 2021 at 6:56 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > Lars pointed out that platform data can also be supported via the
> > generic properties interface, so there is no point in continuing to
> > support it separately.  Hence squish the linux/platform_data/ad5755.h
> > header into the c file and drop accessing the platform data directly.
> >
> > Done by inspection only.  Mostly completely mechanical with the
> > exception of a few places where default value handling is
> > cleaner done by first setting the value, then calling the
> > firmware reading function but and not checking the return value,
> > as opposed to reading firmware then setting the default if an error
> > occurs.
> >
> > Part of general attempt to move all of IIO over to generic
> > device properties, both to enable other firmware types and
> > to remove drivers that can be the source of of_ specific
> > behaviour in new drivers.  
> 
> I was looking again into these enums thinking that it might be a good
> place for them in include/dr-bindings/ but after reading the schema I
> realized that they are rather encoded, while the schema is using
> decoded values. So, scratch this. But I have noticed one more thing
> (see below).
> 
> ...
> 
> >         devm_kfree(dev, pdata);
> >         return NULL;  
> 
> Sorry, haven't noticed this one, do we really need this devm_kfree() call?
> Shouldn't there be better error reporting then? (Note, it's just
> thoughts for further improvements).
>
I wondered about this.  The driver will currently successfully
probe with broken firmware and fall back to the defaults.

If we change that (and hence have this return an ERR_PTR() and fail
the probe) then we risk a regression if someone actually has a platform
with a firmware broken in this fashion.

The path that triggers this is when there are too many child nodes.

I'm tempted to say that supporting something that broken is beyond what
we should bother with and just drop this muddling on (as a follow up patch).
If anyone reports a regression as a result we can put it back again...

Jonathan