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

[PATCH v3 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/

Second version:

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

v3 changes:

[1/15]:
  * Improved (spell fixes) commit message.

[13/15]:
  * Get back to u32 API for 'st,adc-diff-channels' but making it more clear
what's going on;
  * Fix infinite loop;
  * Store number of st,min-sample-time-nsecs properties to avoid
duplication in sanity checks.

[15/15]
  * Improved 'if' condition for readability.

(hopefulyl did not forgot any tag)

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              | 125 ++++++-----
 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, 350 insertions(+), 315 deletions(-)

-- 
2.37.1

[PATCH v3 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/

Second version:

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

v3 changes:

[1/15]:
  * Improved (spell fixes) commit message.

[13/15]:
  * Get back to u32 API for 'st,adc-diff-channels' but making it more clear
what's going on;
  * Fix infinite loop;
  * Store number of st,min-sample-time-nsecs properties to avoid
duplication in sanity checks.

[15/15]
  * Improved 'if' condition for readability.

(hopefulyl did not forgot any tag)

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              | 125 ++++++-----
 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, 350 insertions(+), 315 deletions(-)

-- 
2.37.1

[PATCH v3 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 be freed while the device is still around. Thus, as there
are indeed some of_xlate users which do access the node, a 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.1

[PATCH v3 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.1

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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.1

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:28:51 +0200
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.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Hi Nuno,

Sorry for delay on getting to this.  Sometimes the most useful work
on the core gets queued up at the back of my review queue because it's harder
than the average driver :)

Anyhow, only totally trivial stuff in here from me.
If you don't respin because of other comments, I can clean these up
whilst applying.

Thanks,

Jonathan

> ---
>  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);
> +			}

Trivial, but wrong comment syntax for IIO + inconsistent with surrounding comments.

> +			/* 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;
>  		}
> -

Try to avoid noise like this + I like this whitespace :)

>  		/*
>  		 * 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);

[PATCH v3 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.1

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:28:52 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

> 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>
A few things from a build test (after hand applying this because of
issues with the comment thing following through the series).

Seem obvious so I fixed them up.

> ---
>  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,
static 
> +						 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;
Not used.

>  
> -		/*
> -		 * 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);

[PATCH v3 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.1

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:28:53 +0200
Nuno Sá <nuno.sa@analog.com> wrote:

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

Nice. One trivial follow through of wrong comment syntax. I'll fix up
if I pick the series up before you do a respin for other reasons.
> ---
>  drivers/iio/inkern.c         | 159 ++++++++++++++++++-----------------
>  include/linux/iio/consumer.h |  36 ++++----
>  include/linux/iio/iio.h      |   5 ++
>  3 files changed, 108 insertions(+), 92 deletions(-)
> 
> -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.

Wrong comment syntax. I guess I can fix this one as well if nothing else comes up.
Or I might be lazy and only fix this one as it replaces the previous wrong one.

> +		/* 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);
>  }

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

[Jonathan Cameron]

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

fixed up.

J

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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.1

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

[Daniel Lezcano]

On 15/07/2022 14:28, Nuno Sá wrote:
> 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

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


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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.1

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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.1

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:28:56 +0200
Nuno Sá <nuno.sa@analog.com> 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>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Nitpick, but ideally this would have been two patches. 
1) Drop parameter to *_parse_channels() 
2) the rest.

As that little corner was hardert o check than the rest :)

meh.  It's trivial as I said, otherwise looks good to me.

Jonathan


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

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

[Jonathan Cameron]

On Sat, 6 Aug 2022 19:03:37 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 15 Jul 2022 14:28:56 +0200
> Nuno Sá <nuno.sa@analog.com> 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>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>  
> Nitpick, but ideally this would have been two patches. 
> 1) Drop parameter to *_parse_channels() 
> 2) the rest.
ignore that.  I forgot there isn't a device_of_for_each_child_node().
So it would have been more effort than it was worth.

> 
> As that little corner was hardert o check than the rest :)

> 
> meh.  It's trivial as I said, otherwise looks good to me.
> 
> Jonathan
> 
> 
> > ---
> >  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;
> >    
> 

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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.1

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

[Claudiu.Beznea]

On 15.07.2022 15:28, Nuno Sá wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.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.1
> 

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

[Jonathan Cameron]

On Mon, 18 Jul 2022 05:21:47 +0000
<Claudiu.Beznea@microchip.com> wrote:

> On 15.07.2022 15:28, Nuno Sá wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > 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>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>  
> 
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> 

Hi Nuno / Claudiu

Lots of fuzz on this one because of Claudiu's series that I took whilst this was under review
I haven't really checked the result of forcing it in but please take a look at the
testing branch of iio.git for anything silly that happened.

Jonathan

> 
> > ---
> >  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.1
> >   
> 

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

[Claudiu.Beznea]

On 06.08.2022 21:49, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Mon, 18 Jul 2022 05:21:47 +0000
> <Claudiu.Beznea@microchip.com> wrote:
> 
>> On 15.07.2022 15:28, Nuno Sá wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> 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>
>>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>>
>> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
> 
> Hi Nuno / Claudiu
> 
> Lots of fuzz on this one because of Claudiu's series that I took whilst this was under review
> I haven't really checked the result of forcing it in but please take a look at the
> testing branch of iio.git for anything silly that happened.

Looks good to me.

Thank you,
Claudiu Beznea

> 
> Jonathan
> 
>>
>>> ---
>>>  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.1
>>>
>>
> 

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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.1

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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.1

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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.1

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:29:00 +0200
Nuno Sá <nuno.sa@analog.com> 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>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
+CC Marijin who happend to post a patch for this driver that I just accepted
and hence probably has hardware access.  Any chance of a test for this series?

If not, no problem as this is fairly mechanical and we have testing on some of
the other drivers using the new code.

I'll probably queue this up in the meantime but it won't end up upstream
for a few weeks yet.

Thanks,

Jonathan

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

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

[Marijn Suijten]

On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> On Fri, 15 Jul 2022 14:29:00 +0200
> Nuno Sá <nuno.sa@analog.com> 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>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> +CC Marijin who happend to post a patch for this driver that I just accepted
> and hence probably has hardware access.  Any chance of a test for this series?
> 
> If not, no problem as this is fairly mechanical and we have testing on some of
> the other drivers using the new code.
> 
> I'll probably queue this up in the meantime but it won't end up upstream
> for a few weeks yet.

Jonathan,

This CC just surfaced in my inbox while searching for our current
discussion around missing labels in qcom-spmi-vadc - and on the side a
userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
patch's fwnode_get_name change - we could've caught it if I had not
accidentally marked it as read and/or forgot about it.  My apologies.

- Marijn

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

[Marijn Suijten]

On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> On Fri, 15 Jul 2022 14:29:00 +0200
> Nuno Sá <nuno.sa@analog.com> 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>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> +CC Marijin who happend to post a patch for this driver that I just accepted
> and hence probably has hardware access.  Any chance of a test for this series?
> 
> If not, no problem as this is fairly mechanical and we have testing on some of
> the other drivers using the new code.
> 
> I'll probably queue this up in the meantime but it won't end up upstream
> for a few weeks yet.

Jonathan,

This CC just surfaced in my inbox while searching for our current
discussion around missing labels in qcom-spmi-vadc - and on the side a
userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
patch's fwnode_get_name change - we could've caught it if I had not
accidentally marked it as read and/or forgot about it.  My apologies.

- Marijn

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

[Andy Shevchenko]

On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > On Fri, 15 Jul 2022 14:29:00 +0200
> > Nuno Sá <nuno.sa@analog.com> 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>
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > +CC Marijin who happend to post a patch for this driver that I just accepted
> > and hence probably has hardware access.  Any chance of a test for this series?
> >
> > If not, no problem as this is fairly mechanical and we have testing on some of
> > the other drivers using the new code.
> >
> > I'll probably queue this up in the meantime but it won't end up upstream
> > for a few weeks yet.
>
> Jonathan,
>
> This CC just surfaced in my inbox while searching for our current
> discussion around missing labels in qcom-spmi-vadc - and on the side a
> userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> patch's fwnode_get_name change - we could've caught it if I had not
> accidentally marked it as read and/or forgot about it.  My apologies.

Does the following addition to the top of the
adc5_get_fw_channel_data() fix the issue?

+       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
+       if (!name)
+               return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jan 17, 2023 at 10:53 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > > On Fri, 15 Jul 2022 14:29:00 +0200
> > > Nuno Sá <nuno.sa@analog.com> 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>
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > +CC Marijin who happend to post a patch for this driver that I just accepted
> > > and hence probably has hardware access.  Any chance of a test for this series?
> > >
> > > If not, no problem as this is fairly mechanical and we have testing on some of
> > > the other drivers using the new code.
> > >
> > > I'll probably queue this up in the meantime but it won't end up upstream
> > > for a few weeks yet.
> >
> > Jonathan,
> >
> > This CC just surfaced in my inbox while searching for our current
> > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > patch's fwnode_get_name change - we could've caught it if I had not
> > accidentally marked it as read and/or forgot about it.  My apologies.
>
> Does the following addition to the top of the
> adc5_get_fw_channel_data() fix the issue?
>
> +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> +       if (!name)
> +               return -ENOMEM;

Okay, it probably the same, so it might need additional code to

+ name[strchrnul(name, '@') - name] = '\0';

-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jan 17, 2023 at 10:53 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
> <marijn.suijten@somainline.org> wrote:
> >
> > On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > > On Fri, 15 Jul 2022 14:29:00 +0200
> > > Nuno Sá <nuno.sa@analog.com> 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>
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > +CC Marijin who happend to post a patch for this driver that I just accepted
> > > and hence probably has hardware access.  Any chance of a test for this series?
> > >
> > > If not, no problem as this is fairly mechanical and we have testing on some of
> > > the other drivers using the new code.
> > >
> > > I'll probably queue this up in the meantime but it won't end up upstream
> > > for a few weeks yet.
> >
> > Jonathan,
> >
> > This CC just surfaced in my inbox while searching for our current
> > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > patch's fwnode_get_name change - we could've caught it if I had not
> > accidentally marked it as read and/or forgot about it.  My apologies.
>
> Does the following addition to the top of the
> adc5_get_fw_channel_data() fix the issue?
>
> +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> +       if (!name)
> +               return -ENOMEM;

Okay, it probably the same, so it might need additional code to

+ name[strchrnul(name, '@') - name] = '\0';

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andy Shevchenko]

On Tue, Jan 17, 2023 at 11:06 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jan 17, 2023 at 10:53 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > > > On Fri, 15 Jul 2022 14:29:00 +0200
> > > > Nuno Sá <nuno.sa@analog.com> 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>
> > > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > +CC Marijin who happend to post a patch for this driver that I just accepted
> > > > and hence probably has hardware access.  Any chance of a test for this series?
> > > >
> > > > If not, no problem as this is fairly mechanical and we have testing on some of
> > > > the other drivers using the new code.
> > > >
> > > > I'll probably queue this up in the meantime but it won't end up upstream
> > > > for a few weeks yet.
> > >
> > > Jonathan,
> > >
> > > This CC just surfaced in my inbox while searching for our current
> > > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > > patch's fwnode_get_name change - we could've caught it if I had not
> > > accidentally marked it as read and/or forgot about it.  My apologies.
> >
> > Does the following addition to the top of the
> > adc5_get_fw_channel_data() fix the issue?
> >
> > +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> > +       if (!name)
> > +               return -ENOMEM;
>
> Okay, it probably the same, so it might need additional code to
>
> + name[strchrnul(name, '@') - name] = '\0';

I have just sent a formal patch, please test on top of non-working kernel.


--
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Tue, Jan 17, 2023 at 11:06 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jan 17, 2023 at 10:53 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > > > On Fri, 15 Jul 2022 14:29:00 +0200
> > > > Nuno Sá <nuno.sa@analog.com> 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>
> > > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > +CC Marijin who happend to post a patch for this driver that I just accepted
> > > > and hence probably has hardware access.  Any chance of a test for this series?
> > > >
> > > > If not, no problem as this is fairly mechanical and we have testing on some of
> > > > the other drivers using the new code.
> > > >
> > > > I'll probably queue this up in the meantime but it won't end up upstream
> > > > for a few weeks yet.
> > >
> > > Jonathan,
> > >
> > > This CC just surfaced in my inbox while searching for our current
> > > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > > patch's fwnode_get_name change - we could've caught it if I had not
> > > accidentally marked it as read and/or forgot about it.  My apologies.
> >
> > Does the following addition to the top of the
> > adc5_get_fw_channel_data() fix the issue?
> >
> > +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> > +       if (!name)
> > +               return -ENOMEM;
>
> Okay, it probably the same, so it might need additional code to
>
> + name[strchrnul(name, '@') - name] = '\0';

I have just sent a formal patch, please test on top of non-working kernel.


--
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Andy Shevchenko]

On Tue, Jan 17, 2023 at 11:06 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Tue, Jan 17, 2023 at 10:53 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Jan 16, 2023 at 10:44 PM Marijn Suijten
> > <marijn.suijten@somainline.org> wrote:
> > >
> > > On 2022-08-06 19:20:48, Jonathan Cameron wrote:
> > > > On Fri, 15 Jul 2022 14:29:00 +0200
> > > > Nuno Sá <nuno.sa@analog.com> 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>
> > > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > > > +CC Marijin who happend to post a patch for this driver that I just accepted
> > > > and hence probably has hardware access.  Any chance of a test for this series?
> > > >
> > > > If not, no problem as this is fairly mechanical and we have testing on some of
> > > > the other drivers using the new code.
> > > >
> > > > I'll probably queue this up in the meantime but it won't end up upstream
> > > > for a few weeks yet.
> > >
> > > Jonathan,
> > >
> > > This CC just surfaced in my inbox while searching for our current
> > > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > > patch's fwnode_get_name change - we could've caught it if I had not
> > > accidentally marked it as read and/or forgot about it.  My apologies.
> >
> > Does the following addition to the top of the
> > adc5_get_fw_channel_data() fix the issue?
> >
> > +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> > +       if (!name)
> > +               return -ENOMEM;
>
> Okay, it probably the same, so it might need additional code to
>
> + name[strchrnul(name, '@') - name] = '\0';

I have just sent a formal patch, please test on top of non-working kernel.


--
With Best Regards,
Andy Shevchenko

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

[Marijn Suijten]

On 2023-01-17 11:40:42, Andy Shevchenko wrote:
<snip>
> > > > This CC just surfaced in my inbox while searching for our current
> > > > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > > > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > > > patch's fwnode_get_name change - we could've caught it if I had not
> > > > accidentally marked it as read and/or forgot about it.  My apologies.
> > >
> > > Does the following addition to the top of the
> > > adc5_get_fw_channel_data() fix the issue?
> > >
> > > +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> > > +       if (!name)
> > > +               return -ENOMEM;
> >
> > Okay, it probably the same, so it might need additional code to
> >
> > + name[strchrnul(name, '@') - name] = '\0';
> 
> I have just sent a formal patch, please test on top of non-working kernel.

I would've preferred to fix this in the same way as qcom-spmi-vadc by
implementing read_label instead and basing it on the DT label or driver
string literals instead [1], but dropping extend_name (hence changing
sysfs filenames once again) would be considered an ABI break.

I've instead gone for a similar change that also ignores the node name
in favour of falling back to the driver string literal (if no "label"
property is available) while retaining extend_label [3] and the -
however ugly they are - sysfs filenames, but that'll likely get rejected
as strictly being an ABI break as well, not in the least because DT
needs to be patched up [3] for it to work out.

I'll at least test your patch when getting back to one of these devices.

- Marijn

[1]: https://lore.kernel.org/linux-arm-msm/20230116220909.196926-6-marijn.suijten@somainline.org/
[2]: https://lore.kernel.org/linux-arm-msm/20230116220909.196926-4-marijn.suijten@somainline.org/
[3]: https://lore.kernel.org/linux-arm-msm/20221209215308.1781047-1-marijn.suijten@somainline.org/

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

[Marijn Suijten]

On 2023-01-17 11:40:42, Andy Shevchenko wrote:
<snip>
> > > > This CC just surfaced in my inbox while searching for our current
> > > > discussion around missing labels in qcom-spmi-vadc - and on the side a
> > > > userspace @xx label name ABI break (in qcom-spmi-adc5) caused by this
> > > > patch's fwnode_get_name change - we could've caught it if I had not
> > > > accidentally marked it as read and/or forgot about it.  My apologies.
> > >
> > > Does the following addition to the top of the
> > > adc5_get_fw_channel_data() fix the issue?
> > >
> > > +       name = devm_kasprintf(adc->dev, GFP_KERNEL, "%pfwP", fwnode);
> > > +       if (!name)
> > > +               return -ENOMEM;
> >
> > Okay, it probably the same, so it might need additional code to
> >
> > + name[strchrnul(name, '@') - name] = '\0';
> 
> I have just sent a formal patch, please test on top of non-working kernel.

I would've preferred to fix this in the same way as qcom-spmi-vadc by
implementing read_label instead and basing it on the DT label or driver
string literals instead [1], but dropping extend_name (hence changing
sysfs filenames once again) would be considered an ABI break.

I've instead gone for a similar change that also ignores the node name
in favour of falling back to the driver string literal (if no "label"
property is available) while retaining extend_label [3] and the -
however ugly they are - sysfs filenames, but that'll likely get rejected
as strictly being an ABI break as well, not in the least because DT
needs to be patched up [3] for it to work out.

I'll at least test your patch when getting back to one of these devices.

- Marijn

[1]: https://lore.kernel.org/linux-arm-msm/20230116220909.196926-6-marijn.suijten@somainline.org/
[2]: https://lore.kernel.org/linux-arm-msm/20230116220909.196926-4-marijn.suijten@somainline.org/
[3]: https://lore.kernel.org/linux-arm-msm/20221209215308.1781047-1-marijn.suijten@somainline.org/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/iio/adc/stm32-adc.c | 125 ++++++++++++++++++++----------------
 1 file changed, 70 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 3985fe972892..24248f1de1d0 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"
 
@@ -241,6 +241,7 @@ struct stm32_adc_cfg {
  * @chan_name:		channel name array
  * @num_diff:		number of differential channels
  * @int_ch:		internal channel indexes array
+ * @nsmps:		number of channels with optional sample time
  */
 struct stm32_adc {
 	struct stm32_adc_common	*common;
@@ -267,6 +268,7 @@ struct stm32_adc {
 	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
 	u32			num_diff;
 	int			int_ch[STM32_ADC_INT_CH_NB];
+	int			nsmps;
 };
 
 struct stm32_adc_diff_channel {
@@ -1530,8 +1532,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 +1587,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 +1784,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 +1875,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 +1887,12 @@ 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 so we divide @ret
+	 * to get the *real* number of channels.
+	 */
+	ret = device_property_count_u32(dev, "st,adc-diff-channels");
+	ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32));
 	if (ret > adc_info->max_channels) {
 		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
 		return -EINVAL;
@@ -1896,8 +1902,8 @@ 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");
-	if (ret > 1 && ret != num_channels) {
+	adc->nsmps = device_property_count_u32(dev, "st,min-sample-time-nsecs");
+	if (adc->nsmps > 1 && adc->nsmps != num_channels) {
 		dev_err(&indio_dev->dev, "Invalid st,min-sample-time-nsecs\n");
 		return -EINVAL;
 	}
@@ -1907,21 +1913,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, c;
+	u32 smp = 0, 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 +1947,47 @@ 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 (c = 0; c < nchans; c++) {
+		if (chans[c] >= adc_info->max_channels) {
+			dev_err(&indio_dev->dev, "Invalid channel %d\n",
+				chans[c]);
 			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[c] == diff[i].vinp) {
+				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	chans[c]);
 				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[c], 0, scan_index, false);
 		scan_index++;
 	}
 
+	if (adc->nsmps > 0) {
+		ret = device_property_read_u32_array(dev, "st,min-sample-time-nsecs",
+						     smps, adc->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 < adc->nsmps)
+			smp = smps[i];
 
 		/* Prepare sampling time settings */
 		stm32_adc_smpr_init(adc, channels[i].channel, smp);
@@ -2010,22 +2030,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 +2069,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 +2086,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 +2104,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 +2120,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 +2151,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 +2234,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 +2242,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 +2280,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 +2297,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.1

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

[Fabrice Gasnier]

On 7/15/22 14:29, 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Hi Nuno,

You can add my:
Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Best Regards,
Thanks,
Fabrice

> ---
>  drivers/iio/adc/stm32-adc.c | 125 ++++++++++++++++++++----------------
>  1 file changed, 70 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index 3985fe972892..24248f1de1d0 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"
>  
> @@ -241,6 +241,7 @@ struct stm32_adc_cfg {
>   * @chan_name:		channel name array
>   * @num_diff:		number of differential channels
>   * @int_ch:		internal channel indexes array
> + * @nsmps:		number of channels with optional sample time
>   */
>  struct stm32_adc {
>  	struct stm32_adc_common	*common;
> @@ -267,6 +268,7 @@ struct stm32_adc {
>  	char			chan_name[STM32_ADC_CH_MAX][STM32_ADC_CH_SZ];
>  	u32			num_diff;
>  	int			int_ch[STM32_ADC_INT_CH_NB];
> +	int			nsmps;
>  };
>  
>  struct stm32_adc_diff_channel {
> @@ -1530,8 +1532,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 +1587,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 +1784,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 +1875,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 +1887,12 @@ 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 so we divide @ret
> +	 * to get the *real* number of channels.
> +	 */
> +	ret = device_property_count_u32(dev, "st,adc-diff-channels");
> +	ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32));
>  	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>  		return -EINVAL;
> @@ -1896,8 +1902,8 @@ 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");
> -	if (ret > 1 && ret != num_channels) {
> +	adc->nsmps = device_property_count_u32(dev, "st,min-sample-time-nsecs");
> +	if (adc->nsmps > 1 && adc->nsmps != num_channels) {
>  		dev_err(&indio_dev->dev, "Invalid st,min-sample-time-nsecs\n");
>  		return -EINVAL;
>  	}
> @@ -1907,21 +1913,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, c;
> +	u32 smp = 0, 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 +1947,47 @@ 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 (c = 0; c < nchans; c++) {
> +		if (chans[c] >= adc_info->max_channels) {
> +			dev_err(&indio_dev->dev, "Invalid channel %d\n",
> +				chans[c]);
>  			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[c] == diff[i].vinp) {
> +				dev_err(&indio_dev->dev, "channel %d misconfigured\n",	chans[c]);
>  				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[c], 0, scan_index, false);
>  		scan_index++;
>  	}
>  
> +	if (adc->nsmps > 0) {
> +		ret = device_property_read_u32_array(dev, "st,min-sample-time-nsecs",
> +						     smps, adc->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 < adc->nsmps)
> +			smp = smps[i];
>  
>  		/* Prepare sampling time settings */
>  		stm32_adc_smpr_init(adc, channels[i].channel, smp);
> @@ -2010,22 +2030,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 +2069,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 +2086,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 +2104,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 +2120,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 +2151,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 +2234,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 +2242,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 +2280,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 +2297,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 v3 13/15] iio: adc: stm32-adc: convert to device properties

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:29:01 +0200
Nuno Sá <nuno.sa@analog.com> 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>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
One query below. Otherwise LGTM (not that's true of the other patches I haven't
comment on at all)

> @@ -1885,8 +1887,12 @@ 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 so we divide @ret
> +	 * to get the *real* number of channels.
> +	 */
> +	ret = device_property_count_u32(dev, "st,adc-diff-channels");

I think this can return an error code which should probably be handled.
More than possible that was true of original code though...

If it can't return an error for some reason I'm missing perhaps a
comment to explain why.

> +	ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32));
>  	if (ret > adc_info->max_channels) {
>  		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
>  		return -EINVAL;

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

[Jonathan Cameron]

On Sat, 6 Aug 2022 19:15:38 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 15 Jul 2022 14:29:01 +0200
> Nuno Sá <nuno.sa@analog.com> 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>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>  
> One query below. Otherwise LGTM (not that's true of the other patches I haven't
> comment on at all)
> 
> > @@ -1885,8 +1887,12 @@ 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 so we divide @ret
> > +	 * to get the *real* number of channels.
> > +	 */
> > +	ret = device_property_count_u32(dev, "st,adc-diff-channels");  
> 
> I think this can return an error code which should probably be handled.
> More than possible that was true of original code though...
> 
> If it can't return an error for some reason I'm missing perhaps a
> comment to explain why.

Given this was only outstanding question I stuck a conservative

if (ret < 0)
	return ret;
here whilst applying.

> 
> > +	ret /= (int)(sizeof(struct stm32_adc_diff_channel) / sizeof(u32));
> >  	if (ret > adc_info->max_channels) {
> >  		dev_err(&indio_dev->dev, "Bad st,adc-diff-channels?\n");
> >  		return -EINVAL;  

[PATCH v3 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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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.1

[PATCH v3 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..0c9b1561af31 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.1

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

[Andy Shevchenko]

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

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> 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..0c9b1561af31 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.1
>


-- 
With Best Regards,
Andy Shevchenko

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

[Jonathan Cameron]

On Fri, 15 Jul 2022 14:28:48 +0200
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/
> 
> Second version:
> 
> https://lore.kernel.org/linux-iio/20220711123835.811358-1-nuno.sa@analog.com/
> 
> v3 changes:
> 
> [1/15]:
>   * Improved (spell fixes) commit message.
> 
> [13/15]:
>   * Get back to u32 API for 'st,adc-diff-channels' but making it more clear
> what's going on;
>   * Fix infinite loop;
>   * Store number of st,min-sample-time-nsecs properties to avoid
> duplication in sanity checks.
> 
> [15/15]
>   * Improved 'if' condition for readability.
> 
> (hopefulyl did not forgot any tag)


Very nice work.  All applied, with a few minor tweaks as called out in
relevant patches.  Note I'll push this out as testing.  Will go out as togreg
only after I can rebase on rc1 in a week or so time.

Thanks,

Jonathan

> 
> 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              | 125 ++++++-----
>  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, 350 insertions(+), 315 deletions(-)
>