Re: [PATCH v1 1/2] iio: adc: xilinx-xadc: Make use of device properties

From: Jonathan Cameron
Date: Fri Jun 03 2022 - 13:23:31 EST


On Tue, 31 May 2022 17:11:17 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>

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", &reg);
> + ret = fwnode_property_read_u32(child, "reg", &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;
>