| Message ID | 20220531141118.64540-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v1,1/2] iio: adc: xilinx-xadc: Make use of device properties | expand |
On Tue, 31 May 2022 17:11:17 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Convert the module to be property provider agnostic and allow > it to be used on non-OF platforms. > > Add mod_devicetable.h include. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> These both seem good to me, but as the driver is fairly actively maintained, I'll let this one sit on the list for a while so others can take a look. Whilst it 'seems' unlikely anyone will ever use this driver with other firmware I am keen to reduce the number of of-specific drivers in IIO just to avoid any chance of cut and paste. Who knows, I'm trying to run an aspeed-i2c driver with ACPI at the moment because it's handy for a emulated setup, so maybe the same will one day happen with this device :) Weirder things have happened. Jonathan > --- > drivers/iio/adc/xilinx-xadc-core.c | 39 ++++++++++++------------------ > 1 file changed, 16 insertions(+), 23 deletions(-) > > diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c > index 823c8e5f9809..e883f95f0cda 100644 > --- a/drivers/iio/adc/xilinx-xadc-core.c > +++ b/drivers/iio/adc/xilinx-xadc-core.c > @@ -17,10 +17,11 @@ > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/kernel.h> > +#include <linux/mod_devicetable.h> > #include <linux/module.h> > -#include <linux/of.h> > #include <linux/overflow.h> > #include <linux/platform_device.h> > +#include <linux/property.h> > #include <linux/slab.h> > #include <linux/sysfs.h> > > @@ -1182,14 +1183,13 @@ static const struct of_device_id xadc_of_match_table[] = { > }; > MODULE_DEVICE_TABLE(of, xadc_of_match_table); > > -static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > - unsigned int *conf, int irq) > +static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq) > { > struct device *dev = indio_dev->dev.parent; > struct xadc *xadc = iio_priv(indio_dev); > const struct iio_chan_spec *channel_templates; > struct iio_chan_spec *channels, *chan; > - struct device_node *chan_node, *child; > + struct fwnode_handle *chan_node, *child; > unsigned int max_channels; > unsigned int num_channels; > const char *external_mux; > @@ -1200,7 +1200,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > > *conf = 0; > > - ret = of_property_read_string(np, "xlnx,external-mux", &external_mux); > + ret = device_property_read_string(dev, "xlnx,external-mux", &external_mux); > if (ret < 0 || strcasecmp(external_mux, "none") == 0) > xadc->external_mux_mode = XADC_EXTERNAL_MUX_NONE; > else if (strcasecmp(external_mux, "single") == 0) > @@ -1211,8 +1211,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > return -EINVAL; > > if (xadc->external_mux_mode != XADC_EXTERNAL_MUX_NONE) { > - ret = of_property_read_u32(np, "xlnx,external-mux-channel", > - &ext_mux_chan); > + ret = device_property_read_u32(dev, "xlnx,external-mux-channel", &ext_mux_chan); > if (ret < 0) > return ret; > > @@ -1247,19 +1246,19 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > num_channels = 9; > chan = &channels[9]; > > - chan_node = of_get_child_by_name(np, "xlnx,channels"); > + chan_node = device_get_named_child_node(dev, "xlnx,channels"); > if (chan_node) { > - for_each_child_of_node(chan_node, child) { > + fwnode_for_each_child_node(chan_node, child) { > if (num_channels >= max_channels) { > - of_node_put(child); > + fwnode_handle_put(child); > break; > } > > - ret = of_property_read_u32(child, "reg", ®); > + ret = fwnode_property_read_u32(child, "reg", ®); > if (ret || reg > 16) > continue; > > - if (of_property_read_bool(child, "xlnx,bipolar")) > + if (fwnode_property_read_bool(child, "xlnx,bipolar")) > chan->scan_type.sign = 's'; > > if (reg == 0) { > @@ -1273,7 +1272,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, > chan++; > } > } > - of_node_put(chan_node); > + fwnode_handle_put(chan_node); > > /* No IRQ => no events */ > if (irq <= 0) { > @@ -1316,7 +1315,6 @@ static void xadc_cancel_delayed_work(void *data) > static int xadc_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > - const struct of_device_id *id; > const struct xadc_ops *ops; > struct iio_dev *indio_dev; > unsigned int bipolar_mask; > @@ -1326,15 +1324,10 @@ static int xadc_probe(struct platform_device *pdev) > int irq; > int i; > > - if (!dev->of_node) > - return -ENODEV; > - > - id = of_match_node(xadc_of_match_table, dev->of_node); > - if (!id) > + ops = device_get_match_data(dev); > + if (!ops) > return -EINVAL; > > - ops = id->data; > - > irq = platform_get_irq_optional(pdev, 0); > if (irq < 0 && > (irq != -ENXIO || !(ops->flags & XADC_FLAGS_IRQ_OPTIONAL))) > @@ -1345,7 +1338,7 @@ static int xadc_probe(struct platform_device *pdev) > return -ENOMEM; > > xadc = iio_priv(indio_dev); > - xadc->ops = id->data; > + xadc->ops = ops; > init_completion(&xadc->completion); > mutex_init(&xadc->mutex); > spin_lock_init(&xadc->lock); > @@ -1359,7 +1352,7 @@ static int xadc_probe(struct platform_device *pdev) > indio_dev->modes = INDIO_DIRECT_MODE; > indio_dev->info = &xadc_info; > > - ret = xadc_parse_dt(indio_dev, dev->of_node, &conf0, irq); > + ret = xadc_parse_dt(indio_dev, &conf0, irq); > if (ret) > return ret; >
On 6/3/22 19:32, Jonathan Cameron wrote: > On Tue, 31 May 2022 17:11:17 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > >> Convert the module to be property provider agnostic and allow >> it to be used on non-OF platforms. >> >> Add mod_devicetable.h include. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > These both seem good to me, but as the driver is fairly actively maintained, > I'll let this one sit on the list for a while so others can take a look. > > Whilst it 'seems' unlikely anyone will ever use this driver with other firmware > I am keen to reduce the number of of-specific drivers in IIO just to avoid > any chance of cut and paste. > > Who knows, I'm trying to run an aspeed-i2c driver with ACPI at the moment > because it's handy for a emulated setup, so maybe the same will one day happen > with this device :) Weirder things have happened. Conall: Can you please test these changes and provide your feedback? Thanks, Michal
On Mon, Jun 20, 2022 at 04:58:04PM +0200, Michal Simek wrote: > On 6/3/22 19:32, Jonathan Cameron wrote: > > On Tue, 31 May 2022 17:11:17 +0300 > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > > > > Convert the module to be property provider agnostic and allow > > > it to be used on non-OF platforms. > > > > > > Add mod_devicetable.h include. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > These both seem good to me, but as the driver is fairly actively maintained, > > I'll let this one sit on the list for a while so others can take a look. > > > > Whilst it 'seems' unlikely anyone will ever use this driver with other firmware > > I am keen to reduce the number of of-specific drivers in IIO just to avoid > > any chance of cut and paste. > > > > Who knows, I'm trying to run an aspeed-i2c driver with ACPI at the moment > > because it's handy for a emulated setup, so maybe the same will one day happen > > with this device :) Weirder things have happened. > > Conall: Can you please test these changes and provide your feedback? Hmm... No news?
On 6/3/22 19:32, Jonathan Cameron wrote: > On Tue, 31 May 2022 17:11:17 +0300 > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > >> Convert the module to be property provider agnostic and allow >> it to be used on non-OF platforms. >> >> Add mod_devicetable.h include. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > These both seem good to me, but as the driver is fairly actively maintained, > I'll let this one sit on the list for a while so others can take a look. > > Whilst it 'seems' unlikely anyone will ever use this driver with other firmware > I am keen to reduce the number of of-specific drivers in IIO just to avoid > any chance of cut and paste. I actually have this device on a PCIe card where I instantiate it using mfd with device properties. But no external channels so there was no need to convert that part to device properties.
Hi Andy, On 7/2/22 16:39, Andy Shevchenko wrote: > On Mon, Jun 20, 2022 at 04:58:04PM +0200, Michal Simek wrote: >> On 6/3/22 19:32, Jonathan Cameron wrote: >>> On Tue, 31 May 2022 17:11:17 +0300 >>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: >>> >>>> Convert the module to be property provider agnostic and allow >>>> it to be used on non-OF platforms. >>>> >>>> Add mod_devicetable.h include. >>>> >>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> >>> These both seem good to me, but as the driver is fairly actively maintained, >>> I'll let this one sit on the list for a while so others can take a look. >>> >>> Whilst it 'seems' unlikely anyone will ever use this driver with other firmware >>> I am keen to reduce the number of of-specific drivers in IIO just to avoid >>> any chance of cut and paste. >>> >>> Who knows, I'm trying to run an aspeed-i2c driver with ACPI at the moment >>> because it's handy for a emulated setup, so maybe the same will one day happen >>> with this device :) Weirder things have happened. >> >> Conall: Can you please test these changes and provide your feedback? > > Hmm... No news? Anand unfortunately left the company. I have asked testing team to test this patch and they can't see any issue. That's why: Acked-by: Michal Simek <michal.simek@amd.com> Thanks, Michal
On Mon, Jul 04, 2022 at 12:43:26PM +0200, Michal Simek wrote: > On 7/2/22 16:39, Andy Shevchenko wrote: > > On Mon, Jun 20, 2022 at 04:58:04PM +0200, Michal Simek wrote: ... > > Hmm... No news? > > Anand unfortunately left the company. I have asked testing team to test this > patch and they can't see any issue. > That's why: > Acked-by: Michal Simek <michal.simek@amd.com> Ah, thanks! Jonathan, I guess we are set to apply now.
On Mon, 4 Jul 2022 19:18:05 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, Jul 04, 2022 at 12:43:26PM +0200, Michal Simek wrote: > > On 7/2/22 16:39, Andy Shevchenko wrote: > > > On Mon, Jun 20, 2022 at 04:58:04PM +0200, Michal Simek wrote: > > ... > > > > Hmm... No news? > > > > Anand unfortunately left the company. I have asked testing team to test this > > patch and they can't see any issue. > > That's why: > > Acked-by: Michal Simek <michal.simek@amd.com> > > Ah, thanks! Jonathan, I guess we are set to apply now. > Applied to the togreg branch of iio.git and pushed out as testing for all the normal reasons. Thanks, Jonathan
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c index 823c8e5f9809..e883f95f0cda 100644 --- a/drivers/iio/adc/xilinx-xadc-core.c +++ b/drivers/iio/adc/xilinx-xadc-core.c @@ -17,10 +17,11 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/kernel.h> +#include <linux/mod_devicetable.h> #include <linux/module.h> -#include <linux/of.h> #include <linux/overflow.h> #include <linux/platform_device.h> +#include <linux/property.h> #include <linux/slab.h> #include <linux/sysfs.h> @@ -1182,14 +1183,13 @@ static const struct of_device_id xadc_of_match_table[] = { }; MODULE_DEVICE_TABLE(of, xadc_of_match_table); -static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, - unsigned int *conf, int irq) +static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq) { struct device *dev = indio_dev->dev.parent; struct xadc *xadc = iio_priv(indio_dev); const struct iio_chan_spec *channel_templates; struct iio_chan_spec *channels, *chan; - struct device_node *chan_node, *child; + struct fwnode_handle *chan_node, *child; unsigned int max_channels; unsigned int num_channels; const char *external_mux; @@ -1200,7 +1200,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, *conf = 0; - ret = of_property_read_string(np, "xlnx,external-mux", &external_mux); + ret = device_property_read_string(dev, "xlnx,external-mux", &external_mux); if (ret < 0 || strcasecmp(external_mux, "none") == 0) xadc->external_mux_mode = XADC_EXTERNAL_MUX_NONE; else if (strcasecmp(external_mux, "single") == 0) @@ -1211,8 +1211,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, return -EINVAL; if (xadc->external_mux_mode != XADC_EXTERNAL_MUX_NONE) { - ret = of_property_read_u32(np, "xlnx,external-mux-channel", - &ext_mux_chan); + ret = device_property_read_u32(dev, "xlnx,external-mux-channel", &ext_mux_chan); if (ret < 0) return ret; @@ -1247,19 +1246,19 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, num_channels = 9; chan = &channels[9]; - chan_node = of_get_child_by_name(np, "xlnx,channels"); + chan_node = device_get_named_child_node(dev, "xlnx,channels"); if (chan_node) { - for_each_child_of_node(chan_node, child) { + fwnode_for_each_child_node(chan_node, child) { if (num_channels >= max_channels) { - of_node_put(child); + fwnode_handle_put(child); break; } - ret = of_property_read_u32(child, "reg", ®); + ret = fwnode_property_read_u32(child, "reg", ®); if (ret || reg > 16) continue; - if (of_property_read_bool(child, "xlnx,bipolar")) + if (fwnode_property_read_bool(child, "xlnx,bipolar")) chan->scan_type.sign = 's'; if (reg == 0) { @@ -1273,7 +1272,7 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, chan++; } } - of_node_put(chan_node); + fwnode_handle_put(chan_node); /* No IRQ => no events */ if (irq <= 0) { @@ -1316,7 +1315,6 @@ static void xadc_cancel_delayed_work(void *data) static int xadc_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; - const struct of_device_id *id; const struct xadc_ops *ops; struct iio_dev *indio_dev; unsigned int bipolar_mask; @@ -1326,15 +1324,10 @@ static int xadc_probe(struct platform_device *pdev) int irq; int i; - if (!dev->of_node) - return -ENODEV; - - id = of_match_node(xadc_of_match_table, dev->of_node); - if (!id) + ops = device_get_match_data(dev); + if (!ops) return -EINVAL; - ops = id->data; - irq = platform_get_irq_optional(pdev, 0); if (irq < 0 && (irq != -ENXIO || !(ops->flags & XADC_FLAGS_IRQ_OPTIONAL))) @@ -1345,7 +1338,7 @@ static int xadc_probe(struct platform_device *pdev) return -ENOMEM; xadc = iio_priv(indio_dev); - xadc->ops = id->data; + xadc->ops = ops; init_completion(&xadc->completion); mutex_init(&xadc->mutex); spin_lock_init(&xadc->lock); @@ -1359,7 +1352,7 @@ static int xadc_probe(struct platform_device *pdev) indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->info = &xadc_info; - ret = xadc_parse_dt(indio_dev, dev->of_node, &conf0, irq); + ret = xadc_parse_dt(indio_dev, &conf0, irq); if (ret) return ret;
Convert the module to be property provider agnostic and allow it to be used on non-OF platforms. Add mod_devicetable.h include. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/iio/adc/xilinx-xadc-core.c | 39 ++++++++++++------------------ 1 file changed, 16 insertions(+), 23 deletions(-)