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

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Sun, Dec 5, 2021 at 5:02 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Dec 4, 2021 at 7:07 PM Jonathan Cameron <jic23@kernel.org> wrote:

...

> > +       const struct ad5755_platform_data *pdata = NULL;
> >         struct iio_dev *indio_dev;
> >         struct ad5755_state *st;
> >         int ret;
>
> > -       if (spi->dev.of_node)
> > -               pdata = ad5755_parse_dt(&spi->dev);
> > -       else
> > -               pdata = spi->dev.platform_data;
> > +       if (dev_fwnode(&spi->dev))
> > +               pdata = ad5755_parse_fw(&spi->dev);
> >
> >         if (!pdata) {
> > -               dev_warn(&spi->dev, "no platform data? using default\n");
> > +               dev_warn(&spi->dev,
> > +                        "no firmware provided parameters? using default\n");
>
> It's fine to have it on one line (and not related to the 80 vs 100
> case, it's about string literal as the last argument).
>
> >                 pdata = &ad5755_default_pdata;
> >         }
>
>
> Perhaps
>
>     const struct ad5755_platform_data *pdata;
>     ...
>     if (dev_fwnode(...))
>       pdata = ...
>     else
>       pdata = &_default;
>     if (pdata == &_default)
>       dev_warn(...);
>
> ?

After reading it again, I realized that pdata may be NULL in fwnode
case. So, original code is fine, but I would rather move NULL
assignment closer to the conditional (up to you, I'm fine with either,
i.o.w. you may ignore this comment).

-- 
With Best Regards,
Andy Shevchenko