Re: [RFC PATCH 5/6] iio: inkern: move to fwnode properties

["Nuno Sá" <noname.nuno@gmail.com>]

On Fri, 2022-06-03 at 13:52 +0200, Andy Shevchenko wrote:
> On Thu, Jun 2, 2022 at 4:04 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > 
> > This moves the IIO in kernel interface to use fwnode properties and
> > thus
> > be firmware agnostic.
> > 
> > All the users had to be naturally updated to the new interface
> > exposed by
> > IIO.
> 
> I think you may split this in an easy way, i.e. convert core to
> fwnode, while providing inliners for of_node cases (like it's done in
> IRQ domain) and then remove them after conversion.
> 

I see, in our case that might be really simple as we only have one user
of devm_of_iio_channel_get_by_name() which is the only api directly
using OF. of_iio_channel_get_by_name() has no users and all the other
public APIs use 'struct device' so we can do the conversion
internally...

> I think of_xlate is not needed to be touched at all. Let it die with
> OF altogether. Yes, it won't be fully OF-independent, but it will
> down
> the scope of the next change where you can convert of_xlate to
> something agnostic.
> 
> ...
> 
> > --- a/drivers/iio/adc/ab8500-gpadc.c
> > +++ b/drivers/iio/adc/ab8500-gpadc.c
> > @@ -39,6 +39,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/mfd/abx500.h>
> >  #include <linux/mfd/abx500/ab8500.h>
> > +#include <linux/fwnode.h>
> 
> Ordering.
> 

The ordering is completely wrong anyways. So, I did not cared about
ordering in drivers where it was already bad. Don't mind to fix it
while adding the missing headers (if acceptable).

> ...
> 
> > --- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> > +++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/init.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/fwnode.h>
> 
> Ordering?
> 
> ...
> 
> >   * @consumer_channel:  Unique name to identify the channel on the
> > consumer
> >   *                     side. This typically describes the channels
> > use within
> 
> used / usage ?
> 
> ...
> 
> >   * @consumer_channel:  Unique name to identify the channel on the
> > consumer
> >   *                     side. This typically describes the channels
> > use within
> 
> Ditto.
> 
> >   *                     the consumer. E.g. 'battery_voltage'
> 
> ...
> 
> > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > index 233d2e6b7721..18ca5a7cb154 100644
> > --- a/include/linux/iio/iio.h
> > +++ b/include/linux/iio/iio.h
> > @@ -10,13 +10,14 @@
> >  #include <linux/device.h>
> >  #include <linux/cdev.h>
> >  #include <linux/iio/types.h>
> > -#include <linux/of.h>
> 
> You may split this change easily since there is nothing from of.h in
> use. Just add forward declaration as you have done, but for the OF
> case.
> 
> ...
> 
> That said, I think what you need is to split this series to three
> logical parts:
> 1) shuffle header inclusions around so, iio.h will use forward
> declaration (on driver basis);
> 2) convert inkern.c to fwnode while providing OF wrappers
> (to_of_node() helps);

Just to be clear, we should still add an fwnode_xlate() callback? So we
have both temporarily and if some new driver needs this interface it
can already use it instead of of_xlate...

> 3) convert of_xlate (on driver basis it might be tricky, up to you).
> 

Yeah, I might see how easy it is to fully convert the drivers using 
of_xlate. If easy enough, I'll probably do it...

- Nuno Sá