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

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

[Andy Shevchenko]

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(-)

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

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

[Andy Shevchenko]

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 <andriy.shevchenko@linux.intel.com>
---
 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

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

[Jonathan Cameron]

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", &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;
>  

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

[Michal Simek]

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

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

[Andy Shevchenko]

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? 

-- 
With Best Regards,
Andy Shevchenko

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

[Michal Simek]

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

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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

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

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

[Lars-Peter Clausen]

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.