2022-06-01 20:26:20

by Andy Shevchenko

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

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 <[email protected]>
---
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;

--
2.35.1



2022-06-01 21:03:34

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/2] iio: adc: xilinx-xadc: Drop duplicate NULL check in xadc_parse_dt()

The fwnode_for_each_child_node() is NULL-aware, no need to check
its parameters outside. Drop duplicate NULL check in xadc_parse_dt().

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/iio/adc/xilinx-xadc-core.c | 38 ++++++++++++++----------------
1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index e883f95f0cda..1b247722ba25 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -1247,30 +1247,28 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, unsigned int *conf, int irq)
chan = &channels[9];

chan_node = device_get_named_child_node(dev, "xlnx,channels");
- if (chan_node) {
- fwnode_for_each_child_node(chan_node, child) {
- if (num_channels >= max_channels) {
- fwnode_handle_put(child);
- break;
- }
+ fwnode_for_each_child_node(chan_node, child) {
+ if (num_channels >= max_channels) {
+ fwnode_handle_put(child);
+ break;
+ }

- ret = fwnode_property_read_u32(child, "reg", &reg);
- if (ret || reg > 16)
- continue;
+ ret = fwnode_property_read_u32(child, "reg", &reg);
+ if (ret || reg > 16)
+ continue;

- if (fwnode_property_read_bool(child, "xlnx,bipolar"))
- chan->scan_type.sign = 's';
+ if (fwnode_property_read_bool(child, "xlnx,bipolar"))
+ chan->scan_type.sign = 's';

- if (reg == 0) {
- chan->scan_index = 11;
- chan->address = XADC_REG_VPVN;
- } else {
- chan->scan_index = 15 + reg;
- chan->address = XADC_REG_VAUX(reg - 1);
- }
- num_channels++;
- chan++;
+ if (reg == 0) {
+ chan->scan_index = 11;
+ chan->address = XADC_REG_VPVN;
+ } else {
+ chan->scan_index = 15 + reg;
+ chan->address = XADC_REG_VAUX(reg - 1);
}
+ num_channels++;
+ chan++;
}
fwnode_handle_put(chan_node);

--
2.35.1


2022-06-06 05:56:08

by Jonathan Cameron

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

On Tue, 31 May 2022 17:11:17 +0300
Andy Shevchenko <[email protected]> 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 <[email protected]>

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;
>

2022-06-20 15:29:13

by Michal Simek

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



On 6/3/22 19:32, Jonathan Cameron wrote:
> On Tue, 31 May 2022 17:11:17 +0300
> Andy Shevchenko <[email protected]> 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 <[email protected]>
>
> 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

2022-07-02 14:57:30

by Andy Shevchenko

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

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 <[email protected]> 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 <[email protected]>
> >
> > 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?

--
With Best Regards,
Andy Shevchenko


2022-07-02 14:58:45

by Lars-Peter Clausen

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

On 6/3/22 19:32, Jonathan Cameron wrote:
> On Tue, 31 May 2022 17:11:17 +0300
> Andy Shevchenko <[email protected]> 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 <[email protected]>
> 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.

2022-07-04 10:55:13

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] iio: adc: xilinx-xadc: Make use of 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 <[email protected]> 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 <[email protected]>
>>>
>>> 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 <[email protected]>

Thanks,
Michal

2022-07-04 17:03:01

by Andy Shevchenko

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

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 <[email protected]>

Ah, thanks! Jonathan, I guess we are set to apply now.

--
With Best Regards,
Andy Shevchenko


2022-07-13 16:26:23

by Jonathan Cameron

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

On Mon, 4 Jul 2022 19:18:05 +0300
Andy Shevchenko <[email protected]> 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 <[email protected]>
>
> 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