[PATCH v2 00/15] make iio inkern interface firmware agnostic

[PATCH v2 00/15] make iio inkern interface firmware agnostic

[Nuno Sá]

First version of the series can be found here:

https://lore.kernel.org/linux-iio/20220610084545.547700-1-nuno.sa@analog.com/

v2 changes:

[1/15]
  * Fix typo and added more description in the commit message.

[3/15]
  * Remove superfluous code;
  * Commit message spell fixes and added more details;
  * Improved error handling (this is the most significant change in this
version. More details on the commit message).

[4/15]
  * Drop the 'ugly' parent_lookup flag. With the new error handling,
    we can use -ENODEV to infer if we should proceed or not with the
    lookup.

[5/15]:
  * Moved some local declarations up so long lines first;
  * Use 'bus_find_device_by_fwnode()';
  * Proper ordering in includes.
  * Adapted error handling in '__fwnode_iio_channel_get_by_name()' taking
ACPI into account and when 'name' is given but index < 0. It seems that
ACPI code can actually return -ENOENT with index < 0 for which case we
should continue the search. Not sure if a check  in ACPI ('if (index < 0)
return -EINVAL;) like is done in OF would make sense...

[12/15]:
  * Use 'device_property_count_u64()' to get the number of diff channels.
So no need for 'magic' divisions by 2 (no idea why I haven't done like
this in the first place).

[15/15]
  * Fix wrong conversion of 'if (ptr != NULL)' to 'if (!ptr)'.

Special note for patch 3/15 where -ENODEV is still used despite some talks
about using -ENOENT and hence, be more in line with firmware code. The
reason I kept it was to be consistent with the rest of the file. I'd say
that if we want to move to -ENOENT we should do it in a separate patch
and for the complete file.

Nuno Sá (15):
  iio: inkern: only release the device node when done with it
  iio: inkern: fix return value in devm_of_iio_channel_get_by_name()
  iio: inkern: only return error codes in iio_channel_get_*() APIs
  iio: inkern: split of_iio_channel_get_by_name()
  iio: inkern: move to fwnode properties
  thermal: qcom: qcom-spmi-adc-tm5: convert to IIO fwnode API
  iio: adc: ingenic-adc: convert to IIO fwnode interface
  iio: adc: ab8500-gpadc: convert to device properties
  iio: adc: at91-sama5d2_adc: convert to device properties
  iio: adc: qcom-pm8xxx-xoadc: convert to device properties
  iio: adc: qcom-spmi-vadc: convert to device properties
  iio: adc: qcom-spmi-adc5: convert to device properties
  iio: adc: stm32-adc: convert to device properties
  iio: inkern: remove OF dependencies
  iio: inkern: fix coding style warnings

 drivers/iio/adc/ab8500-gpadc.c           |  27 +--
 drivers/iio/adc/at91-sama5d2_adc.c       |  30 +--
 drivers/iio/adc/ingenic-adc.c            |   8 +-
 drivers/iio/adc/qcom-pm8xxx-xoadc.c      |  58 +++--
 drivers/iio/adc/qcom-spmi-adc5.c         |  63 +++---
 drivers/iio/adc/qcom-spmi-vadc.c         |  44 ++--
 drivers/iio/adc/stm32-adc.c              | 121 +++++-----
 drivers/iio/inkern.c                     | 271 +++++++++++++----------
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c |   3 +-
 include/linux/iio/consumer.h             |  28 +--
 include/linux/iio/iio.h                  |   8 +-
 11 files changed, 347 insertions(+), 314 deletions(-)

-- 
2.37.0

[PATCH v2 00/15] make iio inkern interface firmware agnostic

[Nuno Sá]

First version of the series can be found here:

https://lore.kernel.org/linux-iio/20220610084545.547700-1-nuno.sa@analog.com/

v2 changes:

[1/15]
  * Fix typo and added more description in the commit message.

[3/15]
  * Remove superfluous code;
  * Commit message spell fixes and added more details;
  * Improved error handling (this is the most significant change in this
version. More details on the commit message).

[4/15]
  * Drop the 'ugly' parent_lookup flag. With the new error handling,
    we can use -ENODEV to infer if we should proceed or not with the
    lookup.

[5/15]:
  * Moved some local declarations up so long lines first;
  * Use 'bus_find_device_by_fwnode()';
  * Proper ordering in includes.
  * Adapted error handling in '__fwnode_iio_channel_get_by_name()' taking
ACPI into account and when 'name' is given but index < 0. It seems that
ACPI code can actually return -ENOENT with index < 0 for which case we
should continue the search. Not sure if a check  in ACPI ('if (index < 0)
return -EINVAL;) like is done in OF would make sense...

[12/15]:
  * Use 'device_property_count_u64()' to get the number of diff channels.
So no need for 'magic' divisions by 2 (no idea why I haven't done like
this in the first place).

[15/15]
  * Fix wrong conversion of 'if (ptr != NULL)' to 'if (!ptr)'.

Special note for patch 3/15 where -ENODEV is still used despite some talks
about using -ENOENT and hence, be more in line with firmware code. The
reason I kept it was to be consistent with the rest of the file. I'd say
that if we want to move to -ENOENT we should do it in a separate patch
and for the complete file.

Nuno Sá (15):
  iio: inkern: only release the device node when done with it
  iio: inkern: fix return value in devm_of_iio_channel_get_by_name()
  iio: inkern: only return error codes in iio_channel_get_*() APIs
  iio: inkern: split of_iio_channel_get_by_name()
  iio: inkern: move to fwnode properties
  thermal: qcom: qcom-spmi-adc-tm5: convert to IIO fwnode API
  iio: adc: ingenic-adc: convert to IIO fwnode interface
  iio: adc: ab8500-gpadc: convert to device properties
  iio: adc: at91-sama5d2_adc: convert to device properties
  iio: adc: qcom-pm8xxx-xoadc: convert to device properties
  iio: adc: qcom-spmi-vadc: convert to device properties
  iio: adc: qcom-spmi-adc5: convert to device properties
  iio: adc: stm32-adc: convert to device properties
  iio: inkern: remove OF dependencies
  iio: inkern: fix coding style warnings

 drivers/iio/adc/ab8500-gpadc.c           |  27 +--
 drivers/iio/adc/at91-sama5d2_adc.c       |  30 +--
 drivers/iio/adc/ingenic-adc.c            |   8 +-
 drivers/iio/adc/qcom-pm8xxx-xoadc.c      |  58 +++--
 drivers/iio/adc/qcom-spmi-adc5.c         |  63 +++---
 drivers/iio/adc/qcom-spmi-vadc.c         |  44 ++--
 drivers/iio/adc/stm32-adc.c              | 121 +++++-----
 drivers/iio/inkern.c                     | 271 +++++++++++++----------
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c |   3 +-
 include/linux/iio/consumer.h             |  28 +--
 include/linux/iio/iio.h                  |   8 +-
 11 files changed, 347 insertions(+), 314 deletions(-)

-- 
2.37.0

[PATCH v2 01/15] iio: inkern: only release the device node when done with it

[Nuno Sá]

'of_node_put()' can potentially release the memory pointed to by
'iiospec.np' which would leave us with an invalid pointer (and we would
still pass it in 'of_xlate()'). Note that it is not guaranteed for the
of_node lifespan to be attached to the device (to which is attached)
lifespan so that there is (even though very unlikely) the possibility
for the node to freed while the device is still around. Thus, as there
are indeed some of_xlate users which do access the node, a possible race
is indeed possible.

As such, we can only release the node after we are done with it.

Fixes: 17d82b47a215d ("iio: Add OF support")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/inkern.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index df74765d33dc..9d87057794fc 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -165,9 +165,10 @@ static int __of_iio_channel_get(struct iio_channel *channel,
 
 	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
 			       iio_dev_node_match);
-	of_node_put(iiospec.np);
-	if (idev == NULL)
+	if (idev == NULL) {
+		of_node_put(iiospec.np);
 		return -EPROBE_DEFER;
+	}
 
 	indio_dev = dev_to_iio_dev(idev);
 	channel->indio_dev = indio_dev;
@@ -175,6 +176,7 @@ static int __of_iio_channel_get(struct iio_channel *channel,
 		index = indio_dev->info->of_xlate(indio_dev, &iiospec);
 	else
 		index = __of_iio_simple_xlate(indio_dev, &iiospec);
+	of_node_put(iiospec.np);
 	if (index < 0)
 		goto err_put;
 	channel->channel = &indio_dev->channels[index];
-- 
2.37.0

[PATCH v2 02/15] iio: inkern: fix return value in devm_of_iio_channel_get_by_name()

[Nuno Sá]

of_iio_channel_get_by_name() can either return NULL or an error pointer
so that only doing IS_ERR() is not enough. Fix it by checking the NULL
pointer case and return -ENODEV in that case. Note this is done like this
so that users of the function (which only check for error pointers) do
not need to be changed. This is not ideal since we are losing error codes
and as such, in a follow up change, things will be unified so that
of_iio_channel_get_by_name() only returns error codes.

Fixes: 6e39b145cef7 ("iio: provide of_iio_channel_get_by_name() and devm_ version it")
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/inkern.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 9d87057794fc..87fd2a0d44f2 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -412,6 +412,8 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
 	channel = of_iio_channel_get_by_name(np, channel_name);
 	if (IS_ERR(channel))
 		return channel;
+	if (!channel)
+		return ERR_PTR(-ENODEV);
 
 	ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
 	if (ret)
-- 
2.37.0

[PATCH v2 03/15] iio: inkern: only return error codes in iio_channel_get_*() APIs

[Nuno Sá]

APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
returning a mix of NULL and pointers with NULL being the way to
"notify" that we should do a "system" lookup for channels. This make
it very confusing and prone to errors as commit 9f63cc0921ec
("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
proves. On top of this, patterns like 'if (channel != NULL) return
channel' were being used where channel could actually be an error code
which makes the code hard to read.

This change also makes some functional changes on how errors were being
handled. In the original behavior, even if we get an error like '-ENOMEM',
we still continue with the search. We should only continue to lookup for
the channel when it makes sense to do so. Hence, the main error handling
in 'of_iio_channel_get_by_name()' is changed to the following logic:

 * If a channel 'name' is provided and we do find it via
'io-channel-names', we should be able to get it. If we get any error,
we should not proceed with the lookup. Moreover, we should return an error
so that callers won't proceed with a system lookup.
 * If a channel 'name' is provided and we cannot find it ('index < 0'),
'of_parse_phandle_with_args()' is expected to fail with '-EINVAL'. Hence,
we should only continue if we get that error.
 * If a channel 'name' is not provided we should only carry on with the
search if 'of_parse_phandle_with_args()' returns '-ENOENT'.

Also note that a system channel lookup is only done if the returned
error code (from 'of_iio_channel_get_by_name()' or
'of_iio_channel_get_all()' is -ENODEV.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/inkern.c | 54 +++++++++++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 87fd2a0d44f2..c6f1cfe09bd3 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
 struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
 					       const char *name)
 {
-	struct iio_channel *chan = NULL;
+	struct iio_channel *chan;
 
 	/* Walk up the tree of devices looking for a matching iio channel */
 	while (np) {
@@ -231,13 +231,33 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
 							 name);
 		chan = of_iio_channel_get(np, index);
 		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
-			break;
-		else if (name && index >= 0) {
-			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
-				np, name ? name : "", index);
-			return NULL;
+			return chan;
+		if (name) {
+			if (index >= 0) {
+				pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
+				       np, name, index);
+				/*
+				 * In this case, we found 'name' in 'io-channel-names'
+				 * but somehow we still fail so that we should not proceed
+				 * with any other lookup. Hence, explicitly return -EINVAL
+				 * (maybe not the better error code) so that the caller
+				 * won't do a system lookup.
+				 */
+				return ERR_PTR(-EINVAL);
+			}
+			/* If index < 0, then of_parse_phandle_with_args() fails
+			 * with -EINVAL which is expected. We should not proceed
+			 * if we get any other error.
+			 */
+			if (PTR_ERR(chan) != -EINVAL)
+				return chan;
+		} else if (PTR_ERR(chan) != -ENOENT) {
+			/*
+			 * if !name, then we should only proceed the lookup if
+			 * of_parse_phandle_with_args() returns -ENOENT.
+			 */
+			return chan;
 		}
-
 		/*
 		 * No matching IIO channel found on this node.
 		 * If the parent node has a "io-channel-ranges" property,
@@ -245,10 +265,10 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
 		 */
 		np = np->parent;
 		if (np && !of_get_property(np, "io-channel-ranges", NULL))
-			return NULL;
+			return ERR_PTR(-ENODEV);
 	}
 
-	return chan;
+	return ERR_PTR(-ENODEV);
 }
 EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
 
@@ -267,8 +287,8 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
 			break;
 	} while (++nummaps);
 
-	if (nummaps == 0)	/* no error, return NULL to search map table */
-		return NULL;
+	if (nummaps == 0)
+		return ERR_PTR(-ENODEV);
 
 	/* NULL terminated array to save passing size */
 	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
@@ -295,7 +315,7 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
 
 static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
 {
-	return NULL;
+	return ERR_PTR(-ENODEV);
 }
 
 #endif /* CONFIG_OF */
@@ -362,7 +382,7 @@ struct iio_channel *iio_channel_get(struct device *dev,
 	if (dev) {
 		channel = of_iio_channel_get_by_name(dev->of_node,
 						     channel_name);
-		if (channel != NULL)
+		if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
 			return channel;
 	}
 
@@ -412,8 +432,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
 	channel = of_iio_channel_get_by_name(np, channel_name);
 	if (IS_ERR(channel))
 		return channel;
-	if (!channel)
-		return ERR_PTR(-ENODEV);
 
 	ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
 	if (ret)
@@ -436,7 +454,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 		return ERR_PTR(-EINVAL);
 
 	chans = of_iio_channel_get_all(dev);
-	if (chans)
+	/*
+	 * We only want to carry on if the error is -ENODEV.  Anything else
+	 * should be reported up the stack.
+	 */
+	if (!IS_ERR(chans) || PTR_ERR(chans) != -ENODEV)
 		return chans;
 
 	name = dev_name(dev);
-- 
2.37.0

[PATCH v2 04/15] iio: inkern: split of_iio_channel_get_by_name()

[Nuno Sá]

This change splits of_iio_channel_get_by_name() so that it decouples
looking for channels in the current node from looking in it's parents
nodes. This will be helpful when moving to fwnode properties where we
need to release the handles when looking for channels in parent's nodes.

No functional change intended...

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/inkern.c | 109 ++++++++++++++++++++++++++-----------------
 1 file changed, 65 insertions(+), 44 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index c6f1cfe09bd3..f97b7967d3d9 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -211,61 +211,82 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
 	return ERR_PTR(err);
 }
 
+struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
+						 const char *name)
+{
+	struct iio_channel *chan;
+	int index = 0;
+
+	/*
+	 * For named iio channels, first look up the name in the
+	 * "io-channel-names" property.  If it cannot be found, the
+	 * index will be an error code, and of_iio_channel_get()
+	 * will fail.
+	 */
+	if (name)
+		index = of_property_match_string(np, "io-channel-names", name);
+
+	chan = of_iio_channel_get(np, index);
+	if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
+		return chan;
+	if (name) {
+		if (index >= 0) {
+			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
+			       np, name, index);
+			/*
+			 * In this case, we found 'name' in 'io-channel-names'
+			 * but somehow we still fail so that we should not proceed
+			 * with any other lookup. Hence, explicitly return -EINVAL
+			 * (maybe not the better error code) so that the caller
+			 * won't do a system lookup.
+			 */
+			return ERR_PTR(-EINVAL);
+		}
+		/* If index < 0, then of_parse_phandle_with_args() fails
+		 * with -EINVAL which is expected. We should not proceed
+		 * if we get any other error.
+		 */
+		if (PTR_ERR(chan) != -EINVAL)
+			return chan;
+	} else if (PTR_ERR(chan) != -ENOENT) {
+		/*
+		 * if !name, then we should only proceed the lookup if
+		 * of_parse_phandle_with_args() returns -ENOENT.
+		 */
+		return chan;
+	}
+
+	/* so we continue the lookup */
+	return ERR_PTR(-ENODEV);
+}
+
 struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
 					       const char *name)
 {
 	struct iio_channel *chan;
 
 	/* Walk up the tree of devices looking for a matching iio channel */
+	chan = __of_iio_channel_get_by_name(np, name);
+	if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
+		return chan;
+
+	/*
+	 * No matching IIO channel found on this node.
+	 * If the parent node has a "io-channel-ranges" property,
+	 * then we can try one of its channels.
+	 */
+	np = np->parent;
 	while (np) {
 		int index = 0;
 
-		/*
-		 * For named iio channels, first look up the name in the
-		 * "io-channel-names" property.  If it cannot be found, the
-		 * index will be an error code, and of_iio_channel_get()
-		 * will fail.
-		 */
-		if (name)
-			index = of_property_match_string(np, "io-channel-names",
-							 name);
-		chan = of_iio_channel_get(np, index);
-		if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
-			return chan;
-		if (name) {
-			if (index >= 0) {
-				pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
-				       np, name, index);
-				/*
-				 * In this case, we found 'name' in 'io-channel-names'
-				 * but somehow we still fail so that we should not proceed
-				 * with any other lookup. Hence, explicitly return -EINVAL
-				 * (maybe not the better error code) so that the caller
-				 * won't do a system lookup.
-				 */
-				return ERR_PTR(-EINVAL);
-			}
-			/* If index < 0, then of_parse_phandle_with_args() fails
-			 * with -EINVAL which is expected. We should not proceed
-			 * if we get any other error.
-			 */
-			if (PTR_ERR(chan) != -EINVAL)
-				return chan;
-		} else if (PTR_ERR(chan) != -ENOENT) {
-			/*
-			 * if !name, then we should only proceed the lookup if
-			 * of_parse_phandle_with_args() returns -ENOENT.
-			 */
+		if (!of_get_property(np, "io-channel-ranges", NULL))
+			return ERR_PTR(-ENODEV);
+
+		chan = __of_iio_channel_get_by_name(np, name);
+		if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
 			return chan;
-		}
-		/*
-		 * No matching IIO channel found on this node.
-		 * If the parent node has a "io-channel-ranges" property,
-		 * then we can try one of its channels.
-		 */
+
 		np = np->parent;
-		if (np && !of_get_property(np, "io-channel-ranges", NULL))
-			return ERR_PTR(-ENODEV);
 	}
 
 	return ERR_PTR(-ENODEV);
-- 
2.37.0

[PATCH v2 05/15] iio: inkern: move to fwnode properties

[Nuno Sá]

This moves the IIO in kernel interface to use fwnode properties and thus
be firmware agnostic.

