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@xxxxxxxxxx> 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á
