On Thu, 2023-12-21 at 17:52 +0000, Jonathan Cameron wrote:
> On Wed, 20 Dec 2023 16:34:10 +0100
> Nuno Sa <nuno.sa@xxxxxxxxxx> wrote:
>
> > Convert the driver to use the new IIO backend framework. The device
> > functionality is expected to be the same (meaning no added or removed
> > features).
> >
> > Also note this patch effectively breaks ABI and that's needed so we can
> > properly support this device and add needed features making use of the
> > new IIO framework.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
>
> A few minor comments. Looks good to me.
>
> Jonathan
>
> > +static int ad9467_update_scan_mode(struct iio_dev *indio_dev,
> > + const unsigned long *scan_mask)
> > +{
> > + struct ad9467_state *st = iio_priv(indio_dev);
> > + unsigned int c;
> > + int ret;
> > +
> > + for (c = 0; c < st->info->num_channels; c++) {
> > + if (test_bit(c, scan_mask))
> > + ret = iio_backend_chan_enable(st->back, c);
> > + else
> > + ret = iio_backend_chan_disable(st->back, c);
> > +
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
>
> > static int ad9467_reset(struct device *dev)
> > @@ -443,25 +475,63 @@ static int ad9467_reset(struct device *dev)
> > return 0;
> > }
> >
> > +static int ad9467_iio_backend_get(struct ad9467_state *st)
> > +{
> > + struct device *dev = &st->spi->dev;
> > + struct device_node *__back;
> > +
> > + st->back = devm_iio_backend_get_optional(&st->spi->dev, NULL);
> > + if (IS_ERR(st->back))
> > + return PTR_ERR(st->back);
>
> As per the comment on previous patch I'd just get it using the normal
> function and handle PTR_ERR(-ENOENT) here as meaning we need to
> try the old way.
That makes sense. So we can add a devm_iio_backend_get_optional() API when/if we
really need one and not just for handling legacy code.
>
> > + if (st->back)
> > + return 0;
> > + /*
> > + * if we don't get the backend using the normal API's, use the legacy
> > + * 'adi,adc-dev' property. So we get all nodes with that property, and
> > + * look for the one pointing at us. Then we directly lookup that fwnode
> > + * on the backend list of registered devices. This is done so we don't
> > + * make io-backends mandatory which would break DT ABI.
> > + */
> > + for_each_node_with_property(__back, "adi,adc-dev") {
> > + struct device_node *__me;
> > +
> > + __me = of_parse_phandle(__back, "adi,adc-dev", 0);
> > + if (!__me)
> > + continue;
> > +
> > + if (!device_match_of_node(dev, __me)) {
> > + of_node_put(__me);
> > + continue;
> > + }
> > +
> > + of_node_put(__me);
> > + st->back = devm_iio_backend_get_from_fwnode_lookup(dev,
> > +
> > of_fwnode_handle(__back));
> > + of_node_put(__back);
>
> If it lands first the patch
> RFC PATCH 1/4] of: Add cleanup.h based autorelease via __free(device_node)
> markings.
> will get rid of this manual handling for you for both the continue and return.
> This will make a very nice example for that :)
Yeah, and I'll happily rebase on that...
- Nuno Sá
>