Note that the interface is still not firmware agnostic. At this point we
have both OF and fwnode interfaces so that we don't break any user. On
top of this we also want to have a per driver conversion and that is the
main reason we have both of_xlate() and fwnode_xlate() support.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/inkern.c         | 159 ++++++++++++++++++-----------------
 include/linux/iio/consumer.h |  36 ++++----
 include/linux/iio/iio.h      |   5 ++
 3 files changed, 108 insertions(+), 92 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f97b7967d3d9..95e015e88645 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -5,6 +5,7 @@
  */
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
@@ -117,15 +118,8 @@ static const struct iio_chan_spec
 	return chan;
 }
 
-#ifdef CONFIG_OF
-
-static int iio_dev_node_match(struct device *dev, const void *data)
-{
-	return dev->of_node == data && dev->type == &iio_device_type;
-}
-
 /**
- * __of_iio_simple_xlate - translate iiospec to the IIO channel index
+ * __fwnode_iio_simple_xlate - translate iiospec to the IIO channel index
  * @indio_dev:	pointer to the iio_dev structure
  * @iiospec:	IIO specifier as found in the device tree
  *
@@ -134,14 +128,14 @@ static int iio_dev_node_match(struct device *dev, const void *data)
  * whether IIO index is less than num_channels (that is specified in the
  * iio_dev).
  */
-static int __of_iio_simple_xlate(struct iio_dev *indio_dev,
-				const struct of_phandle_args *iiospec)
+static int __fwnode_iio_simple_xlate(struct iio_dev *indio_dev,
+				     const struct fwnode_reference_args *iiospec)
 {
-	if (!iiospec->args_count)
+	if (!iiospec->nargs)
 		return 0;
 
 	if (iiospec->args[0] >= indio_dev->num_channels) {
-		dev_err(&indio_dev->dev, "invalid channel index %u\n",
+		dev_err(&indio_dev->dev, "invalid channel index %llu\n",
 			iiospec->args[0]);
 		return -EINVAL;
 	}
@@ -149,34 +143,55 @@ static int __of_iio_simple_xlate(struct iio_dev *indio_dev,
 	return iiospec->args[0];
 }
 
-static int __of_iio_channel_get(struct iio_channel *channel,
-				struct device_node *np, int index)
+/*
+ * Simple helper to copy fwnode_reference_args into of_phandle_args so we
+ * can pass it to of_xlate(). Ultimate goal is to drop this together with
+ * of_xlate().
+ */
+static int __fwnode_to_of_xlate(struct iio_dev *indio_dev,
+				const struct fwnode_reference_args *iiospec)
+{
+	struct of_phandle_args of_args;
+	unsigned int i;
+
+	of_args.args_count = iiospec->nargs;
+	of_args.np = to_of_node(iiospec->fwnode);
+
+	for (i = 0; i < MAX_PHANDLE_ARGS; i++)
+		of_args.args[i] = i < iiospec->nargs ? iiospec->args[i] : 0;
+
+	return indio_dev->info->of_xlate(indio_dev, &of_args);
+}
+
+static int __fwnode_iio_channel_get(struct iio_channel *channel,
+				    struct fwnode_handle *fwnode, int index)
 {
+	struct fwnode_reference_args iiospec;
 	struct device *idev;
 	struct iio_dev *indio_dev;
 	int err;
-	struct of_phandle_args iiospec;
 
-	err = of_parse_phandle_with_args(np, "io-channels",
-					 "#io-channel-cells",
-					 index, &iiospec);
+	err = fwnode_property_get_reference_args(fwnode, "io-channels",
+						 "#io-channel-cells", 0,
+						 index, &iiospec);
 	if (err)
 		return err;
 
-	idev = bus_find_device(&iio_bus_type, NULL, iiospec.np,
-			       iio_dev_node_match);
+	idev = bus_find_device_by_fwnode(&iio_bus_type, iiospec.fwnode);
 	if (idev == NULL) {
-		of_node_put(iiospec.np);
+		fwnode_handle_put(iiospec.fwnode);
 		return -EPROBE_DEFER;
 	}
 
 	indio_dev = dev_to_iio_dev(idev);
 	channel->indio_dev = indio_dev;
 	if (indio_dev->info->of_xlate)
-		index = indio_dev->info->of_xlate(indio_dev, &iiospec);
+		index = __fwnode_to_of_xlate(indio_dev, &iiospec);
+	else if (indio_dev->info->fwnode_xlate)
+		index = indio_dev->info->fwnode_xlate(indio_dev, &iiospec);
 	else
-		index = __of_iio_simple_xlate(indio_dev, &iiospec);
-	of_node_put(iiospec.np);
+		index = __fwnode_iio_simple_xlate(indio_dev, &iiospec);
+	fwnode_handle_put(iiospec.fwnode);
 	if (index < 0)
 		goto err_put;
 	channel->channel = &indio_dev->channels[index];
@@ -188,7 +203,8 @@ static int __of_iio_channel_get(struct iio_channel *channel,
 	return index;
 }
 
-static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
+static struct iio_channel *fwnode_iio_channel_get(struct fwnode_handle *fwnode,
+						  int index)
 {
 	struct iio_channel *channel;
 	int err;
@@ -200,7 +216,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
 	if (channel == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	err = __of_iio_channel_get(channel, np, index);
+	err = __fwnode_iio_channel_get(channel, fwnode, index);
 	if (err)
 		goto err_free_channel;
 
@@ -211,8 +227,8 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
 	return ERR_PTR(err);
 }
 
-struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
-						 const char *name)
+struct iio_channel *
+__fwnode_iio_channel_get_by_name(struct fwnode_handle *fwnode, const char *name)
 {
 	struct iio_channel *chan;
 	int index = 0;
@@ -220,19 +236,20 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
 	/*
 	 * For named iio channels, first look up the name in the
 	 * "io-channel-names" property.  If it cannot be found, the
-	 * index will be an error code, and of_iio_channel_get()
+	 * index will be an error code, and fwnode_iio_channel_get()
 	 * will fail.
 	 */
 	if (name)
-		index = of_property_match_string(np, "io-channel-names", name);
+		index = fwnode_property_match_string(fwnode, "io-channel-names",
+						     name);
 
-	chan = of_iio_channel_get(np, index);
+	chan = fwnode_iio_channel_get(fwnode, index);
 	if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
 		return chan;
 	if (name) {
 		if (index >= 0) {
-			pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
-			       np, name, index);
+			pr_err("ERROR: could not get IIO channel %pfw:%s(%i)\n",
+			       fwnode, name, index);
 			/*
 			 * In this case, we found 'name' in 'io-channel-names'
 			 * but somehow we still fail so that we should not proceed
@@ -242,16 +259,16 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
 			 */
 			return ERR_PTR(-EINVAL);
 		}
-		/* If index < 0, then of_parse_phandle_with_args() fails
-		 * with -EINVAL which is expected. We should not proceed
-		 * if we get any other error.
+		/* If index < 0, then fwnode_property_get_reference_args() fails
+		 * with -EINVAL or -ENOENT (ACPI case) which is expected. We
+		 * should not proceed if we get any other error.
 		 */
-		if (PTR_ERR(chan) != -EINVAL)
+		if (PTR_ERR(chan) != -EINVAL && PTR_ERR(chan) != -ENOENT)
 			return chan;
 	} else if (PTR_ERR(chan) != -ENOENT) {
 		/*
 		 * if !name, then we should only proceed the lookup if
-		 * of_parse_phandle_with_args() returns -ENOENT.
+		 * fwnode_property_get_reference_args() returns -ENOENT.
 		 */
 		return chan;
 	}
@@ -260,13 +277,14 @@ struct iio_channel *__of_iio_channel_get_by_name(struct device_node *np,
 	return ERR_PTR(-ENODEV);
 }
 
-struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
-					       const char *name)
+struct iio_channel *fwnode_iio_channel_get_by_name(struct fwnode_handle *fwnode,
+						   const char *name)
 {
+	struct fwnode_handle *parent;
 	struct iio_channel *chan;
 
 	/* Walk up the tree of devices looking for a matching iio channel */
-	chan = __of_iio_channel_get_by_name(np, name);
+	chan = __fwnode_iio_channel_get_by_name(fwnode, name);
 	if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
 		return chan;
 
@@ -275,35 +293,34 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
 	 * If the parent node has a "io-channel-ranges" property,
 	 * then we can try one of its channels.
 	 */
-	np = np->parent;
-	while (np) {
-		int index = 0;
-
-		if (!of_get_property(np, "io-channel-ranges", NULL))
+	fwnode_for_each_parent_node(fwnode, parent) {
+		if (!fwnode_property_present(parent, "io-channel-ranges")) {
+			fwnode_handle_put(parent);
 			return ERR_PTR(-ENODEV);
+		}
 
-		chan = __of_iio_channel_get_by_name(np, name);
-		if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV)
+		chan = __fwnode_iio_channel_get_by_name(fwnode, name);
+		if (!IS_ERR(chan) || PTR_ERR(chan) != -ENODEV) {
+			fwnode_handle_put(parent);
 			return chan;
-
-		np = np->parent;
+		}
 	}
 
 	return ERR_PTR(-ENODEV);
 }
-EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
+EXPORT_SYMBOL_GPL(fwnode_iio_channel_get_by_name);
 
-static struct iio_channel *of_iio_channel_get_all(struct device *dev)
+static struct iio_channel *fwnode_iio_channel_get_all(struct device *dev)
 {
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
 	struct iio_channel *chans;
 	int i, mapind, nummaps = 0;
 	int ret;
 
 	do {
-		ret = of_parse_phandle_with_args(dev->of_node,
-						 "io-channels",
-						 "#io-channel-cells",
-						 nummaps, NULL);
+		ret = fwnode_property_get_reference_args(fwnode, "io-channels",
+							 "#io-channel-cells", 0,
+							 nummaps, NULL);
 		if (ret < 0)
 			break;
 	} while (++nummaps);
@@ -316,10 +333,9 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
 	if (chans == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	/* Search for OF matches */
+	/* Search for FW matches */
 	for (mapind = 0; mapind < nummaps; mapind++) {
-		ret = __of_iio_channel_get(&chans[mapind], dev->of_node,
-					   mapind);
+		ret = __fwnode_iio_channel_get(&chans[mapind], fwnode, mapind);
 		if (ret)
 			goto error_free_chans;
 	}
@@ -332,15 +348,6 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
 	return ERR_PTR(ret);
 }
 
-#else /* CONFIG_OF */
-
-static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
-{
-	return ERR_PTR(-ENODEV);
-}
-
-#endif /* CONFIG_OF */
-
 static struct iio_channel *iio_channel_get_sys(const char *name,
 					       const char *channel_name)
 {
@@ -401,8 +408,8 @@ struct iio_channel *iio_channel_get(struct device *dev,
 	struct iio_channel *channel;
 
 	if (dev) {
-		channel = of_iio_channel_get_by_name(dev->of_node,
-						     channel_name);
+		channel = fwnode_iio_channel_get_by_name(dev_fwnode(dev),
+							 channel_name);
 		if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
 			return channel;
 	}
@@ -443,14 +450,14 @@ struct iio_channel *devm_iio_channel_get(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_iio_channel_get);
 
-struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
-						    struct device_node *np,
-						    const char *channel_name)
+struct iio_channel *devm_fwnode_iio_channel_get_by_name(struct device *dev,
+							struct fwnode_handle *fwnode,
+							const char *channel_name)
 {
 	struct iio_channel *channel;
 	int ret;
 
-	channel = of_iio_channel_get_by_name(np, channel_name);
+	channel = fwnode_iio_channel_get_by_name(fwnode, channel_name);
 	if (IS_ERR(channel))
 		return channel;
 
@@ -460,7 +467,7 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
 
 	return channel;
 }
-EXPORT_SYMBOL_GPL(devm_of_iio_channel_get_by_name);
+EXPORT_SYMBOL_GPL(devm_fwnode_iio_channel_get_by_name);
 
 struct iio_channel *iio_channel_get_all(struct device *dev)
 {
@@ -474,7 +481,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 	if (dev == NULL)
 		return ERR_PTR(-EINVAL);
 
-	chans = of_iio_channel_get_all(dev);
+	chans = fwnode_iio_channel_get_all(dev);
 	/*
 	 * We only want to carry on if the error is -ENODEV.  Anything else
 	 * should be reported up the stack.
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 5fa5957586cf..2adb1306da3e 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -7,6 +7,7 @@
 #ifndef _IIO_INKERN_CONSUMER_H_
 #define _IIO_INKERN_CONSUMER_H_
 
+#include <linux/of.h>
 #include <linux/types.h>
 #include <linux/iio/types.h>
 
@@ -14,6 +15,7 @@ struct iio_dev;
 struct iio_chan_spec;
 struct device;
 struct device_node;
+struct fwnode_handle;
 
 /**
  * struct iio_channel - everything needed for a consumer to use a channel
@@ -99,26 +101,20 @@ void iio_channel_release_all(struct iio_channel *chan);
 struct iio_channel *devm_iio_channel_get_all(struct device *dev);
 
 /**
- * of_iio_channel_get_by_name() - get description of all that is needed to access channel.
- * @np:			Pointer to consumer device tree node
+ * fwnode_iio_channel_get_by_name() - get description of all that is needed to access channel.
+ * @fwnode:		Pointer to consumer Firmware node
  * @consumer_channel:	Unique name to identify the channel on the consumer
  *			side. This typically describes the channels use within
  *			the consumer. E.g. 'battery_voltage'
  */
-#ifdef CONFIG_OF
-struct iio_channel *of_iio_channel_get_by_name(struct device_node *np, const char *name);
-#else
-static inline struct iio_channel *
-of_iio_channel_get_by_name(struct device_node *np, const char *name)
-{
-	return NULL;
-}
-#endif
+struct iio_channel *fwnode_iio_channel_get_by_name(struct fwnode_handle *fwnode,
+						   const char *name);
 
 /**
- * devm_of_iio_channel_get_by_name() - Resource managed version of of_iio_channel_get_by_name().
+ * devm_fwnode_iio_channel_get_by_name() - Resource managed version of
+ *					   fwnode_iio_channel_get_by_name().
  * @dev:		Pointer to consumer device.
- * @np:			Pointer to consumer device tree node
+ * @fwnode:		Pointer to consumer Firmware node
  * @consumer_channel:	Unique name to identify the channel on the consumer
  *			side. This typically describes the channels use within
  *			the consumer. E.g. 'battery_voltage'
@@ -129,9 +125,17 @@ of_iio_channel_get_by_name(struct device_node *np, const char *name)
  * The allocated iio channel is automatically released when the device is
  * unbound.
  */
-struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
-						    struct device_node *np,
-						    const char *consumer_channel);
+struct iio_channel *devm_fwnode_iio_channel_get_by_name(struct device *dev,
+							struct fwnode_handle *fwnode,
+							const char *consumer_channel);
+
+static inline struct iio_channel
+*devm_of_iio_channel_get_by_name(struct device *dev, struct device_node *np,
+				 const char *consumer_channel)
+{
+	return devm_fwnode_iio_channel_get_by_name(dev, of_fwnode_handle(np),
+						   consumer_channel);
+}
 
 struct iio_cb_buffer;
 /**
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index d9b4a9ca9a0f..494abb63406e 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -18,6 +18,7 @@
  */
 
 struct of_phandle_args;
+struct fwnode_reference_args;
 
 enum iio_shared_by {
 	IIO_SEPARATE,
@@ -429,6 +430,8 @@ struct iio_trigger; /* forward declaration */
  *			provide a custom of_xlate function that reads the
  *			*args* and returns the appropriate index in registered
  *			IIO channels array.
+ * @fwnode_xlate:	fwnode based function pointer to obtain channel specifier index.
+ *			Functionally the same as @of_xlate.
  * @hwfifo_set_watermark: function pointer to set the current hardware
  *			fifo watermark level; see hwfifo_* entries in
  *			Documentation/ABI/testing/sysfs-bus-iio for details on
@@ -510,6 +513,8 @@ struct iio_info {
 				  unsigned *readval);
 	int (*of_xlate)(struct iio_dev *indio_dev,
 			const struct of_phandle_args *iiospec);
+	int (*fwnode_xlate)(struct iio_dev *indio_dev,
+			    const struct fwnode_reference_args *iiospec);
 	int (*hwfifo_set_watermark)(struct iio_dev *indio_dev, unsigned val);
 	int (*hwfifo_flush_to_buffer)(struct iio_dev *indio_dev,
 				      unsigned count);
-- 
2.37.0

[PATCH v2 06/15] thermal: qcom: qcom-spmi-adc-tm5: convert to IIO fwnode API

[Nuno Sá]

Make usage of the new firmware agnostic API
'devm_of_iio_channel_get_by_name()' to get the IIO channel.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
index d9c9c975f931..0b8543c627f0 100644
--- a/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
+++ b/drivers/thermal/qcom/qcom-spmi-adc-tm5.c
@@ -825,7 +825,8 @@ static int adc_tm5_get_dt_channel_data(struct adc_tm5_chip *adc_tm,
 	}
 	channel->adc_channel = args.args[0];
 
-	channel->iio = devm_of_iio_channel_get_by_name(adc_tm->dev, node, NULL);
+	channel->iio = devm_fwnode_iio_channel_get_by_name(adc_tm->dev,
+							   of_fwnode_handle(node), NULL);
 	if (IS_ERR(channel->iio)) {
 		ret = PTR_ERR(channel->iio);
 		if (ret != -EPROBE_DEFER)
-- 
2.37.0

[PATCH v2 07/15] iio: adc: ingenic-adc: convert to IIO fwnode interface

[Nuno Sá]

Move from 'of_xlate()' to 'fwnode_xlate()'. The end goal is to completely
drop OF from the IIO inkernel interface.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/adc/ingenic-adc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
index bf5c03c34f84..9e08f3abeea6 100644
--- a/drivers/iio/adc/ingenic-adc.c
+++ b/drivers/iio/adc/ingenic-adc.c
@@ -719,12 +719,12 @@ static int ingenic_adc_read_raw(struct iio_dev *iio_dev,
 	}
 }
 
-static int ingenic_adc_of_xlate(struct iio_dev *iio_dev,
-				const struct of_phandle_args *iiospec)
+static int ingenic_adc_fwnode_xlate(struct iio_dev *iio_dev,
+				    const struct fwnode_reference_args *iiospec)
 {
 	int i;
 
-	if (!iiospec->args_count)
+	if (!iiospec->nargs)
 		return -EINVAL;
 
 	for (i = 0; i < iio_dev->num_channels; ++i)
@@ -743,7 +743,7 @@ static const struct iio_info ingenic_adc_info = {
 	.write_raw = ingenic_adc_write_raw,
 	.read_raw = ingenic_adc_read_raw,
 	.read_avail = ingenic_adc_read_avail,
-	.of_xlate = ingenic_adc_of_xlate,
+	.fwnode_xlate = ingenic_adc_fwnode_xlate,
 };
 
 static int ingenic_adc_buffer_enable(struct iio_dev *iio_dev)
-- 
2.37.0

[PATCH v2 08/15] iio: adc: ab8500-gpadc: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/adc/ab8500-gpadc.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/ab8500-gpadc.c b/drivers/iio/adc/ab8500-gpadc.c
index 930ce96e6ff5..4fa2126a354b 100644
--- a/drivers/iio/adc/ab8500-gpadc.c
+++ b/drivers/iio/adc/ab8500-gpadc.c
@@ -925,8 +925,8 @@ static int ab8500_gpadc_read_raw(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int ab8500_gpadc_of_xlate(struct iio_dev *indio_dev,
-				 const struct of_phandle_args *iiospec)
+static int ab8500_gpadc_fwnode_xlate(struct iio_dev *indio_dev,
+				     const struct fwnode_reference_args *iiospec)
 {
 	int i;
 
@@ -938,7 +938,7 @@ static int ab8500_gpadc_of_xlate(struct iio_dev *indio_dev,
 }
 
 static const struct iio_info ab8500_gpadc_info = {
-	.of_xlate = ab8500_gpadc_of_xlate,
+	.fwnode_xlate = ab8500_gpadc_fwnode_xlate,
 	.read_raw = ab8500_gpadc_read_raw,
 };
 
@@ -968,7 +968,7 @@ static int ab8500_gpadc_runtime_resume(struct device *dev)
 /**
  * ab8500_gpadc_parse_channel() - process devicetree channel configuration
  * @dev: pointer to containing device
- * @np: device tree node for the channel to configure
+ * @fwnode: fw node for the channel to configure
  * @ch: channel info to fill in
  * @iio_chan: IIO channel specification to fill in
  *
@@ -976,15 +976,15 @@ static int ab8500_gpadc_runtime_resume(struct device *dev)
  * and define usage for things like AUX GPADC inputs more precisely.
  */
 static int ab8500_gpadc_parse_channel(struct device *dev,
-				      struct device_node *np,
+				      struct fwnode_handle *fwnode,
 				      struct ab8500_gpadc_chan_info *ch,
 				      struct iio_chan_spec *iio_chan)
 {
-	const char *name = np->name;
+	const char *name = fwnode_get_name(fwnode);
 	u32 chan;
 	int ret;
 
-	ret = of_property_read_u32(np, "reg", &chan);
+	ret = fwnode_property_read_u32(fwnode, "reg", &chan);
 	if (ret) {
 		dev_err(dev, "invalid channel number %s\n", name);
 		return ret;
@@ -1021,22 +1021,20 @@ static int ab8500_gpadc_parse_channel(struct device *dev,
 /**
  * ab8500_gpadc_parse_channels() - Parse the GPADC channels from DT
  * @gpadc: the GPADC to configure the channels for
- * @np: device tree node containing the channel configurations
  * @chans: the IIO channels we parsed
  * @nchans: the number of IIO channels we parsed
  */
 static int ab8500_gpadc_parse_channels(struct ab8500_gpadc *gpadc,
-				       struct device_node *np,
 				       struct iio_chan_spec **chans_parsed,
 				       unsigned int *nchans_parsed)
 {
-	struct device_node *child;
+	struct fwnode_handle *child;
 	struct ab8500_gpadc_chan_info *ch;
 	struct iio_chan_spec *iio_chans;
 	unsigned int nchans;
 	int i;
 
-	nchans = of_get_available_child_count(np);
+	nchans = device_get_child_node_count(gpadc->dev);
 	if (!nchans) {
 		dev_err(gpadc->dev, "no channel children\n");
 		return -ENODEV;
@@ -1054,7 +1052,7 @@ static int ab8500_gpadc_parse_channels(struct ab8500_gpadc *gpadc,
 		return -ENOMEM;
 
 	i = 0;
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node(gpadc->dev, child) {
 		struct iio_chan_spec *iio_chan;
 		int ret;
 
@@ -1064,7 +1062,7 @@ static int ab8500_gpadc_parse_channels(struct ab8500_gpadc *gpadc,
 		ret = ab8500_gpadc_parse_channel(gpadc->dev, child, ch,
 						 iio_chan);
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 		i++;
@@ -1081,7 +1079,6 @@ static int ab8500_gpadc_probe(struct platform_device *pdev)
 	struct ab8500_gpadc *gpadc;
 	struct iio_dev *indio_dev;
 	struct device *dev = &pdev->dev;
-	struct device_node *np = pdev->dev.of_node;
 	struct iio_chan_spec *iio_chans;
 	unsigned int n_iio_chans;
 	int ret;
@@ -1096,7 +1093,7 @@ static int ab8500_gpadc_probe(struct platform_device *pdev)
 	gpadc->dev = dev;
 	gpadc->ab8500 = dev_get_drvdata(dev->parent);
 
-	ret = ab8500_gpadc_parse_channels(gpadc, np, &iio_chans, &n_iio_chans);
+	ret = ab8500_gpadc_parse_channels(gpadc, &iio_chans, &n_iio_chans);
 	if (ret)
 		return ret;
 
-- 
2.37.0

[PATCH v2 09/15] iio: adc: at91-sama5d2_adc: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/adc/at91-sama5d2_adc.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index fe3131c9593c..df716584c117 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -16,8 +16,9 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/iio/iio.h>
@@ -650,8 +651,8 @@ at91_adc_chan_get(struct iio_dev *indio_dev, int chan)
 	return indio_dev->channels + index;
 }
 
-static inline int at91_adc_of_xlate(struct iio_dev *indio_dev,
-				    const struct of_phandle_args *iiospec)
+static inline int at91_adc_fwnode_xlate(struct iio_dev *indio_dev,
+					const struct fwnode_reference_args *iiospec)
 {
 	return at91_adc_chan_xlate(indio_dev, iiospec->args[0]);
 }
@@ -1876,7 +1877,7 @@ static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 	.write_raw = &at91_adc_write_raw,
 	.update_scan_mode = &at91_adc_update_scan_mode,
-	.of_xlate = &at91_adc_of_xlate,
+	.fwnode_xlate = &at91_adc_fwnode_xlate,
 	.hwfifo_set_watermark = &at91_adc_set_watermark,
 };
 
@@ -1920,6 +1921,7 @@ static int at91_adc_buffer_and_trigger_init(struct device *dev,
 
 static int at91_adc_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	struct at91_adc_state *st;
 	struct resource	*res;
@@ -1933,7 +1935,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 	st = iio_priv(indio_dev);
 	st->indio_dev = indio_dev;
 
-	st->soc_info.platform = of_device_get_match_data(&pdev->dev);
+	st->soc_info.platform = device_get_match_data(dev);
 
 	indio_dev->name = dev_name(&pdev->dev);
 	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
@@ -1950,34 +1952,32 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st->oversampling_ratio = AT91_OSR_1SAMPLES;
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-				   "atmel,min-sample-rate-hz",
-				   &st->soc_info.min_sample_rate);
+	ret = device_property_read_u32(dev, "atmel,min-sample-rate-hz",
+				       &st->soc_info.min_sample_rate);
 	if (ret) {
 		dev_err(&pdev->dev,
 			"invalid or missing value for atmel,min-sample-rate-hz\n");
 		return ret;
 	}
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-				   "atmel,max-sample-rate-hz",
-				   &st->soc_info.max_sample_rate);
+	ret = device_property_read_u32(dev, "atmel,max-sample-rate-hz",
+				       &st->soc_info.max_sample_rate);
 	if (ret) {
 		dev_err(&pdev->dev,
 			"invalid or missing value for atmel,max-sample-rate-hz\n");
 		return ret;
 	}
 
-	ret = of_property_read_u32(pdev->dev.of_node, "atmel,startup-time-ms",
-				   &st->soc_info.startup_time);
+	ret = device_property_read_u32(dev, "atmel,startup-time-ms",
+				       &st->soc_info.startup_time);
 	if (ret) {
 		dev_err(&pdev->dev,
 			"invalid or missing value for atmel,startup-time-ms\n");
 		return ret;
 	}
 
-	ret = of_property_read_u32(pdev->dev.of_node,
-				   "atmel,trigger-edge-type", &edge_type);
+	ret = device_property_read_u32(dev, "atmel,trigger-edge-type",
+				       &edge_type);
 	if (ret) {
 		dev_dbg(&pdev->dev,
 			"atmel,trigger-edge-type not specified, only software trigger available\n");
-- 
2.37.0

[PATCH v2 10/15] iio: adc: qcom-pm8xxx-xoadc: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/adc/qcom-pm8xxx-xoadc.c | 58 ++++++++++++++---------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/qcom-pm8xxx-xoadc.c b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
index 5e9e56821075..eb424496ee1d 100644
--- a/drivers/iio/adc/qcom-pm8xxx-xoadc.c
+++ b/drivers/iio/adc/qcom-pm8xxx-xoadc.c
@@ -14,9 +14,9 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -694,8 +694,8 @@ static int pm8xxx_read_raw(struct iio_dev *indio_dev,
 	}
 }
 
-static int pm8xxx_of_xlate(struct iio_dev *indio_dev,
-			   const struct of_phandle_args *iiospec)
+static int pm8xxx_fwnode_xlate(struct iio_dev *indio_dev,
+			       const struct fwnode_reference_args *iiospec)
 {
 	struct pm8xxx_xoadc *adc = iio_priv(indio_dev);
 	u8 pre_scale_mux;
@@ -706,10 +706,10 @@ static int pm8xxx_of_xlate(struct iio_dev *indio_dev,
 	 * First cell is prescaler or premux, second cell is analog
 	 * mux.
 	 */
-	if (iiospec->args_count != 2) {
-		dev_err(&indio_dev->dev, "wrong number of arguments for %pOFn need 2 got %d\n",
-			iiospec->np,
-			iiospec->args_count);
+	if (iiospec->nargs != 2) {
+		dev_err(&indio_dev->dev, "wrong number of arguments for %pfwP need 2 got %d\n",
+			iiospec->fwnode,
+			iiospec->nargs);
 		return -EINVAL;
 	}
 	pre_scale_mux = (u8)iiospec->args[0];
@@ -727,34 +727,34 @@ static int pm8xxx_of_xlate(struct iio_dev *indio_dev,
 }
 
 static const struct iio_info pm8xxx_xoadc_info = {
-	.of_xlate = pm8xxx_of_xlate,
+	.fwnode_xlate = pm8xxx_fwnode_xlate,
 	.read_raw = pm8xxx_read_raw,
 };
 
 static int pm8xxx_xoadc_parse_channel(struct device *dev,
-				      struct device_node *np,
+				      struct fwnode_handle *fwnode,
 				      const struct xoadc_channel *hw_channels,
 				      struct iio_chan_spec *iio_chan,
 				      struct pm8xxx_chan_info *ch)
 {
-	const char *name = np->name;
+	const char *name = fwnode_get_name(fwnode);
 	const struct xoadc_channel *hwchan;
-	u32 pre_scale_mux, amux_channel;
+	u32 pre_scale_mux, amux_channel, reg[2];
 	u32 rsv, dec;
 	int ret;
 	int chid;
 
-	ret = of_property_read_u32_index(np, "reg", 0, &pre_scale_mux);
+	ret = fwnode_property_read_u32_array(fwnode, "reg", reg,
+					     ARRAY_SIZE(reg));
 	if (ret) {
-		dev_err(dev, "invalid pre scale/mux number %s\n", name);
-		return ret;
-	}
-	ret = of_property_read_u32_index(np, "reg", 1, &amux_channel);
-	if (ret) {
-		dev_err(dev, "invalid amux channel number %s\n", name);
+		dev_err(dev, "invalid pre scale/mux or amux channel number %s\n",
+			name);
 		return ret;
 	}
 
+	pre_scale_mux = reg[0];
+	amux_channel = reg[1];
+
 	/* Find the right channel setting */
 	chid = 0;
 	hwchan = &hw_channels[0];
@@ -778,7 +778,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
 	/* Everyone seems to use default ("type 2") decimation */
 	ch->decimation = VADC_DEF_DECIMATION;
 
-	if (!of_property_read_u32(np, "qcom,ratiometric", &rsv)) {
+	if (!fwnode_property_read_u32(fwnode, "qcom,ratiometric", &rsv)) {
 		ch->calibration = VADC_CALIB_RATIOMETRIC;
 		if (rsv > XOADC_RSV_MAX) {
 			dev_err(dev, "%s too large RSV value %d\n", name, rsv);
@@ -791,7 +791,7 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
 	}
 
 	/* Optional decimation, if omitted we use the default */
-	ret = of_property_read_u32(np, "qcom,decimation", &dec);
+	ret = fwnode_property_read_u32(fwnode, "qcom,decimation", &dec);
 	if (!ret) {
 		ret = qcom_vadc_decimation_from_dt(dec);
 		if (ret < 0) {
@@ -820,15 +820,14 @@ static int pm8xxx_xoadc_parse_channel(struct device *dev,
 	return 0;
 }
 
-static int pm8xxx_xoadc_parse_channels(struct pm8xxx_xoadc *adc,
-				       struct device_node *np)
+static int pm8xxx_xoadc_parse_channels(struct pm8xxx_xoadc *adc)
 {
-	struct device_node *child;
+	struct fwnode_handle *child;
 	struct pm8xxx_chan_info *ch;
 	int ret;
 	int i;
 
-	adc->nchans = of_get_available_child_count(np);
+	adc->nchans = device_get_child_node_count(adc->dev);
 	if (!adc->nchans) {
 		dev_err(adc->dev, "no channel children\n");
 		return -ENODEV;
@@ -846,14 +845,14 @@ static int pm8xxx_xoadc_parse_channels(struct pm8xxx_xoadc *adc,
 		return -ENOMEM;
 
 	i = 0;
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node(adc->dev, child) {
 		ch = &adc->chans[i];
 		ret = pm8xxx_xoadc_parse_channel(adc->dev, child,
 						 adc->variant->channels,
 						 &adc->iio_chans[i],
 						 ch);
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 		i++;
@@ -884,12 +883,11 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 	const struct xoadc_variant *variant;
 	struct pm8xxx_xoadc *adc;
 	struct iio_dev *indio_dev;
-	struct device_node *np = pdev->dev.of_node;
 	struct regmap *map;
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	variant = of_device_get_match_data(dev);
+	variant = device_get_match_data(dev);
 	if (!variant)
 		return -ENODEV;
 
@@ -904,7 +902,7 @@ static int pm8xxx_xoadc_probe(struct platform_device *pdev)
 	init_completion(&adc->complete);
 	mutex_init(&adc->lock);
 
-	ret = pm8xxx_xoadc_parse_channels(adc, np);
+	ret = pm8xxx_xoadc_parse_channels(adc);
 	if (ret)
 		return ret;
 
-- 
2.37.0

[PATCH v2 11/15] iio: adc: qcom-spmi-vadc: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/adc/qcom-spmi-vadc.c | 44 ++++++++++++++++----------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-vadc.c b/drivers/iio/adc/qcom-spmi-vadc.c
index 34202ba52469..bcff0f62b70e 100644
--- a/drivers/iio/adc/qcom-spmi-vadc.c
+++ b/drivers/iio/adc/qcom-spmi-vadc.c
@@ -13,8 +13,9 @@
 #include <linux/kernel.h>
 #include <linux/math64.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/log2.h>
@@ -481,8 +482,8 @@ static int vadc_read_raw(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int vadc_of_xlate(struct iio_dev *indio_dev,
-			 const struct of_phandle_args *iiospec)
+static int vadc_fwnode_xlate(struct iio_dev *indio_dev,
+			     const struct fwnode_reference_args *iiospec)
 {
 	struct vadc_priv *vadc = iio_priv(indio_dev);
 	unsigned int i;
@@ -496,7 +497,7 @@ static int vadc_of_xlate(struct iio_dev *indio_dev,
 
 static const struct iio_info vadc_info = {
 	.read_raw = vadc_read_raw,
-	.of_xlate = vadc_of_xlate,
+	.fwnode_xlate = vadc_fwnode_xlate,
 };
 
 struct vadc_channels {
@@ -647,15 +648,15 @@ static const struct vadc_channels vadc_chans[] = {
 	VADC_CHAN_NO_SCALE(LR_MUX3_BUF_PU1_PU2_XO_THERM, 0)
 };
 
-static int vadc_get_dt_channel_data(struct device *dev,
+static int vadc_get_fw_channel_data(struct device *dev,
 				    struct vadc_channel_prop *prop,
-				    struct device_node *node)
+				    struct fwnode_handle *fwnode)
 {
-	const char *name = node->name;
+	const char *name = fwnode_get_name(fwnode);
 	u32 chan, value, varr[2];
 	int ret;
 
-	ret = of_property_read_u32(node, "reg", &chan);
+	ret = fwnode_property_read_u32(fwnode, "reg", &chan);
 	if (ret) {
 		dev_err(dev, "invalid channel number %s\n", name);
 		return ret;
@@ -669,7 +670,7 @@ static int vadc_get_dt_channel_data(struct device *dev,
 	/* the channel has DT description */
 	prop->channel = chan;
 
-	ret = of_property_read_u32(node, "qcom,decimation", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,decimation", &value);
 	if (!ret) {
 		ret = qcom_vadc_decimation_from_dt(value);
 		if (ret < 0) {
@@ -682,7 +683,7 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->decimation = VADC_DEF_DECIMATION;
 	}
 
-	ret = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2);
+	ret = fwnode_property_read_u32_array(fwnode, "qcom,pre-scaling", varr, 2);
 	if (!ret) {
 		ret = vadc_prescaling_from_dt(varr[0], varr[1]);
 		if (ret < 0) {
@@ -695,7 +696,7 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->prescale = vadc_chans[prop->channel].prescale_index;
 	}
 
-	ret = of_property_read_u32(node, "qcom,hw-settle-time", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,hw-settle-time", &value);
 	if (!ret) {
 		ret = vadc_hw_settle_time_from_dt(value);
 		if (ret < 0) {
@@ -708,7 +709,7 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->hw_settle_time = VADC_DEF_HW_SETTLE_TIME;
 	}
 
-	ret = of_property_read_u32(node, "qcom,avg-samples", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,avg-samples", &value);
 	if (!ret) {
 		ret = vadc_avg_samples_from_dt(value);
 		if (ret < 0) {
@@ -721,7 +722,7 @@ static int vadc_get_dt_channel_data(struct device *dev,
 		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
 	}
 
-	if (of_property_read_bool(node, "qcom,ratiometric"))
+	if (fwnode_property_read_bool(fwnode, "qcom,ratiometric"))
 		prop->calibration = VADC_CALIB_RATIOMETRIC;
 	else
 		prop->calibration = VADC_CALIB_ABSOLUTE;
@@ -731,16 +732,16 @@ static int vadc_get_dt_channel_data(struct device *dev,
 	return 0;
 }
 
-static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
+static int vadc_get_fw_data(struct vadc_priv *vadc)
 {
 	const struct vadc_channels *vadc_chan;
 	struct iio_chan_spec *iio_chan;
 	struct vadc_channel_prop prop;
-	struct device_node *child;
+	struct fwnode_handle *child;
 	unsigned int index = 0;
 	int ret;
 
-	vadc->nchannels = of_get_available_child_count(node);
+	vadc->nchannels = device_get_child_node_count(vadc->dev);
 	if (!vadc->nchannels)
 		return -EINVAL;
 
@@ -756,10 +757,10 @@ static int vadc_get_dt_data(struct vadc_priv *vadc, struct device_node *node)
 
 	iio_chan = vadc->iio_chans;
 
-	for_each_available_child_of_node(node, child) {
-		ret = vadc_get_dt_channel_data(vadc->dev, &prop, child);
+	device_for_each_child_node(vadc->dev, child) {
+		ret = vadc_get_fw_channel_data(vadc->dev, &prop, child);
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 
@@ -848,7 +849,6 @@ static int vadc_check_revision(struct vadc_priv *vadc)
 
 static int vadc_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	struct vadc_priv *vadc;
@@ -860,7 +860,7 @@ static int vadc_probe(struct platform_device *pdev)
 	if (!regmap)
 		return -ENODEV;
 
-	ret = of_property_read_u32(node, "reg", &reg);
+	ret = device_property_read_u32(dev, "reg", &reg);
 	if (ret < 0)
 		return ret;
 
@@ -880,7 +880,7 @@ static int vadc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = vadc_get_dt_data(vadc, node);
+	ret = vadc_get_fw_data(vadc);
 	if (ret)
 		return ret;
 
-- 
2.37.0

[PATCH v2 12/15] iio: adc: qcom-spmi-adc5: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/adc/qcom-spmi-adc5.c | 63 +++++++++++++++-----------------
 1 file changed, 30 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index 87438d1e5c0b..a23f9293d6c1 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -14,9 +14,9 @@
 #include <linux/log2.h>
 #include <linux/math64.h>
 #include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
@@ -403,8 +403,8 @@ static irqreturn_t adc5_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int adc5_of_xlate(struct iio_dev *indio_dev,
-				const struct of_phandle_args *iiospec)
+static int adc5_fwnode_xlate(struct iio_dev *indio_dev,
+			     const struct fwnode_reference_args *iiospec)
 {
 	struct adc5_chip *adc = iio_priv(indio_dev);
 	int i;
@@ -416,8 +416,8 @@ static int adc5_of_xlate(struct iio_dev *indio_dev,
 	return -EINVAL;
 }
 
-static int adc7_of_xlate(struct iio_dev *indio_dev,
-				const struct of_phandle_args *iiospec)
+static int adc7_fwnode_xlate(struct iio_dev *indio_dev,
+			     const struct fwnode_reference_args *iiospec)
 {
 	struct adc5_chip *adc = iio_priv(indio_dev);
 	int i, v_channel;
@@ -481,12 +481,12 @@ static int adc7_read_raw(struct iio_dev *indio_dev,
 
 static const struct iio_info adc5_info = {
 	.read_raw = adc5_read_raw,
-	.of_xlate = adc5_of_xlate,
+	.fwnode_xlate = adc5_fwnode_xlate,
 };
 
 static const struct iio_info adc7_info = {
 	.read_raw = adc7_read_raw,
-	.of_xlate = adc7_of_xlate,
+	.fwnode_xlate = adc7_fwnode_xlate,
 };
 
 struct adc5_channels {
@@ -611,18 +611,18 @@ static const struct adc5_channels adc5_chans_rev2[ADC5_MAX_CHANNEL] = {
 					SCALE_HW_CALIB_THERM_100K_PULLUP)
 };
 
-static int adc5_get_dt_channel_data(struct adc5_chip *adc,
+static int adc5_get_fw_channel_data(struct adc5_chip *adc,
 				    struct adc5_channel_prop *prop,
-				    struct device_node *node,
+				    struct fwnode_handle *fwnode,
 				    const struct adc5_data *data)
 {
-	const char *name = node->name, *channel_name;
+	const char *name = fwnode_get_name(fwnode), *channel_name;
 	u32 chan, value, varr[2];
 	u32 sid = 0;
 	int ret;
 	struct device *dev = adc->dev;
 
-	ret = of_property_read_u32(node, "reg", &chan);
+	ret = fwnode_property_read_u32(fwnode, "reg", &chan);
 	if (ret) {
 		dev_err(dev, "invalid channel number %s\n", name);
 		return ret;
@@ -647,15 +647,13 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
 	prop->channel = chan;
 	prop->sid = sid;
 
-	channel_name = of_get_property(node,
-				"label", NULL) ? : node->name;
-	if (!channel_name) {
-		dev_err(dev, "Invalid channel name\n");
-		return -EINVAL;
-	}
+	ret = fwnode_property_read_string(fwnode, "label", &channel_name);
+	if (ret)
+		channel_name = name;
+
 	prop->datasheet_name = channel_name;
 
-	ret = of_property_read_u32(node, "qcom,decimation", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,decimation", &value);
 	if (!ret) {
 		ret = qcom_adc5_decimation_from_dt(value, data->decimation);
 		if (ret < 0) {
@@ -668,7 +666,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
 		prop->decimation = ADC5_DECIMATION_DEFAULT;
 	}
 
-	ret = of_property_read_u32_array(node, "qcom,pre-scaling", varr, 2);
+	ret = fwnode_property_read_u32_array(fwnode, "qcom,pre-scaling", varr, 2);
 	if (!ret) {
 		ret = qcom_adc5_prescaling_from_dt(varr[0], varr[1]);
 		if (ret < 0) {
@@ -682,7 +680,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
 			adc->data->adc_chans[prop->channel].prescale_index;
 	}
 
-	ret = of_property_read_u32(node, "qcom,hw-settle-time", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,hw-settle-time", &value);
 	if (!ret) {
 		u8 dig_version[2];
 
@@ -713,7 +711,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
 		prop->hw_settle_time = VADC_DEF_HW_SETTLE_TIME;
 	}
 
-	ret = of_property_read_u32(node, "qcom,avg-samples", &value);
+	ret = fwnode_property_read_u32(fwnode, "qcom,avg-samples", &value);
 	if (!ret) {
 		ret = qcom_adc5_avg_samples_from_dt(value);
 		if (ret < 0) {
@@ -726,7 +724,7 @@ static int adc5_get_dt_channel_data(struct adc5_chip *adc,
 		prop->avg_samples = VADC_DEF_AVG_SAMPLES;
 	}
 
-	if (of_property_read_bool(node, "qcom,ratiometric"))
+	if (fwnode_property_read_bool(fwnode, "qcom,ratiometric"))
 		prop->cal_method = ADC5_RATIOMETRIC_CAL;
 	else
 		prop->cal_method = ADC5_ABSOLUTE_CAL;
@@ -801,16 +799,16 @@ static const struct of_device_id adc5_match_table[] = {
 };
 MODULE_DEVICE_TABLE(of, adc5_match_table);
 
-static int adc5_get_dt_data(struct adc5_chip *adc, struct device_node *node)
+static int adc5_get_fw_data(struct adc5_chip *adc)
 {
 	const struct adc5_channels *adc_chan;
 	struct iio_chan_spec *iio_chan;
 	struct adc5_channel_prop prop, *chan_props;
-	struct device_node *child;
+	struct fwnode_handle *child;
 	unsigned int index = 0;
 	int ret;
 
-	adc->nchannels = of_get_available_child_count(node);
+	adc->nchannels = device_get_child_node_count(adc->dev);
 	if (!adc->nchannels)
 		return -EINVAL;
 
@@ -826,14 +824,14 @@ static int adc5_get_dt_data(struct adc5_chip *adc, struct device_node *node)
 
 	chan_props = adc->chan_props;
 	iio_chan = adc->iio_chans;
-	adc->data = of_device_get_match_data(adc->dev);
+	adc->data = device_get_match_data(adc->dev);
 	if (!adc->data)
 		adc->data = &adc5_data_pmic;
 
-	for_each_available_child_of_node(node, child) {
-		ret = adc5_get_dt_channel_data(adc, &prop, child, adc->data);
+	device_for_each_child_node(adc->dev, child) {
+		ret = adc5_get_fw_channel_data(adc, &prop, child, adc->data);
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 
@@ -858,7 +856,6 @@ static int adc5_get_dt_data(struct adc5_chip *adc, struct device_node *node)
 
 static int adc5_probe(struct platform_device *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
 	struct iio_dev *indio_dev;
 	struct adc5_chip *adc;
@@ -870,7 +867,7 @@ static int adc5_probe(struct platform_device *pdev)
 	if (!regmap)
 		return -ENODEV;
 
-	ret = of_property_read_u32(node, "reg", &reg);
+	ret = device_property_read_u32(dev, "reg", &reg);
 	if (ret < 0)
 		return ret;
 
@@ -886,7 +883,7 @@ static int adc5_probe(struct platform_device *pdev)
 	init_completion(&adc->complete);
 	mutex_init(&adc->lock);
 
-	ret = adc5_get_dt_data(adc, node);
+	ret = adc5_get_fw_data(adc);
 	if (ret) {
 		dev_err(dev, "adc get dt data failed\n");
 		return ret;
-- 
2.37.0

[PATCH v2 13/15] iio: adc: stm32-adc: convert to device properties

[Nuno Sá]

Make the conversion to firmware agnostic device properties. As part of
the conversion the IIO inkern interface 'of_xlate()' is also converted to
'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
dependencies from IIO.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/adc/stm32-adc.c | 121 ++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 54 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 3985fe972892..e55859113df8 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -21,11 +21,11 @@
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/mod_devicetable.h>
 #include <linux/nvmem-consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
-#include <linux/of.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 
 #include "stm32-adc-core.h"
 
@@ -1530,8 +1530,8 @@ static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
-			      const struct of_phandle_args *iiospec)
+static int stm32_adc_fwnode_xlate(struct iio_dev *indio_dev,
+				  const struct fwnode_reference_args *iiospec)
 {
 	int i;
 
@@ -1585,7 +1585,7 @@ static const struct iio_info stm32_adc_iio_info = {
 	.hwfifo_set_watermark = stm32_adc_set_watermark,
 	.update_scan_mode = stm32_adc_update_scan_mode,
 	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
-	.of_xlate = stm32_adc_of_xlate,
+	.fwnode_xlate = stm32_adc_fwnode_xlate,
 };
 
 static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
@@ -1782,14 +1782,14 @@ static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
 	{},
 };
 
-static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
+static int stm32_adc_fw_get_resolution(struct iio_dev *indio_dev)
 {
-	struct device_node *node = indio_dev->dev.of_node;
+	struct device *dev = &indio_dev->dev;
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	unsigned int i;
 	u32 res;
 
-	if (of_property_read_u32(node, "assigned-resolution-bits", &res))
+	if (device_property_read_u32(dev, "assigned-resolution-bits", &res))
 		res = adc->cfg->adc_info->resolutions[0];
 
 	for (i = 0; i < adc->cfg->adc_info->num_res; i++)
@@ -1873,11 +1873,11 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
 
 static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm32_adc *adc)
 {
-	struct device_node *node = indio_dev->dev.of_node;
+	struct device *dev = &indio_dev->dev;
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
 	int num_channels = 0, ret;
 
-	ret = of_property_count_u32_elems(node, "st,adc-channels");
+	ret = device_property_count_u32(dev, "st,adc-channels");
 	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
 		return -EINVAL;
@@ -1885,8 +1885,8 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
 		num_channels += ret;
 	}
 
-	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
-					      sizeof(struct stm32_adc_diff_channel));
+	/* each st,adc-diff-channels is a group of 2 u32 */
+	ret = device_property_count_u64(dev, "st,adc-diff-channels");
 	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
 		return -EINVAL;
@@ -1896,7 +1896,7 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
 	}
 
 	/* Optional sample time is provided either for each, or all channels */
-	ret = of_property_count_u32_elems(node, "st,min-sample-time-nsecs");
+	ret = device_property_count_u32(dev, "st,min-sample-time-nsecs");
 	if (ret > 1 && ret != num_channels) {
 		dev_err(&indio_dev->dev, "Invalid st,min-sample-time-nsecs\n");
 		return -EINVAL;
@@ -1907,21 +1907,20 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
 
 static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
 				      struct stm32_adc *adc,
-				      struct iio_chan_spec *channels)
+				      struct iio_chan_spec *channels,
+				      int nchans)
 {
-	struct device_node *node = indio_dev->dev.of_node;
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
 	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
+	struct device *dev = &indio_dev->dev;
 	u32 num_diff = adc->num_diff;
 	int size = num_diff * sizeof(*diff) / sizeof(u32);
-	int scan_index = 0, val, ret, i;
-	struct property *prop;
-	const __be32 *cur;
-	u32 smp = 0;
+	int scan_index = 0, ret, i;
+	u32 smp = 0, nsmps, smps[STM32_ADC_CH_MAX], chans[STM32_ADC_CH_MAX];
 
 	if (num_diff) {
-		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
-						 (u32 *)diff, size);
+		ret = device_property_read_u32_array(dev, "st,adc-diff-channels",
+						     (u32 *)diff, size);
 		if (ret) {
 			dev_err(&indio_dev->dev, "Failed to get diff channels %d\n", ret);
 			return ret;
@@ -1942,32 +1941,51 @@ static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
 		}
 	}
 
-	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
-		if (val >= adc_info->max_channels) {
-			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
+	ret = device_property_read_u32_array(dev, "st,adc-channels", chans,
+					     nchans);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < nchans; i++) {
+		if (chans[i] >= adc_info->max_channels) {
+			dev_err(&indio_dev->dev, "Invalid channel %d\n",
+				chans[i]);
 			return -EINVAL;
 		}
 
 		/* Channel can't be configured both as single-ended & diff */
 		for (i = 0; i < num_diff; i++) {
-			if (val == diff[i].vinp) {
-				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	val);
+			if (chans[i] == diff[i].vinp) {
+				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	chans[i]);
 				return -EINVAL;
 			}
 		}
-		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
-					0, scan_index, false);
+		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
+					chans[i], 0, scan_index, false);
 		scan_index++;
 	}
 
+	nsmps = device_property_count_u32(dev, "st,min-sample-time-nsecs");
+	if (nsmps) {
+		if (nsmps >= nchans)
+			nsmps = nchans;
+
+		ret = device_property_read_u32_array(dev, "st,min-sample-time-nsecs",
+						     smps, nsmps);
+		if (ret)
+			return ret;
+	}
+
 	for (i = 0; i < scan_index; i++) {
 		/*
-		 * Using of_property_read_u32_index(), smp value will only be
-		 * modified if valid u32 value can be decoded. This allows to
-		 * get either no value, 1 shared value for all indexes, or one
-		 * value per channel.
+		 * This check is used with the above logic so that smp value
+		 * will only be modified if valid u32 value can be decoded. This
+		 * allows to get either no value, 1 shared value for all indexes,
+		 * or one value per channel. The point is to have the same
+		 * behavior as 'of_property_read_u32_index()'.
 		 */
-		of_property_read_u32_index(node, "st,min-sample-time-nsecs", i, &smp);
+		if (i < nsmps)
+			smp = smps[i];
 
 		/* Prepare sampling time settings */
 		stm32_adc_smpr_init(adc, channels[i].channel, smp);
@@ -2010,22 +2028,21 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 				       struct stm32_adc *adc,
 				       struct iio_chan_spec *channels)
 {
-	struct device_node *node = indio_dev->dev.of_node;
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
-	struct device_node *child;
+	struct fwnode_handle *child;
 	const char *name;
 	int val, scan_index = 0, ret;
 	bool differential;
 	u32 vin[2];
 
-	for_each_available_child_of_node(node, child) {
-		ret = of_property_read_u32(child, "reg", &val);
+	device_for_each_child_node(&indio_dev->dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &val);
 		if (ret) {
 			dev_err(&indio_dev->dev, "Missing channel index %d\n", ret);
 			goto err;
 		}
 
-		ret = of_property_read_string(child, "label", &name);
+		ret = fwnode_property_read_string(child, "label", &name);
 		/* label is optional */
 		if (!ret) {
 			if (strlen(name) >= STM32_ADC_CH_SZ) {
@@ -2050,7 +2067,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 		}
 
 		differential = false;
-		ret = of_property_read_u32_array(child, "diff-channels", vin, 2);
+		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);
 		/* diff-channels is optional */
 		if (!ret) {
 			differential = true;
@@ -2067,7 +2084,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
 					vin[1], scan_index, differential);
 
-		ret = of_property_read_u32(child, "st,min-sample-time-ns", &val);
+		ret = fwnode_property_read_u32(child, "st,min-sample-time-ns", &val);
 		/* st,min-sample-time-ns is optional */
 		if (!ret) {
 			stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
@@ -2085,14 +2102,13 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 	return scan_index;
 
 err:
-	of_node_put(child);
+	fwnode_handle_put(child);
 
 	return ret;
 }
 
-static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
+static int stm32_adc_chan_fw_init(struct iio_dev *indio_dev, bool timestamping)
 {
-	struct device_node *node = indio_dev->dev.of_node;
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
 	struct iio_chan_spec *channels;
@@ -2102,7 +2118,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
 	for (i = 0; i < STM32_ADC_INT_CH_NB; i++)
 		adc->int_ch[i] = STM32_ADC_INT_CH_NONE;
 
-	num_channels = of_get_available_child_count(node);
+	num_channels = device_get_child_node_count(&indio_dev->dev);
 	/* If no channels have been found, fallback to channels legacy properties. */
 	if (!num_channels) {
 		legacy = true;
@@ -2133,7 +2149,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
 		return -ENOMEM;
 
 	if (legacy)
-		ret = stm32_adc_legacy_chan_init(indio_dev, adc, channels);
+		ret = stm32_adc_legacy_chan_init(indio_dev, adc, channels,
+						 num_channels);
 	else
 		ret = stm32_adc_generic_chan_init(indio_dev, adc, channels);
 	if (ret < 0)
@@ -2215,9 +2232,6 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	bool timestamping = false;
 	int ret;
 
-	if (!pdev->dev.of_node)
-		return -ENODEV;
-
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -2226,17 +2240,16 @@ static int stm32_adc_probe(struct platform_device *pdev)
 	adc->common = dev_get_drvdata(pdev->dev.parent);
 	spin_lock_init(&adc->lock);
 	init_completion(&adc->completion);
-	adc->cfg = (const struct stm32_adc_cfg *)
-		of_match_device(dev->driver->of_match_table, dev)->data;
+	adc->cfg = device_get_match_data(dev);
 
 	indio_dev->name = dev_name(&pdev->dev);
-	indio_dev->dev.of_node = pdev->dev.of_node;
+	device_set_node(&indio_dev->dev, dev_fwnode(&pdev->dev));
 	indio_dev->info = &stm32_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
+	ret = device_property_read_u32(dev, "reg", &adc->offset);
 	if (ret != 0) {
 		dev_err(&pdev->dev, "missing reg property\n");
 		return -EINVAL;
@@ -2265,7 +2278,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = stm32_adc_of_get_resolution(indio_dev);
+	ret = stm32_adc_fw_get_resolution(indio_dev);
 	if (ret < 0)
 		return ret;
 
@@ -2282,7 +2295,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
 		timestamping = true;
 	}
 
-	ret = stm32_adc_chan_of_init(indio_dev, timestamping);
+	ret = stm32_adc_chan_fw_init(indio_dev, timestamping);
 	if (ret < 0)
 		goto err_dma_disable;
 
-- 
2.37.0

[PATCH v2 14/15] iio: inkern: remove OF dependencies

[Nuno Sá]

Since all users of the OF dependendent API are now converted to use the
firmware agnostic alternative, we can drop OF dependencies from the IIO
in kernel interface.

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/inkern.c         | 25 +------------------------
 include/linux/iio/consumer.h | 10 ----------
 include/linux/iio/iio.h      |  3 ---
 3 files changed, 1 insertion(+), 37 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 95e015e88645..fab951546086 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -8,7 +8,6 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/iio-opaque.h>
@@ -143,26 +142,6 @@ static int __fwnode_iio_simple_xlate(struct iio_dev *indio_dev,
 	return iiospec->args[0];
 }
 
-/*
- * Simple helper to copy fwnode_reference_args into of_phandle_args so we
- * can pass it to of_xlate(). Ultimate goal is to drop this together with
- * of_xlate().
- */
-static int __fwnode_to_of_xlate(struct iio_dev *indio_dev,
-				const struct fwnode_reference_args *iiospec)
-{
-	struct of_phandle_args of_args;
-	unsigned int i;
-
-	of_args.args_count = iiospec->nargs;
-	of_args.np = to_of_node(iiospec->fwnode);
-
-	for (i = 0; i < MAX_PHANDLE_ARGS; i++)
-		of_args.args[i] = i < iiospec->nargs ? iiospec->args[i] : 0;
-
-	return indio_dev->info->of_xlate(indio_dev, &of_args);
-}
-
 static int __fwnode_iio_channel_get(struct iio_channel *channel,
 				    struct fwnode_handle *fwnode, int index)
 {
@@ -185,9 +164,7 @@ static int __fwnode_iio_channel_get(struct iio_channel *channel,
 
 	indio_dev = dev_to_iio_dev(idev);
 	channel->indio_dev = indio_dev;
-	if (indio_dev->info->of_xlate)
-		index = __fwnode_to_of_xlate(indio_dev, &iiospec);
-	else if (indio_dev->info->fwnode_xlate)
+	if (indio_dev->info->fwnode_xlate)
 		index = indio_dev->info->fwnode_xlate(indio_dev, &iiospec);
 	else
 		index = __fwnode_iio_simple_xlate(indio_dev, &iiospec);
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index 2adb1306da3e..6802596b017c 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -7,14 +7,12 @@
 #ifndef _IIO_INKERN_CONSUMER_H_
 #define _IIO_INKERN_CONSUMER_H_
 
-#include <linux/of.h>
 #include <linux/types.h>
 #include <linux/iio/types.h>
 
 struct iio_dev;
 struct iio_chan_spec;
 struct device;
-struct device_node;
 struct fwnode_handle;
 
 /**
@@ -129,14 +127,6 @@ struct iio_channel *devm_fwnode_iio_channel_get_by_name(struct device *dev,
 							struct fwnode_handle *fwnode,
 							const char *consumer_channel);
 
-static inline struct iio_channel
-*devm_of_iio_channel_get_by_name(struct device *dev, struct device_node *np,
-				 const char *consumer_channel)
-{
-	return devm_fwnode_iio_channel_get_by_name(dev, of_fwnode_handle(np),
-						   consumer_channel);
-}
-
 struct iio_cb_buffer;
 /**
  * iio_channel_get_all_cb() - register callback for triggered capture
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 494abb63406e..7093be1531c1 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -17,7 +17,6 @@
  * Currently assumes nano seconds.
  */
 
-struct of_phandle_args;
 struct fwnode_reference_args;
 
 enum iio_shared_by {
@@ -511,8 +510,6 @@ struct iio_info {
 	int (*debugfs_reg_access)(struct iio_dev *indio_dev,
 				  unsigned reg, unsigned writeval,
 				  unsigned *readval);
-	int (*of_xlate)(struct iio_dev *indio_dev,
-			const struct of_phandle_args *iiospec);
 	int (*fwnode_xlate)(struct iio_dev *indio_dev,
 			    const struct fwnode_reference_args *iiospec);
 	int (*hwfifo_set_watermark)(struct iio_dev *indio_dev, unsigned val);
-- 
2.37.0

[PATCH v2 15/15] iio: inkern: fix coding style warnings

[Nuno Sá]

Just cosmetics. No functional change intended...

Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
 drivers/iio/inkern.c | 64 ++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index fab951546086..9ce6ff2a3484 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -45,13 +45,13 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
 	int i = 0, ret = 0;
 	struct iio_map_internal *mapi;
 
-	if (maps == NULL)
+	if (!maps)
 		return 0;
 
 	mutex_lock(&iio_map_list_lock);
-	while (maps[i].consumer_dev_name != NULL) {
+	while (maps[i].consumer_dev_name) {
 		mapi = kzalloc(sizeof(*mapi), GFP_KERNEL);
-		if (mapi == NULL) {
+		if (!mapi) {
 			ret = -ENOMEM;
 			goto error_ret;
 		}
@@ -69,7 +69,6 @@ int iio_map_array_register(struct iio_dev *indio_dev, struct iio_map *maps)
 }
 EXPORT_SYMBOL_GPL(iio_map_array_register);
 
-
 /*
  * Remove all map entries associated with the given iio device
  */
@@ -157,7 +156,7 @@ static int __fwnode_iio_channel_get(struct iio_channel *channel,
 		return err;
 
 	idev = bus_find_device_by_fwnode(&iio_bus_type, iiospec.fwnode);
-	if (idev == NULL) {
+	if (!idev) {
 		fwnode_handle_put(iiospec.fwnode);
 		return -EPROBE_DEFER;
 	}
@@ -190,7 +189,7 @@ static struct iio_channel *fwnode_iio_channel_get(struct fwnode_handle *fwnode,
 		return ERR_PTR(-EINVAL);
 
 	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
-	if (channel == NULL)
+	if (!channel)
 		return ERR_PTR(-ENOMEM);
 
 	err = __fwnode_iio_channel_get(channel, fwnode, index);
@@ -307,7 +306,7 @@ static struct iio_channel *fwnode_iio_channel_get_all(struct device *dev)
 
 	/* NULL terminated array to save passing size */
 	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
-	if (chans == NULL)
+	if (!chans)
 		return ERR_PTR(-ENOMEM);
 
 	/* Search for FW matches */
@@ -332,7 +331,7 @@ static struct iio_channel *iio_channel_get_sys(const char *name,
 	struct iio_channel *channel;
 	int err;
 
-	if (name == NULL && channel_name == NULL)
+	if (!name && !channel_name)
 		return ERR_PTR(-ENODEV);
 
 	/* first find matching entry the channel map */
@@ -347,11 +346,11 @@ static struct iio_channel *iio_channel_get_sys(const char *name,
 		break;
 	}
 	mutex_unlock(&iio_map_list_lock);
-	if (c == NULL)
+	if (!c)
 		return ERR_PTR(-ENODEV);
 
 	channel = kzalloc(sizeof(*channel), GFP_KERNEL);
-	if (channel == NULL) {
+	if (!channel) {
 		err = -ENOMEM;
 		goto error_no_mem;
 	}
@@ -363,7 +362,7 @@ static struct iio_channel *iio_channel_get_sys(const char *name,
 			iio_chan_spec_from_name(channel->indio_dev,
 						c->map->adc_channel_label);
 
-		if (channel->channel == NULL) {
+		if (!channel->channel) {
 			err = -EINVAL;
 			goto error_no_chan;
 		}
@@ -455,7 +454,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 	int mapind = 0;
 	int i, ret;
 
-	if (dev == NULL)
+	if (!dev)
 		return ERR_PTR(-EINVAL);
 
 	chans = fwnode_iio_channel_get_all(dev);
@@ -483,7 +482,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 
 	/* NULL terminated array to save passing size */
 	chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
-	if (chans == NULL) {
+	if (!chans) {
 		ret = -ENOMEM;
 		goto error_ret;
 	}
@@ -497,7 +496,7 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
 		chans[mapind].channel =
 			iio_chan_spec_from_name(chans[mapind].indio_dev,
 						c->map->adc_channel_label);
-		if (chans[mapind].channel == NULL) {
+		if (!chans[mapind].channel) {
 			ret = -EINVAL;
 			goto error_free_chans;
 		}
@@ -559,14 +558,14 @@ struct iio_channel *devm_iio_channel_get_all(struct device *dev)
 EXPORT_SYMBOL_GPL(devm_iio_channel_get_all);
 
 static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
-	enum iio_chan_info_enum info)
+			    enum iio_chan_info_enum info)
 {
 	int unused;
 	int vals[INDIO_MAX_RAW_ELEMENTS];
 	int ret;
 	int val_len = 2;
 
-	if (val2 == NULL)
+	if (!val2)
 		val2 = &unused;
 
 	if (!iio_channel_has_info(chan->channel, info))
@@ -578,9 +577,10 @@ static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
 					vals, &val_len, info);
 		*val = vals[0];
 		*val2 = vals[1];
-	} else
+	} else {
 		ret = chan->indio_dev->info->read_raw(chan->indio_dev,
 					chan->channel, val, val2, info);
+	}
 
 	return ret;
 }
@@ -591,7 +591,7 @@ int iio_read_channel_raw(struct iio_channel *chan, int *val)
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -610,7 +610,7 @@ int iio_read_channel_average_raw(struct iio_channel *chan, int *val)
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -624,7 +624,8 @@ int iio_read_channel_average_raw(struct iio_channel *chan, int *val)
 EXPORT_SYMBOL_GPL(iio_read_channel_average_raw);
 
 static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
-	int raw, int *processed, unsigned int scale)
+						 int raw, int *processed,
+						 unsigned int scale)
 {
 	int scale_type, scale_val, scale_val2;
 	int offset_type, offset_val, offset_val2;
@@ -657,7 +658,7 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 	}
 
 	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
-					IIO_CHAN_INFO_SCALE);
+				      IIO_CHAN_INFO_SCALE);
 	if (scale_type < 0) {
 		/*
 		 * If no channel scaling is available apply consumer scale to
@@ -702,19 +703,19 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
 }
 
 int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
-	int *processed, unsigned int scale)
+				 int *processed, unsigned int scale)
 {
 	struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
 
 	ret = iio_convert_raw_to_processed_unlocked(chan, raw, processed,
-							scale);
+						    scale);
 err_unlock:
 	mutex_unlock(&iio_dev_opaque->info_exist_lock);
 
@@ -729,7 +730,7 @@ int iio_read_channel_attribute(struct iio_channel *chan, int *val, int *val2,
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -755,7 +756,7 @@ int iio_read_channel_processed_scale(struct iio_channel *chan, int *val,
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -833,7 +834,7 @@ int iio_read_avail_channel_raw(struct iio_channel *chan,
 	int type;
 
 	ret = iio_read_avail_channel_attribute(chan, vals, &type, length,
-					 IIO_CHAN_INFO_RAW);
+					       IIO_CHAN_INFO_RAW);
 
 	if (ret >= 0 && type != IIO_VAL_INT)
 		/* raw values are assumed to be IIO_VAL_INT */
@@ -917,7 +918,7 @@ int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
 	/* Need to verify underlying driver has not gone away */
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -944,7 +945,7 @@ int iio_write_channel_attribute(struct iio_channel *chan, int val, int val2,
 	int ret;
 
 	mutex_lock(&iio_dev_opaque->info_exist_lock);
-	if (chan->indio_dev->info == NULL) {
+	if (!chan->indio_dev->info) {
 		ret = -ENODEV;
 		goto err_unlock;
 	}
@@ -978,9 +979,8 @@ unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
 }
 EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
 
-static const struct iio_chan_spec_ext_info *iio_lookup_ext_info(
-						const struct iio_channel *chan,
-						const char *attr)
+static const struct iio_chan_spec_ext_info *
+iio_lookup_ext_info(const struct iio_channel *chan, const char *attr)
 {
 	const struct iio_chan_spec_ext_info *ext_info;
 
-- 
2.37.0

Re: [PATCH v2 01/15] iio: inkern: only release the device node when done with it

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 2:38 PM Nuno Sá <nuno.sa@analog.com> wrote:
>
> 'of_node_put()' can potentially release the memory pointed to by
> 'iiospec.np' which would leave us with an invalid pointer (and we would
> still pass it in 'of_xlate()'). Note that it is not guaranteed for the
> of_node lifespan to be attached to the device (to which is attached)
> lifespan so that there is (even though very unlikely) the possibility
> for the node to freed while the device is still around. Thus, as there

be freed

> are indeed some of_xlate users which do access the node, a possible race
> is indeed possible.

possible ... possible.

(I would drop the first one)

> As such, we can only release the node after we are done with it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 2:40 PM Nuno Sá <nuno.sa@analog.com> wrote:
>
> Just cosmetics. No functional change intended...

...

> -       if (name == NULL && channel_name == NULL)
> +       if (!name && !channel_name)
>                 return ERR_PTR(-ENODEV);

After this change in place, I think it's better to convert it to

  if (!(name || channel_name))

which shows intentions clearer.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/15] make iio inkern interface firmware agnostic

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 2:38 PM Nuno Sá <nuno.sa@analog.com> wrote:
>
> First version of the series can be found here:
>
> https://lore.kernel.org/linux-iio/20220610084545.547700-1-nuno.sa@analog.com/

I'm under the impression that I gave tags for some of these patches
when they were the part of the bigger series. Am I wrong?
In any case for patch 6-14,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> v2 changes:
>
> [1/15]
>   * Fix typo and added more description in the commit message.
>
> [3/15]
>   * Remove superfluous code;
>   * Commit message spell fixes and added more details;
>   * Improved error handling (this is the most significant change in this
> version. More details on the commit message).
>
> [4/15]
>   * Drop the 'ugly' parent_lookup flag. With the new error handling,
>     we can use -ENODEV to infer if we should proceed or not with the
>     lookup.
>
> [5/15]:
>   * Moved some local declarations up so long lines first;
>   * Use 'bus_find_device_by_fwnode()';
>   * Proper ordering in includes.
>   * Adapted error handling in '__fwnode_iio_channel_get_by_name()' taking
> ACPI into account and when 'name' is given but index < 0. It seems that
> ACPI code can actually return -ENOENT with index < 0 for which case we
> should continue the search. Not sure if a check  in ACPI ('if (index < 0)
> return -EINVAL;) like is done in OF would make sense...
>
> [12/15]:
>   * Use 'device_property_count_u64()' to get the number of diff channels.
> So no need for 'magic' divisions by 2 (no idea why I haven't done like
> this in the first place).
>
> [15/15]
>   * Fix wrong conversion of 'if (ptr != NULL)' to 'if (!ptr)'.
>
> Special note for patch 3/15 where -ENODEV is still used despite some talks
> about using -ENOENT and hence, be more in line with firmware code. The
> reason I kept it was to be consistent with the rest of the file. I'd say
> that if we want to move to -ENOENT we should do it in a separate patch
> and for the complete file.
>
> Nuno Sá (15):
>   iio: inkern: only release the device node when done with it
>   iio: inkern: fix return value in devm_of_iio_channel_get_by_name()
>   iio: inkern: only return error codes in iio_channel_get_*() APIs
>   iio: inkern: split of_iio_channel_get_by_name()
>   iio: inkern: move to fwnode properties
>   thermal: qcom: qcom-spmi-adc-tm5: convert to IIO fwnode API
>   iio: adc: ingenic-adc: convert to IIO fwnode interface
>   iio: adc: ab8500-gpadc: convert to device properties
>   iio: adc: at91-sama5d2_adc: convert to device properties
>   iio: adc: qcom-pm8xxx-xoadc: convert to device properties
>   iio: adc: qcom-spmi-vadc: convert to device properties
>   iio: adc: qcom-spmi-adc5: convert to device properties
>   iio: adc: stm32-adc: convert to device properties
>   iio: inkern: remove OF dependencies
>   iio: inkern: fix coding style warnings
>
>  drivers/iio/adc/ab8500-gpadc.c           |  27 +--
>  drivers/iio/adc/at91-sama5d2_adc.c       |  30 +--
>  drivers/iio/adc/ingenic-adc.c            |   8 +-
>  drivers/iio/adc/qcom-pm8xxx-xoadc.c      |  58 +++--
>  drivers/iio/adc/qcom-spmi-adc5.c         |  63 +++---
>  drivers/iio/adc/qcom-spmi-vadc.c         |  44 ++--
>  drivers/iio/adc/stm32-adc.c              | 121 +++++-----
>  drivers/iio/inkern.c                     | 271 +++++++++++++----------
>  drivers/thermal/qcom/qcom-spmi-adc-tm5.c |   3 +-
>  include/linux/iio/consumer.h             |  28 +--
>  include/linux/iio/iio.h                  |   8 +-
>  11 files changed, 347 insertions(+), 314 deletions(-)
>
> --
> 2.37.0
>


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 15/15] iio: inkern: fix coding style warnings

[Biju Das]

Hi Andy,

> Subject: Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings
> 
> On Mon, Jul 11, 2022 at 2:40 PM Nuno Sá <nuno.sa@analog.com> wrote:
> >
> > Just cosmetics. No functional change intended...
> 
> ...
> 
> > -       if (name == NULL && channel_name == NULL)
> > +       if (!name && !channel_name)
> >                 return ERR_PTR(-ENODEV);
> 
> After this change in place, I think it's better to convert it to
> 
>   if (!(name || channel_name))

It should be name && channel_name

See below.
 (! ( 1 || 0)->  (! (1 && 1)

Cheers,
Biju

> 
> which shows intentions clearer.
> 
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH v2 03/15] iio: inkern: only return error codes in iio_channel_get_*() APIs

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 2:38 PM Nuno Sá <nuno.sa@analog.com> wrote:
>
> APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all() were
> returning a mix of NULL and pointers with NULL being the way to
> "notify" that we should do a "system" lookup for channels. This make
> it very confusing and prone to errors as commit 9f63cc0921ec
> ("iio: inkern: fix return value in devm_of_iio_channel_get_by_name()")
> proves. On top of this, patterns like 'if (channel != NULL) return
> channel' were being used where channel could actually be an error code
> which makes the code hard to read.
>
> This change also makes some functional changes on how errors were being
> handled. In the original behavior, even if we get an error like '-ENOMEM',
> we still continue with the search. We should only continue to lookup for
> the channel when it makes sense to do so. Hence, the main error handling
> in 'of_iio_channel_get_by_name()' is changed to the following logic:
>
>  * If a channel 'name' is provided and we do find it via
> 'io-channel-names', we should be able to get it. If we get any error,
> we should not proceed with the lookup. Moreover, we should return an error
> so that callers won't proceed with a system lookup.
>  * If a channel 'name' is provided and we cannot find it ('index < 0'),
> 'of_parse_phandle_with_args()' is expected to fail with '-EINVAL'. Hence,
> we should only continue if we get that error.
>  * If a channel 'name' is not provided we should only carry on with the
> search if 'of_parse_phandle_with_args()' returns '-ENOENT'.
>
> Also note that a system channel lookup is only done if the returned
> error code (from 'of_iio_channel_get_by_name()' or
> 'of_iio_channel_get_all()' is -ENODEV.

LGTM (but I might miss something, it's a bit too many conditionals),
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/inkern.c | 54 +++++++++++++++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 87fd2a0d44f2..c6f1cfe09bd3 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -214,7 +214,7 @@ static struct iio_channel *of_iio_channel_get(struct device_node *np, int index)
>  struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>                                                const char *name)
>  {
> -       struct iio_channel *chan = NULL;
> +       struct iio_channel *chan;
>
>         /* Walk up the tree of devices looking for a matching iio channel */
>         while (np) {
> @@ -231,13 +231,33 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>                                                          name);
>                 chan = of_iio_channel_get(np, index);
>                 if (!IS_ERR(chan) || PTR_ERR(chan) == -EPROBE_DEFER)
> -                       break;
> -               else if (name && index >= 0) {
> -                       pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
> -                               np, name ? name : "", index);
> -                       return NULL;
> +                       return chan;
> +               if (name) {
> +                       if (index >= 0) {
> +                               pr_err("ERROR: could not get IIO channel %pOF:%s(%i)\n",
> +                                      np, name, index);
> +                               /*
> +                                * In this case, we found 'name' in 'io-channel-names'
> +                                * but somehow we still fail so that we should not proceed
> +                                * with any other lookup. Hence, explicitly return -EINVAL
> +                                * (maybe not the better error code) so that the caller
> +                                * won't do a system lookup.
> +                                */
> +                               return ERR_PTR(-EINVAL);
> +                       }
> +                       /* If index < 0, then of_parse_phandle_with_args() fails
> +                        * with -EINVAL which is expected. We should not proceed
> +                        * if we get any other error.
> +                        */
> +                       if (PTR_ERR(chan) != -EINVAL)
> +                               return chan;
> +               } else if (PTR_ERR(chan) != -ENOENT) {
> +                       /*
> +                        * if !name, then we should only proceed the lookup if
> +                        * of_parse_phandle_with_args() returns -ENOENT.
> +                        */
> +                       return chan;
>                 }
> -
>                 /*
>                  * No matching IIO channel found on this node.
>                  * If the parent node has a "io-channel-ranges" property,
> @@ -245,10 +265,10 @@ struct iio_channel *of_iio_channel_get_by_name(struct device_node *np,
>                  */
>                 np = np->parent;
>                 if (np && !of_get_property(np, "io-channel-ranges", NULL))
> -                       return NULL;
> +                       return ERR_PTR(-ENODEV);
>         }
>
> -       return chan;
> +       return ERR_PTR(-ENODEV);
>  }
>  EXPORT_SYMBOL_GPL(of_iio_channel_get_by_name);
>
> @@ -267,8 +287,8 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>                         break;
>         } while (++nummaps);
>
> -       if (nummaps == 0)       /* no error, return NULL to search map table */
> -               return NULL;
> +       if (nummaps == 0)
> +               return ERR_PTR(-ENODEV);
>
>         /* NULL terminated array to save passing size */
>         chans = kcalloc(nummaps + 1, sizeof(*chans), GFP_KERNEL);
> @@ -295,7 +315,7 @@ static struct iio_channel *of_iio_channel_get_all(struct device *dev)
>
>  static inline struct iio_channel *of_iio_channel_get_all(struct device *dev)
>  {
> -       return NULL;
> +       return ERR_PTR(-ENODEV);
>  }
>
>  #endif /* CONFIG_OF */
> @@ -362,7 +382,7 @@ struct iio_channel *iio_channel_get(struct device *dev,
>         if (dev) {
>                 channel = of_iio_channel_get_by_name(dev->of_node,
>                                                      channel_name);
> -               if (channel != NULL)
> +               if (!IS_ERR(channel) || PTR_ERR(channel) != -ENODEV)
>                         return channel;
>         }
>
> @@ -412,8 +432,6 @@ struct iio_channel *devm_of_iio_channel_get_by_name(struct device *dev,
>         channel = of_iio_channel_get_by_name(np, channel_name);
>         if (IS_ERR(channel))
>                 return channel;
> -       if (!channel)
> -               return ERR_PTR(-ENODEV);
>
>         ret = devm_add_action_or_reset(dev, devm_iio_channel_free, channel);
>         if (ret)
> @@ -436,7 +454,11 @@ struct iio_channel *iio_channel_get_all(struct device *dev)
>                 return ERR_PTR(-EINVAL);
>
>         chans = of_iio_channel_get_all(dev);
> -       if (chans)
> +       /*
> +        * We only want to carry on if the error is -ENODEV.  Anything else
> +        * should be reported up the stack.
> +        */
> +       if (!IS_ERR(chans) || PTR_ERR(chans) != -ENODEV)
>                 return chans;
>
>         name = dev_name(dev);
> --
> 2.37.0
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 3:28 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings
> > On Mon, Jul 11, 2022 at 2:40 PM Nuno Sá <nuno.sa@analog.com> wrote:

...

> > > -       if (name == NULL && channel_name == NULL)
> > > +       if (!name && !channel_name)
> > >                 return ERR_PTR(-ENODEV);
> >
> > After this change in place, I think it's better to convert it to
> >
> >   if (!(name || channel_name))
>
> It should be name && channel_name
>
> See below.
>  (! ( 1 || 0)->  (! (1 && 1)

I didn't get what you are implying here. Please, check again what's in
the original code and what's being suggested.

> > which shows intentions clearer.

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v2 15/15] iio: inkern: fix coding style warnings

[Biju Das]

Hi Andy,

> Subject: Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings
> 
> On Mon, Jul 11, 2022 at 3:28 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings
> > > On Mon, Jul 11, 2022 at 2:40 PM Nuno Sá <nuno.sa@analog.com> wrote:
> 
> ...
> 
> > > > -       if (name == NULL && channel_name == NULL)
> > > > +       if (!name && !channel_name)
> > > >                 return ERR_PTR(-ENODEV);
> > >
> > > After this change in place, I think it's better to convert it to
> > >
> > >   if (!(name || channel_name))
> >
> > It should be name && channel_name
> >
> > See below.
> >  (! ( 1 || 0)->  (! (1 && 1)
> 
> I didn't get what you are implying here. Please, check again what's in
> the original code and what's being suggested.

Ok, you are right. I was looking at the modified code.

Cheers,
Biju

Re: [PATCH v2 00/15] make iio inkern interface firmware agnostic

[Nuno Sá]

On Mon, 2022-07-11 at 15:22 +0200, Andy Shevchenko wrote:
> On Mon, Jul 11, 2022 at 2:38 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > 
> > First version of the series can be found here:
> > 
> > https://lore.kernel.org/linux-iio/20220610084545.547700-1-nuno.sa@analog.com/
> 
> I'm under the impression that I gave tags for some of these patches
> when they were the part of the bigger series. Am I wrong?
> In any case for patch 6-14,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Yes, you're right. Should I have dropped them?

- Nuno Sá

Re: [PATCH v2 13/15] iio: adc: stm32-adc: convert to device properties

[Fabrice Gasnier]

On 7/11/22 14:38, Nuno Sá wrote:
> Make the conversion to firmware agnostic device properties. As part of
> the conversion the IIO inkern interface 'of_xlate()' is also converted to
> 'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and hence OF
> dependencies from IIO.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  drivers/iio/adc/stm32-adc.c | 121 ++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 3985fe972892..e55859113df8 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -21,11 +21,11 @@
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/nvmem-consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> -#include <linux/of.h>
> -#include <linux/of_device.h>
> +#include <linux/property.h>
>  
>  #include "stm32-adc-core.h"
>  
> @@ -1530,8 +1530,8 @@ static int stm32_adc_update_scan_mode(struct iio_dev *indio_dev,
>  	return ret;
>  }
>  
> -static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> -			      const struct of_phandle_args *iiospec)
> +static int stm32_adc_fwnode_xlate(struct iio_dev *indio_dev,
> +				  const struct fwnode_reference_args *iiospec)
>  {
>  	int i;
>  
> @@ -1585,7 +1585,7 @@ static const struct iio_info stm32_adc_iio_info = {
>  	.hwfifo_set_watermark = stm32_adc_set_watermark,
>  	.update_scan_mode = stm32_adc_update_scan_mode,
>  	.debugfs_reg_access = stm32_adc_debugfs_reg_access,
> -	.of_xlate = stm32_adc_of_xlate,
> +	.fwnode_xlate = stm32_adc_fwnode_xlate,
>  };
>  
>  static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
> @@ -1782,14 +1782,14 @@ static const struct iio_chan_spec_ext_info stm32_adc_ext_info[] = {
>  	{},
>  };
>  
> -static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
> +static int stm32_adc_fw_get_resolution(struct iio_dev *indio_dev)
>  {
> -	struct device_node *node = indio_dev->dev.of_node;
> +	struct device *dev = &indio_dev->dev;
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	unsigned int i;
>  	u32 res;
>  
> -	if (of_property_read_u32(node, "assigned-resolution-bits", &res))
> +	if (device_property_read_u32(dev, "assigned-resolution-bits", &res))
>  		res = adc->cfg->adc_info->resolutions[0];
>  
>  	for (i = 0; i < adc->cfg->adc_info->num_res; i++)
> @@ -1873,11 +1873,11 @@ static void stm32_adc_chan_init_one(struct iio_dev *indio_dev,
>  
>  static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm32_adc *adc)
>  {
> -	struct device_node *node = indio_dev->dev.of_node;
> +	struct device *dev = &indio_dev->dev;
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>  	int num_channels = 0, ret;
>  
> -	ret = of_property_count_u32_elems(node, "st,adc-channels");
> +	ret = device_property_count_u32(dev, "st,adc-channels");
>  	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>  		return -EINVAL;
> @@ -1885,8 +1885,8 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
>  		num_channels += ret;
>  	}
>  
> -	ret = of_property_count_elems_of_size(node, "st,adc-diff-channels",
> -					      sizeof(struct stm32_adc_diff_channel));
> +	/* each st,adc-diff-channels is a group of 2 u32 */
> +	ret = device_property_count_u64(dev, "st,adc-diff-channels");
>  	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>  		return -EINVAL;
> @@ -1896,7 +1896,7 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
>  	}
>  
>  	/* Optional sample time is provided either for each, or all channels */
> -	ret = of_property_count_u32_elems(node, "st,min-sample-time-nsecs");
> +	ret = device_property_count_u32(dev, "st,min-sample-time-nsecs");
>  	if (ret > 1 && ret != num_channels) {
>  		dev_err(&indio_dev->dev, "Invalid st,min-sample-time-nsecs\n");
>  		return -EINVAL;
> @@ -1907,21 +1907,20 @@ static int stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct stm
>  
>  static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
>  				      struct stm32_adc *adc,
> -				      struct iio_chan_spec *channels)
> +				      struct iio_chan_spec *channels,
> +				      int nchans)
>  {
> -	struct device_node *node = indio_dev->dev.of_node;
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>  	struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
> +	struct device *dev = &indio_dev->dev;
>  	u32 num_diff = adc->num_diff;
>  	int size = num_diff * sizeof(*diff) / sizeof(u32);
> -	int scan_index = 0, val, ret, i;
> -	struct property *prop;
> -	const __be32 *cur;
> -	u32 smp = 0;
> +	int scan_index = 0, ret, i;
> +	u32 smp = 0, nsmps, smps[STM32_ADC_CH_MAX], chans[STM32_ADC_CH_MAX];
>  
>  	if (num_diff) {
> -		ret = of_property_read_u32_array(node, "st,adc-diff-channels",
> -						 (u32 *)diff, size);
> +		ret = device_property_read_u32_array(dev, "st,adc-diff-channels",
> +						     (u32 *)diff, size);
>  		if (ret) {
>  			dev_err(&indio_dev->dev, "Failed to get diff channels %d\n", ret);
>  			return ret;
> @@ -1942,32 +1941,51 @@ static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
>  		}
>  	}
>  
> -	of_property_for_each_u32(node, "st,adc-channels", prop, cur, val) {
> -		if (val >= adc_info->max_channels) {
> -			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
> +	ret = device_property_read_u32_array(dev, "st,adc-channels", chans,
> +					     nchans);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < nchans; i++) {

Hi Nuno,

There's an endless loop issue here due to the two for loops are using
the same index 'i'. (Please look at my earlier comments in v1)

for (i = 0; i < nchans; i++) {
	...
	// Here i gets cleared, num_diff may vary (example with 0 here)
	for (i = 0; i < num_diff; i++) {
		...
	}

	stm32_adc_chan_init_one(.. &channels[scan_index].. scan_index..)

	scan_index++; // Still, scan index gets incremented indefinitly
}

There's an out of boudary situation with scan_index above, wich ends-up
in crashing in stm32_adc_chan_init_one().

> +		if (chans[i] >= adc_info->max_channels) {
> +			dev_err(&indio_dev->dev, "Invalid channel %d\n",
> +				chans[i]);
>  			return -EINVAL;
>  		}
>  
>  		/* Channel can't be configured both as single-ended & diff */
>  		for (i = 0; i < num_diff; i++) {
> -			if (val == diff[i].vinp) {
> -				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	val);
> +			if (chans[i] == diff[i].vinp) {
> +				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	chans[i]);
>  				return -EINVAL;
>  			}
>  		}
> -		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
> -					0, scan_index, false);
> +		stm32_adc_chan_init_one(indio_dev, &channels[scan_index],
> +					chans[i], 0, scan_index, false);
>  		scan_index++;
>  	}
>  
> +	nsmps = device_property_count_u32(dev, "st,min-sample-time-nsecs");
> +	if (nsmps) {
> +		if (nsmps >= nchans)

nit: if (nsmps > nchans)

> +			nsmps = nchans;
> +

There's a bit of redundancy in checking nsmps,
"st,min-sample-time-nsecs" is already sanitized in
stm32_adc_get_legacy_chan_count():

/* Optional sample time is provided either for each, or all channels */
ret = device_property_count_u32(dev, "st,min-sample-time-nsecs");
if (ret > 1 && ret != num_channels) {
	dev_err(...

So just sharing my thoughts here:
- Maybe this could be dropped ?
  (Thinking loudly) The earliest this gets sanitized, the less un-needed
initialisations happen before failing?
- Or the earlier check could be moved here ?

I've no strong opinion.

Best Regards,
Fabrice

> +		ret = device_property_read_u32_array(dev, "st,min-sample-time-nsecs",
> +						     smps, nsmps);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	for (i = 0; i < scan_index; i++) {
>  		/*
> -		 * Using of_property_read_u32_index(), smp value will only be
> -		 * modified if valid u32 value can be decoded. This allows to
> -		 * get either no value, 1 shared value for all indexes, or one
> -		 * value per channel.
> +		 * This check is used with the above logic so that smp value
> +		 * will only be modified if valid u32 value can be decoded. This
> +		 * allows to get either no value, 1 shared value for all indexes,
> +		 * or one value per channel. The point is to have the same
> +		 * behavior as 'of_property_read_u32_index()'.
>  		 */
> -		of_property_read_u32_index(node, "st,min-sample-time-nsecs", i, &smp);
> +		if (i < nsmps)
> +			smp = smps[i];
>  
>  		/* Prepare sampling time settings */
>  		stm32_adc_smpr_init(adc, channels[i].channel, smp);
> @@ -2010,22 +2028,21 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
>  				       struct stm32_adc *adc,
>  				       struct iio_chan_spec *channels)
>  {
> -	struct device_node *node = indio_dev->dev.of_node;
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> -	struct device_node *child;
> +	struct fwnode_handle *child;
>  	const char *name;
>  	int val, scan_index = 0, ret;
>  	bool differential;
>  	u32 vin[2];
>  
> -	for_each_available_child_of_node(node, child) {
> -		ret = of_property_read_u32(child, "reg", &val);
> +	device_for_each_child_node(&indio_dev->dev, child) {
> +		ret = fwnode_property_read_u32(child, "reg", &val);
>  		if (ret) {
>  			dev_err(&indio_dev->dev, "Missing channel index %d\n", ret);
>  			goto err;
>  		}
>  
> -		ret = of_property_read_string(child, "label", &name);
> +		ret = fwnode_property_read_string(child, "label", &name);
>  		/* label is optional */
>  		if (!ret) {
>  			if (strlen(name) >= STM32_ADC_CH_SZ) {
> @@ -2050,7 +2067,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
>  		}
>  
>  		differential = false;
> -		ret = of_property_read_u32_array(child, "diff-channels", vin, 2);
> +		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);
>  		/* diff-channels is optional */
>  		if (!ret) {
>  			differential = true;
> @@ -2067,7 +2084,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
>  		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
>  					vin[1], scan_index, differential);
>  
> -		ret = of_property_read_u32(child, "st,min-sample-time-ns", &val);
> +		ret = fwnode_property_read_u32(child, "st,min-sample-time-ns", &val);
>  		/* st,min-sample-time-ns is optional */
>  		if (!ret) {
>  			stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
> @@ -2085,14 +2102,13 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
>  	return scan_index;
>  
>  err:
> -	of_node_put(child);
> +	fwnode_handle_put(child);
>  
>  	return ret;
>  }
>  
> -static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
> +static int stm32_adc_chan_fw_init(struct iio_dev *indio_dev, bool timestamping)
>  {
> -	struct device_node *node = indio_dev->dev.of_node;
>  	struct stm32_adc *adc = iio_priv(indio_dev);
>  	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>  	struct iio_chan_spec *channels;
> @@ -2102,7 +2118,7 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
>  	for (i = 0; i < STM32_ADC_INT_CH_NB; i++)
>  		adc->int_ch[i] = STM32_ADC_INT_CH_NONE;
>  
> -	num_channels = of_get_available_child_count(node);
> +	num_channels = device_get_child_node_count(&indio_dev->dev);
>  	/* If no channels have been found, fallback to channels legacy properties. */
>  	if (!num_channels) {
>  		legacy = true;
> @@ -2133,7 +2149,8 @@ static int stm32_adc_chan_of_init(struct iio_dev *indio_dev, bool timestamping)
>  		return -ENOMEM;
>  
>  	if (legacy)
> -		ret = stm32_adc_legacy_chan_init(indio_dev, adc, channels);
> +		ret = stm32_adc_legacy_chan_init(indio_dev, adc, channels,
> +						 num_channels);
>  	else
>  		ret = stm32_adc_generic_chan_init(indio_dev, adc, channels);
>  	if (ret < 0)
> @@ -2215,9 +2232,6 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	bool timestamping = false;
>  	int ret;
>  
> -	if (!pdev->dev.of_node)
> -		return -ENODEV;
> -
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*adc));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -2226,17 +2240,16 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  	adc->common = dev_get_drvdata(pdev->dev.parent);
>  	spin_lock_init(&adc->lock);
>  	init_completion(&adc->completion);
> -	adc->cfg = (const struct stm32_adc_cfg *)
> -		of_match_device(dev->driver->of_match_table, dev)->data;
> +	adc->cfg = device_get_match_data(dev);
>  
>  	indio_dev->name = dev_name(&pdev->dev);
> -	indio_dev->dev.of_node = pdev->dev.of_node;
> +	device_set_node(&indio_dev->dev, dev_fwnode(&pdev->dev));
>  	indio_dev->info = &stm32_adc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_HARDWARE_TRIGGERED;
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> -	ret = of_property_read_u32(pdev->dev.of_node, "reg", &adc->offset);
> +	ret = device_property_read_u32(dev, "reg", &adc->offset);
>  	if (ret != 0) {
>  		dev_err(&pdev->dev, "missing reg property\n");
>  		return -EINVAL;
> @@ -2265,7 +2278,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	ret = stm32_adc_of_get_resolution(indio_dev);
> +	ret = stm32_adc_fw_get_resolution(indio_dev);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -2282,7 +2295,7 @@ static int stm32_adc_probe(struct platform_device *pdev)
>  		timestamping = true;
>  	}
>  
> -	ret = stm32_adc_chan_of_init(indio_dev, timestamping);
> +	ret = stm32_adc_chan_fw_init(indio_dev, timestamping);
>  	if (ret < 0)
>  		goto err_dma_disable;
>  

Re: [PATCH v2 15/15] iio: inkern: fix coding style warnings

[Nuno Sá]

On Mon, 2022-07-11 at 15:15 +0200, Andy Shevchenko wrote:
> On Mon, Jul 11, 2022 at 2:40 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > 
> > Just cosmetics. No functional change intended...
> 
> ...
> 
> > -       if (name == NULL && channel_name == NULL)
> > +       if (!name && !channel_name)
> >                 return ERR_PTR(-ENODEV);
> 
> After this change in place, I think it's better to convert it to
> 
>   if (!(name || channel_name))
> 
> which shows intentions clearer.
> 

No strong feelings from my side so I can do that if a v3 is needed
(which, most likely, will be needed).

- Nuno Sá

Re: [PATCH v2 03/15] iio: inkern: only return error codes in iio_channel_get_*() APIs

[Nuno Sá]

On Mon, 2022-07-11 at 15:29 +0200, Andy Shevchenko wrote:
> On Mon, Jul 11, 2022 at 2:38 PM Nuno Sá <nuno.sa@analog.com> wrote:
> > 
> > APIs like of_iio_channel_get_by_name() and of_iio_channel_get_all()
> > were
> > returning a mix of NULL and pointers with NULL being the way to
> > "notify" that we should do a "system" lookup for channels. This
> > make
> > it very confusing and prone to errors as commit 9f63cc0921ec
> > ("iio: inkern: fix return value in
> > devm_of_iio_channel_get_by_name()")
> > proves. On top of this, patterns like 'if (channel != NULL) return
> > channel' were being used where channel could actually be an error
> > code
> > which makes the code hard to read.
> > 
> > This change also makes some functional changes on how errors were
> > being
> > handled. In the original behavior, even if we get an error like '-
> > ENOMEM',
> > we still continue with the search. We should only continue to
> > lookup for
> > the channel when it makes sense to do so. Hence, the main error
> > handling
> > in 'of_iio_channel_get_by_name()' is changed to the following
> > logic:
> > 
> >  * If a channel 'name' is provided and we do find it via
> > 'io-channel-names', we should be able to get it. If we get any
> > error,
> > we should not proceed with the lookup. Moreover, we should return
> > an error
> > so that callers won't proceed with a system lookup.
> >  * If a channel 'name' is provided and we cannot find it ('index <
> > 0'),
> > 'of_parse_phandle_with_args()' is expected to fail with '-EINVAL'.
> > Hence,
> > we should only continue if we get that error.
> >  * If a channel 'name' is not provided we should only carry on with
> > the
> > search if 'of_parse_phandle_with_args()' returns '-ENOENT'.
> > 
> > Also note that a system channel lookup is only done if the returned
> > error code (from 'of_iio_channel_get_by_name()' or
> > 'of_iio_channel_get_all()' is -ENODEV.
> 
> LGTM (but I might miss something, it's a bit too many conditionals),
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 

Agreed. It ended up being more complicated than I thought...

- Nuno Sá

Re: [PATCH v2 13/15] iio: adc: stm32-adc: convert to device properties

[Nuno Sá]

Hi Fabrice,

Nice that someone in ST is looking at this one :)

On Mon, 2022-07-11 at 16:04 +0200, Fabrice Gasnier wrote:
> On 7/11/22 14:38, Nuno Sá wrote:
> > Make the conversion to firmware agnostic device properties. As part
> > of
> > the conversion the IIO inkern interface 'of_xlate()' is also
> > converted to
> > 'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and
> > hence OF
> > dependencies from IIO.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/iio/adc/stm32-adc.c | 121 ++++++++++++++++++++------------
> > ----
> >  1 file changed, 67 insertions(+), 54 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-
> > adc.c
> > index 3985fe972892..e55859113df8 100644
> > --- a/drivers/iio/adc/stm32-adc.c
> > +++ b/drivers/iio/adc/stm32-adc.c
> > @@ -21,11 +21,11 @@
> >  #include <linux/io.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/module.h>
> > +#include <linux/mod_devicetable.h>
> >  #include <linux/nvmem-consumer.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_runtime.h>
> > -#include <linux/of.h>
> > -#include <linux/of_device.h>
> > +#include <linux/property.h>
> >  
> >  #include "stm32-adc-core.h"
> >  
> > @@ -1530,8 +1530,8 @@ static int stm32_adc_update_scan_mode(struct
> > iio_dev *indio_dev,
> >         return ret;
> >  }
> >  
> > -static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> > -                             const struct of_phandle_args
> > *iiospec)
> > +static int stm32_adc_fwnode_xlate(struct iio_dev *indio_dev,
> > +                                 const struct
> > fwnode_reference_args *iiospec)
> >  {
> >         int i;
> >  
> > @@ -1585,7 +1585,7 @@ static const struct iio_info
> > stm32_adc_iio_info = {
> >         .hwfifo_set_watermark = stm32_adc_set_watermark,
> >         .update_scan_mode = stm32_adc_update_scan_mode,
> >         .debugfs_reg_access = stm32_adc_debugfs_reg_access,
> > -       .of_xlate = stm32_adc_of_xlate,
> > +       .fwnode_xlate = stm32_adc_fwnode_xlate,
> >  };
> >  
> >  static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
> > @@ -1782,14 +1782,14 @@ static const struct iio_chan_spec_ext_info
> > stm32_adc_ext_info[] = {
> >         {},
> >  };
> >  
> > -static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
> > +static int stm32_adc_fw_get_resolution(struct iio_dev *indio_dev)
> >  {
> > -       struct device_node *node = indio_dev->dev.of_node;
> > +       struct device *dev = &indio_dev->dev;
> >         struct stm32_adc *adc = iio_priv(indio_dev);
> >         unsigned int i;
> >         u32 res;
> >  
> > -       if (of_property_read_u32(node, "assigned-resolution-bits",
> > &res))
> > +       if (device_property_read_u32(dev, "assigned-resolution-
> > bits", &res))
> >                 res = adc->cfg->adc_info->resolutions[0];
> >  
> >         for (i = 0; i < adc->cfg->adc_info->num_res; i++)
> > @@ -1873,11 +1873,11 @@ static void stm32_adc_chan_init_one(struct
> > iio_dev *indio_dev,
> >  
> >  static int stm32_adc_get_legacy_chan_count(struct iio_dev
> > *indio_dev, struct stm32_adc *adc)
> >  {
> > -       struct device_node *node = indio_dev->dev.of_node;
> > +       struct device *dev = &indio_dev->dev;
> >         const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> >         int num_channels = 0, ret;
> >  
> > -       ret = of_property_count_u32_elems(node, "st,adc-channels");
> > +       ret = device_property_count_u32(dev, "st,adc-channels");
> >         if (ret > adc_info->max_channels) {
> >                 dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
> >                 return -EINVAL;
> > @@ -1885,8 +1885,8 @@ static int
> > stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
> > stm
> >                 num_channels += ret;
> >         }
> >  
> > -       ret = of_property_count_elems_of_size(node, "st,adc-diff-
> > channels",
> > -                                             sizeof(struct
> > stm32_adc_diff_channel));
> > +       /* each st,adc-diff-channels is a group of 2 u32 */
> > +       ret = device_property_count_u64(dev, "st,adc-diff-
> > channels");

Are you fine with this change (still does not have any reference to the
target struct but the comment might be helpful and there's no magic 2)?

> >         if (ret > adc_info->max_channels) {
> >                 dev_err(&indio_dev->dev, "Bad st,adc-diff-
> > channels?\n");
> >                 return -EINVAL;
> > @@ -1896,7 +1896,7 @@ static int
> > stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
> > stm
> >         }
> >  
> >         /* Optional sample time is provided either for each, or all
> > channels */
> > -       ret = of_property_count_u32_elems(node, "st,min-sample-
> > time-nsecs");
> > +       ret = device_property_count_u32(dev, "st,min-sample-time-
> > nsecs");
> >         if (ret > 1 && ret != num_channels) {
> >                 dev_err(&indio_dev->dev, "Invalid st,min-sample-
> > time-nsecs\n");
> >                 return -EINVAL;
> > @@ -1907,21 +1907,20 @@ static int
> > stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
> > stm
> >  
> >  static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
> >                                       struct stm32_adc *adc,
> > -                                     struct iio_chan_spec
> > *channels)
> > +                                     struct iio_chan_spec
> > *channels,
> > +                                     int nchans)
> >  {
> > -       struct device_node *node = indio_dev->dev.of_node;
> >         const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
> >         struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
> > +       struct device *dev = &indio_dev->dev;
> >         u32 num_diff = adc->num_diff;
> >         int size = num_diff * sizeof(*diff) / sizeof(u32);
> > -       int scan_index = 0, val, ret, i;
> > -       struct property *prop;
> > -       const __be32 *cur;
> > -       u32 smp = 0;
> > +       int scan_index = 0, ret, i;
> > +       u32 smp = 0, nsmps, smps[STM32_ADC_CH_MAX],
> > chans[STM32_ADC_CH_MAX];
> >  
> >         if (num_diff) {
> > -               ret = of_property_read_u32_array(node, "st,adc-
> > diff-channels",
> > -                                                (u32 *)diff,
> > size);
> > +               ret = device_property_read_u32_array(dev, "st,adc-
> > diff-channels",
> > +                                                    (u32 *)diff,
> > size);
> >                 if (ret) {
> >                         dev_err(&indio_dev->dev, "Failed to get
> > diff channels %d\n", ret);
> >                         return ret;
> > @@ -1942,32 +1941,51 @@ static int
> > stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
> >                 }
> >         }
> >  
> > -       of_property_for_each_u32(node, "st,adc-channels", prop,
> > cur, val) {
> > -               if (val >= adc_info->max_channels) {
> > -                       dev_err(&indio_dev->dev, "Invalid channel
> > %d\n", val);
> > +       ret = device_property_read_u32_array(dev, "st,adc-
> > channels", chans,
> > +                                            nchans);
> > +       if (ret)
> > +               return ret;
> > +
> > +       for (i = 0; i < nchans; i++) {
> 
> Hi Nuno,
> 
> There's an endless loop issue here due to the two for loops are using
> the same index 'i'. (Please look at my earlier comments in v1)
> 

Ouch... 

Regarding your v1 comments (that for some reason I missed), I do not
think there's any helper like "of_property_for_each_u32()". For now we
have to live with this...

> for (i = 0; i < nchans; i++) {
>         ...
>         // Here i gets cleared, num_diff may vary (example with 0
> here)
>         for (i = 0; i < num_diff; i++) {
>                 ...
>         }
> 
>         stm32_adc_chan_init_one(.. &channels[scan_index]..
> scan_index..)
> 
>         scan_index++; // Still, scan index gets incremented
> indefinitly
> }
> 
> There's an out of boudary situation with scan_index above, wich ends-
> up
> in crashing in stm32_adc_chan_init_one().
> 
> > +               if (chans[i] >= adc_info->max_channels) {
> > +                       dev_err(&indio_dev->dev, "Invalid channel
> > %d\n",
> > +                               chans[i]);
> >                         return -EINVAL;
> >                 }
> >  
> >                 /* Channel can't be configured both as single-ended
> > & diff */
> >                 for (i = 0; i < num_diff; i++) {
> > -                       if (val == diff[i].vinp) {
> > -                               dev_err(&indio_dev->dev, "channel
> > %d misconfigured\n",  val);
> > +                       if (chans[i] == diff[i].vinp) {
> > +                               dev_err(&indio_dev->dev, "channel
> > %d misconfigured\n",  chans[i]);
> >                                 return -EINVAL;
> >                         }
> >                 }
> > -               stm32_adc_chan_init_one(indio_dev,
> > &channels[scan_index], val,
> > -                                       0, scan_index, false);
> > +               stm32_adc_chan_init_one(indio_dev,
> > &channels[scan_index],
> > +                                       chans[i], 0, scan_index,
> > false);
> >                 scan_index++;
> >         }
> >  
> > +       nsmps = device_property_count_u32(dev, "st,min-sample-time-
> > nsecs");
> > +       if (nsmps) {
> > +               if (nsmps >= nchans)
> 
> nit: if (nsmps > nchans)
> 
> > +                       nsmps = nchans;
> > +
> 
> There's a bit of redundancy in checking nsmps,
> "st,min-sample-time-nsecs" is already sanitized in
> stm32_adc_get_legacy_chan_count():
> 
> /* Optional sample time is provided either for each, or all channels
> */
> ret = device_property_count_u32(dev, "st,min-sample-time-nsecs");
> if (ret > 1 && ret != num_channels) {
>         dev_err(...
> 
> So just sharing my thoughts here:
> - Maybe this could be dropped ?
>   (Thinking loudly) The earliest this gets sanitized, the less un-
> needed
> initialisations happen before failing?
> - Or the earlier check could be moved here ?
> 
> I've no strong opinion.
> 

Ahh, didn't noticed the first call to 'device_property_count_u32(dev,
"st,min-sample-time-nsecs")'. So I do agree with you that we should
bail as soon as possible but I'm also not a big fan of calling
device_property_count_u32() for the same purpose. So I would suggest:

1) Your option 2.
2) Option 1 but we would store 'nsmps' in the first call (making it a
member of struct stm32_adc) and then use it here accordingly.

Thought?

> Best Regards,
> Fabrice
> 
> > +               ret = device_property_read_u32_array(dev, "st,min-
> > sample-time-nsecs",
> > +                                                    smps, nsmps);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >         for (i = 0; i < scan_index; i++) {
> >                 /*
> > -                * Using of_property_read_u32_index(), smp value
> > will only be
> > -                * modified if valid u32 value can be decoded. This
> > allows to
> > -                * get either no value, 1 shared value for all
> > indexes, or one
> > -                * value per channel.
> > +                * This check is used with the above logic so that
> > smp value
> > +                * will only be modified if valid u32 value can be
> > decoded. This
> > +                * allows to get either no value, 1 shared value
> > for all indexes,
> > +                * or one value per channel. The point is to have
> > the same
> > +                * behavior as 'of_property_read_u32_index()'.
> >                  */
> > -               of_property_read_u32_index(node, "st,min-sample-
> > time-nsecs", i, &smp);
> > +               if (i < nsmps)
> > +                       smp = smps[i];
> >  

Regarding your comment on v1, I would prefer to keep this as-is. I
think it's enough together with the comment. Can change it though if
you fell too strong about it :)

Thanks!
- Nuno Sá

Re: [PATCH v2 13/15] iio: adc: stm32-adc: convert to device properties

[Fabrice Gasnier]

On 7/12/22 12:33, Nuno Sá wrote:
> Hi Fabrice,
> 
> Nice that someone in ST is looking at this one :)

Hi Nuno,

Thank you for taking care of converting all these drivers to device
properties, including this one :-).

> 
> On Mon, 2022-07-11 at 16:04 +0200, Fabrice Gasnier wrote:
>> On 7/11/22 14:38, Nuno Sá wrote:
>>> Make the conversion to firmware agnostic device properties. As part
>>> of
>>> the conversion the IIO inkern interface 'of_xlate()' is also
>>> converted to
>>> 'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and
>>> hence OF
>>> dependencies from IIO.
>>>
>>> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
>>> ---
>>>  drivers/iio/adc/stm32-adc.c | 121 ++++++++++++++++++++------------
>>> ----
>>>  1 file changed, 67 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-
>>> adc.c
>>> index 3985fe972892..e55859113df8 100644
>>> --- a/drivers/iio/adc/stm32-adc.c
>>> +++ b/drivers/iio/adc/stm32-adc.c
>>> @@ -21,11 +21,11 @@
>>>  #include <linux/io.h>
>>>  #include <linux/iopoll.h>
>>>  #include <linux/module.h>
>>> +#include <linux/mod_devicetable.h>
>>>  #include <linux/nvmem-consumer.h>
>>>  #include <linux/platform_device.h>
>>>  #include <linux/pm_runtime.h>
>>> -#include <linux/of.h>
>>> -#include <linux/of_device.h>
>>> +#include <linux/property.h>
>>>  
>>>  #include "stm32-adc-core.h"
>>>  
>>> @@ -1530,8 +1530,8 @@ static int stm32_adc_update_scan_mode(struct
>>> iio_dev *indio_dev,
>>>         return ret;
>>>  }
>>>  
>>> -static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
>>> -                             const struct of_phandle_args
>>> *iiospec)
>>> +static int stm32_adc_fwnode_xlate(struct iio_dev *indio_dev,
>>> +                                 const struct
>>> fwnode_reference_args *iiospec)
>>>  {
>>>         int i;
>>>  
>>> @@ -1585,7 +1585,7 @@ static const struct iio_info
>>> stm32_adc_iio_info = {
>>>         .hwfifo_set_watermark = stm32_adc_set_watermark,
>>>         .update_scan_mode = stm32_adc_update_scan_mode,
>>>         .debugfs_reg_access = stm32_adc_debugfs_reg_access,
>>> -       .of_xlate = stm32_adc_of_xlate,
>>> +       .fwnode_xlate = stm32_adc_fwnode_xlate,
>>>  };
>>>  
>>>  static unsigned int stm32_adc_dma_residue(struct stm32_adc *adc)
>>> @@ -1782,14 +1782,14 @@ static const struct iio_chan_spec_ext_info
>>> stm32_adc_ext_info[] = {
>>>         {},
>>>  };
>>>  
>>> -static int stm32_adc_of_get_resolution(struct iio_dev *indio_dev)
>>> +static int stm32_adc_fw_get_resolution(struct iio_dev *indio_dev)
>>>  {
>>> -       struct device_node *node = indio_dev->dev.of_node;
>>> +       struct device *dev = &indio_dev->dev;
>>>         struct stm32_adc *adc = iio_priv(indio_dev);
>>>         unsigned int i;
>>>         u32 res;
>>>  
>>> -       if (of_property_read_u32(node, "assigned-resolution-bits",
>>> &res))
>>> +       if (device_property_read_u32(dev, "assigned-resolution-
>>> bits", &res))
>>>                 res = adc->cfg->adc_info->resolutions[0];
>>>  
>>>         for (i = 0; i < adc->cfg->adc_info->num_res; i++)
>>> @@ -1873,11 +1873,11 @@ static void stm32_adc_chan_init_one(struct
>>> iio_dev *indio_dev,
>>>  
>>>  static int stm32_adc_get_legacy_chan_count(struct iio_dev
>>> *indio_dev, struct stm32_adc *adc)
>>>  {
>>> -       struct device_node *node = indio_dev->dev.of_node;
>>> +       struct device *dev = &indio_dev->dev;
>>>         const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>>>         int num_channels = 0, ret;
>>>  
>>> -       ret = of_property_count_u32_elems(node, "st,adc-channels");
>>> +       ret = device_property_count_u32(dev, "st,adc-channels");
>>>         if (ret > adc_info->max_channels) {
>>>                 dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>>>                 return -EINVAL;
>>> @@ -1885,8 +1885,8 @@ static int
>>> stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
>>> stm
>>>                 num_channels += ret;
>>>         }
>>>  
>>> -       ret = of_property_count_elems_of_size(node, "st,adc-diff-
>>> channels",
>>> -                                             sizeof(struct
>>> stm32_adc_diff_channel));
>>> +       /* each st,adc-diff-channels is a group of 2 u32 */
>>> +       ret = device_property_count_u64(dev, "st,adc-diff-
>>> channels");
> 
> Are you fine with this change (still does not have any reference to the
> target struct but the comment might be helpful and there's no magic 2)?


Since you added that comment, this sounds better. IMHO, This still looks
a bit weird. I'd feel more comfortable by using u32 API for a
'uint32-matrix' as defined in dt-bindings.
Strictly speaking, something like
sizeof(struct stm32_adc_diff_channel) / sizeof(u32) could be used, along
with this comment and device_property_count_u32() to make it clear ?

Maybe Jonathan has an opinion, as he first raised a comment here ?

> 
>>>         if (ret > adc_info->max_channels) {
>>>                 dev_err(&indio_dev->dev, "Bad st,adc-diff-
>>> channels?\n");
>>>                 return -EINVAL;
>>> @@ -1896,7 +1896,7 @@ static int
>>> stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
>>> stm
>>>         }
>>>  
>>>         /* Optional sample time is provided either for each, or all
>>> channels */
>>> -       ret = of_property_count_u32_elems(node, "st,min-sample-
>>> time-nsecs");
>>> +       ret = device_property_count_u32(dev, "st,min-sample-time-
>>> nsecs");
>>>         if (ret > 1 && ret != num_channels) {
>>>                 dev_err(&indio_dev->dev, "Invalid st,min-sample-
>>> time-nsecs\n");
>>>                 return -EINVAL;
>>> @@ -1907,21 +1907,20 @@ static int
>>> stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev, struct
>>> stm
>>>  
>>>  static int stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
>>>                                       struct stm32_adc *adc,
>>> -                                     struct iio_chan_spec
>>> *channels)
>>> +                                     struct iio_chan_spec
>>> *channels,
>>> +                                     int nchans)
>>>  {
>>> -       struct device_node *node = indio_dev->dev.of_node;
>>>         const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
>>>         struct stm32_adc_diff_channel diff[STM32_ADC_CH_MAX];
>>> +       struct device *dev = &indio_dev->dev;
>>>         u32 num_diff = adc->num_diff;
>>>         int size = num_diff * sizeof(*diff) / sizeof(u32);
>>> -       int scan_index = 0, val, ret, i;
>>> -       struct property *prop;
>>> -       const __be32 *cur;
>>> -       u32 smp = 0;
>>> +       int scan_index = 0, ret, i;
>>> +       u32 smp = 0, nsmps, smps[STM32_ADC_CH_MAX],
>>> chans[STM32_ADC_CH_MAX];
>>>  
>>>         if (num_diff) {
>>> -               ret = of_property_read_u32_array(node, "st,adc-
>>> diff-channels",
>>> -                                                (u32 *)diff,
>>> size);
>>> +               ret = device_property_read_u32_array(dev, "st,adc-
>>> diff-channels",
>>> +                                                    (u32 *)diff,
>>> size);
>>>                 if (ret) {
>>>                         dev_err(&indio_dev->dev, "Failed to get
>>> diff channels %d\n", ret);
>>>                         return ret;
>>> @@ -1942,32 +1941,51 @@ static int
>>> stm32_adc_legacy_chan_init(struct iio_dev *indio_dev,
>>>                 }
>>>         }
>>>  
>>> -       of_property_for_each_u32(node, "st,adc-channels", prop,
>>> cur, val) {
>>> -               if (val >= adc_info->max_channels) {
>>> -                       dev_err(&indio_dev->dev, "Invalid channel
>>> %d\n", val);
>>> +       ret = device_property_read_u32_array(dev, "st,adc-
>>> channels", chans,
>>> +                                            nchans);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       for (i = 0; i < nchans; i++) {
>>
>> Hi Nuno,
>>
>> There's an endless loop issue here due to the two for loops are using
>> the same index 'i'. (Please look at my earlier comments in v1)
>>
> 
> Ouch... 
> 
> Regarding your v1 comments (that for some reason I missed), I do not
> think there's any helper like "of_property_for_each_u32()". For now we
> have to live with this...

Indeed, still need to fix this for loop.

> 
>> for (i = 0; i < nchans; i++) {
>>         ...
>>         // Here i gets cleared, num_diff may vary (example with 0
>> here)
>>         for (i = 0; i < num_diff; i++) {
>>                 ...
>>         }
>>
>>         stm32_adc_chan_init_one(.. &channels[scan_index]..
>> scan_index..)
>>
>>         scan_index++; // Still, scan index gets incremented
>> indefinitly
>> }
>>
>> There's an out of boudary situation with scan_index above, wich ends-
>> up
>> in crashing in stm32_adc_chan_init_one().
>>
>>> +               if (chans[i] >= adc_info->max_channels) {
>>> +                       dev_err(&indio_dev->dev, "Invalid channel
>>> %d\n",
>>> +                               chans[i]);
>>>                         return -EINVAL;
>>>                 }
>>>  
>>>                 /* Channel can't be configured both as single-ended
>>> & diff */
>>>                 for (i = 0; i < num_diff; i++) {
>>> -                       if (val == diff[i].vinp) {
>>> -                               dev_err(&indio_dev->dev, "channel
>>> %d misconfigured\n",  val);
>>> +                       if (chans[i] == diff[i].vinp) {
>>> +                               dev_err(&indio_dev->dev, "channel
>>> %d misconfigured\n",  chans[i]);
>>>                                 return -EINVAL;
>>>                         }
>>>                 }
>>> -               stm32_adc_chan_init_one(indio_dev,
>>> &channels[scan_index], val,
>>> -                                       0, scan_index, false);
>>> +               stm32_adc_chan_init_one(indio_dev,
>>> &channels[scan_index],
>>> +                                       chans[i], 0, scan_index,
>>> false);
>>>                 scan_index++;
>>>         }
>>>  
>>> +       nsmps = device_property_count_u32(dev, "st,min-sample-time-
>>> nsecs");
>>> +       if (nsmps) {
>>> +               if (nsmps >= nchans)
>>
>> nit: if (nsmps > nchans)
>>
>>> +                       nsmps = nchans;
>>> +
>>
>> There's a bit of redundancy in checking nsmps,
>> "st,min-sample-time-nsecs" is already sanitized in
>> stm32_adc_get_legacy_chan_count():
>>
>> /* Optional sample time is provided either for each, or all channels
>> */
>> ret = device_property_count_u32(dev, "st,min-sample-time-nsecs");
>> if (ret > 1 && ret != num_channels) {
>>         dev_err(...
>>
>> So just sharing my thoughts here:
>> - Maybe this could be dropped ?
>>   (Thinking loudly) The earliest this gets sanitized, the less un-
>> needed
>> initialisations happen before failing?
>> - Or the earlier check could be moved here ?
>>
>> I've no strong opinion.
>>
> 
> Ahh, didn't noticed the first call to 'device_property_count_u32(dev,
> "st,min-sample-time-nsecs")'. So I do agree with you that we should
> bail as soon as possible but I'm also not a big fan of calling
> device_property_count_u32() for the same purpose. So I would suggest:
> 
> 1) Your option 2.
> 2) Option 1 but we would store 'nsmps' in the first call (making it a
> member of struct stm32_adc) and then use it here accordingly.
> 
> Thought?

I'd prefer Your option 2) to follow the logic of the current code (e.g.
get count then init chans). It has the advantage to tackle errors as
soon as possible, and store 'nsmps' at once, when sanitized. Its avoids
redundancy here, only cost is an extra variable indeed.

Best Regards,
Fabrice

> 
>> Best Regards,
>> Fabrice
>>
>>> +               ret = device_property_read_u32_array(dev, "st,min-
>>> sample-time-nsecs",
>>> +                                                    smps, nsmps);
>>> +               if (ret)
>>> +                       return ret;
>>> +       }
>>> +
>>>         for (i = 0; i < scan_index; i++) {
>>>                 /*
>>> -                * Using of_property_read_u32_index(), smp value
>>> will only be
>>> -                * modified if valid u32 value can be decoded. This
>>> allows to
>>> -                * get either no value, 1 shared value for all
>>> indexes, or one
>>> -                * value per channel.
>>> +                * This check is used with the above logic so that
>>> smp value
>>> +                * will only be modified if valid u32 value can be
>>> decoded. This
>>> +                * allows to get either no value, 1 shared value
>>> for all indexes,
>>> +                * or one value per channel. The point is to have
>>> the same
>>> +                * behavior as 'of_property_read_u32_index()'.
>>>                  */
>>> -               of_property_read_u32_index(node, "st,min-sample-
>>> time-nsecs", i, &smp);
>>> +               if (i < nsmps)
>>> +                       smp = smps[i];
>>>  
> 
> Regarding your comment on v1, I would prefer to keep this as-is. I
> think it's enough together with the comment. Can change it though if
> you fell too strong about it :)
> 
> Thanks!
> - Nuno Sá
> 

Re: [PATCH v2 13/15] iio: adc: stm32-adc: convert to device properties

[Nuno Sá]

On Wed, 2022-07-13 at 17:48 +0200, Fabrice Gasnier wrote:
> On 7/12/22 12:33, Nuno Sá wrote:
> > Hi Fabrice,
> > 
> > Nice that someone in ST is looking at this one :)
> 
> Hi Nuno,
> 
> Thank you for taking care of converting all these drivers to device
> properties, including this one :-).
> 
> > 
> > On Mon, 2022-07-11 at 16:04 +0200, Fabrice Gasnier wrote:
> > > On 7/11/22 14:38, Nuno Sá wrote:
> > > > Make the conversion to firmware agnostic device properties. As
> > > > part
> > > > of
> > > > the conversion the IIO inkern interface 'of_xlate()' is also
> > > > converted to
> > > > 'fwnode_xlate()'. The goal is to completely drop 'of_xlate' and
> > > > hence OF
> > > > dependencies from IIO.
> > > > 
> > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > > > ---
> > > >  drivers/iio/adc/stm32-adc.c | 121 ++++++++++++++++++++--------
> > > > ----
> > > > ----
> > > >  1 file changed, 67 insertions(+), 54 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/adc/stm32-adc.c
> > > > b/drivers/iio/adc/stm32-
> > > > adc.c
> > > > index 3985fe972892..e55859113df8 100644
> > > > --- a/drivers/iio/adc/stm32-adc.c
> > > > +++ b/drivers/iio/adc/stm32-adc.c
> > > > @@ -21,11 +21,11 @@
> > > >  #include <linux/io.h>
> > > >  #include <linux/iopoll.h>
> > > >  #include <linux/module.h>
> > > > +#include <linux/mod_devicetable.h>
> > > >  #include <linux/nvmem-consumer.h>
> > > >  #include <linux/platform_device.h>
> > > >  #include <linux/pm_runtime.h>
> > > > -#include <linux/of.h>
> > > > -#include <linux/of_device.h>
> > > > +#include <linux/property.h>
> > > >  
> > > >  #include "stm32-adc-core.h"
> > > >  
> > > > @@ -1530,8 +1530,8 @@ static int
> > > > stm32_adc_update_scan_mode(struct
> > > > iio_dev *indio_dev,
> > > >         return ret;
> > > >  }
> > > >  
> > > > -static int stm32_adc_of_xlate(struct iio_dev *indio_dev,
> > > > -                             const struct of_phandle_args
> > > > *iiospec)
> > > > +static int stm32_adc_fwnode_xlate(struct iio_dev *indio_dev,
> > > > +                                 const struct
> > > > fwnode_reference_args *iiospec)
> > > >  {
> > > >         int i;
> > > >  
> > > > @@ -1585,7 +1585,7 @@ static const struct iio_info
> > > > stm32_adc_iio_info = {
> > > >         .hwfifo_set_watermark = stm32_adc_set_watermark,
> > > >         .update_scan_mode = stm32_adc_update_scan_mode,
> > > >         .debugfs_reg_access = stm32_adc_debugfs_reg_access,
> > > > -       .of_xlate = stm32_adc_of_xlate,
> > > > +       .fwnode_xlate = stm32_adc_fwnode_xlate,
> > > >  };
> > > >  
> > > >  static unsigned int stm32_adc_dma_residue(struct stm32_adc
> > > > *adc)
> > > > @@ -1782,14 +1782,14 @@ static const struct
> > > > iio_chan_spec_ext_info
> > > > stm32_adc_ext_info[] = {
> > > >         {},
> > > >  };
> > > >  
> > > > -static int stm32_adc_of_get_resolution(struct iio_dev
> > > > *indio_dev)
> > > > +static int stm32_adc_fw_get_resolution(struct iio_dev
> > > > *indio_dev)
> > > >  {
> > > > -       struct device_node *node = indio_dev->dev.of_node;
> > > > +       struct device *dev = &indio_dev->dev;
> > > >         struct stm32_adc *adc = iio_priv(indio_dev);
> > > >         unsigned int i;
> > > >         u32 res;
> > > >  
> > > > -       if (of_property_read_u32(node, "assigned-resolution-
> > > > bits",
> > > > &res))
> > > > +       if (device_property_read_u32(dev, "assigned-resolution-
> > > > bits", &res))
> > > >                 res = adc->cfg->adc_info->resolutions[0];
> > > >  
> > > >         for (i = 0; i < adc->cfg->adc_info->num_res; i++)
> > > > @@ -1873,11 +1873,11 @@ static void
> > > > stm32_adc_chan_init_one(struct
> > > > iio_dev *indio_dev,
> > > >  
> > > >  static int stm32_adc_get_legacy_chan_count(struct iio_dev
> > > > *indio_dev, struct stm32_adc *adc)
> > > >  {
> > > > -       struct device_node *node = indio_dev->dev.of_node;
> > > > +       struct device *dev = &indio_dev->dev;
> > > >         const struct stm32_adc_info *adc_info = adc->cfg-
> > > > >adc_info;
> > > >         int num_channels = 0, ret;
> > > >  
> > > > -       ret = of_property_count_u32_elems(node, "st,adc-
> > > > channels");
> > > > +       ret = device_property_count_u32(dev, "st,adc-
> > > > channels");
> > > >         if (ret > adc_info->max_channels) {
> > > >                 dev_err(&indio_dev->dev, "Bad st,adc-
> > > > channels?\n");
> > > >                 return -EINVAL;
> > > > @@ -1885,8 +1885,8 @@ static int
> > > > stm32_adc_get_legacy_chan_count(struct iio_dev *indio_dev,
> > > > struct
> > > > stm
> > > >                 num_channels += ret;
> > > >         }
> > > >  
> > > > -       ret = of_property_count_elems_of_size(node, "st,adc-
> > > > diff-
> > > > channels",
> > > > -                                             sizeof(struct
> > > > stm32_adc_diff_channel));
> > > > +       /* each st,adc-diff-channels is a group of 2 u32 */
> > > > +       ret = device_property_count_u64(dev, "st,adc-diff-
> > > > channels");
> > 
> > Are you fine with this change (still does not have any reference to
> > the
> > target struct but the comment might be helpful and there's no magic
> > 2)?
> 
> 
> Since you added that comment, this sounds better. IMHO, This still
> looks
> a bit weird. I'd feel more comfortable by using u32 API for a
> 'uint32-matrix' as defined in dt-bindings.
> Strictly speaking, something like
> sizeof(struct stm32_adc_diff_channel) / sizeof(u32) could be used,
> along
> with this comment and device_property_count_u32() to make it clear ?
> 

No strong option so I can do as you prefer:

	ret = device_property_count_u32();
	if (ret / 
(sizeof(struct stm32_adc_diff_channel) / sizeof(u32)) > max_chan) {
		...
	}
	...

Probably it's a good idea to store that sizeof() division in a local
variable :)

- Nuno Sá

> > > 
>