[PATCH v6 0/8] IIO: Use device_for_each_child_scope()

[PATCH v6 0/8] IIO: Use device_for_each_child_scope()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Looking for review of these remaining patches from the earlier series.
Chances since V6:
- Rebased
- Add a precursor fix for stm32-adc to aid backporting.
 
The infrastructure being used here is now in iio.git/togreg. It missed
the merge window, but the similar DT series landed without problem.

I'm slimming this series down by taking patches that have been reviewed
but nothing yet on these ones yet.


Jonathan Cameron (8):
  iio: adc: mcp3564: Use device_for_each_child_node_scoped()
  iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()
  iio: adc: stm32: Fixing err code to not indicate success
  iio: adc: stm32: Use device_for_each_child_node_scoped()
  iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()
  iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped()
  iio: dac: ad3552r: Use device_for_each_child_node_scoped()
  iio: dac: ad5770r: Use device_for_each_child_node_scoped()

 drivers/iio/adc/mcp3564.c        | 16 ++++-----
 drivers/iio/adc/qcom-spmi-adc5.c |  7 ++--
 drivers/iio/adc/stm32-adc.c      | 61 +++++++++++++-------------------
 drivers/iio/adc/ti-ads1015.c     |  5 +--
 drivers/iio/adc/ti-ads131e08.c   | 13 +++----
 drivers/iio/dac/ad3552r.c        | 51 ++++++++++----------------
 drivers/iio/dac/ad5770r.c        | 19 ++++------
 7 files changed, 63 insertions(+), 109 deletions(-)

-- 
2.44.0

[PATCH v6 1/8] iio: adc: mcp3564: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Marius Cristea <marius.cristea@microchip.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/mcp3564.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/mcp3564.c b/drivers/iio/adc/mcp3564.c
index 311b613b6057..e2ae13f1e842 100644
--- a/drivers/iio/adc/mcp3564.c
+++ b/drivers/iio/adc/mcp3564.c
@@ -998,7 +998,6 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 	struct mcp3564_state *adc = iio_priv(indio_dev);
 	struct device *dev = &adc->spi->dev;
 	struct iio_chan_spec *channels;
-	struct fwnode_handle *child;
 	struct iio_chan_spec chanspec = mcp3564_channel_template;
 	struct iio_chan_spec temp_chanspec = mcp3564_temp_channel_template;
 	struct iio_chan_spec burnout_chanspec = mcp3564_burnout_channel_template;
@@ -1025,7 +1024,7 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 	if (!channels)
 		return dev_err_probe(dev, -ENOMEM, "Can't allocate memory\n");
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		node_name = fwnode_get_name(child);
 
 		if (fwnode_property_present(child, "diff-channels")) {
@@ -1033,26 +1032,25 @@ static int mcp3564_parse_fw_children(struct iio_dev *indio_dev)
 							     "diff-channels",
 							     inputs,
 							     ARRAY_SIZE(inputs));
+			if (ret)
+				return ret;
+
 			chanspec.differential = 1;
 		} else {
 			ret = fwnode_property_read_u32(child, "reg", &inputs[0]);
+			if (ret)
+				return ret;
 
 			chanspec.differential = 0;
 			inputs[1] = MCP3564_AGND;
 		}
-		if (ret) {
-			fwnode_handle_put(child);
-			return ret;
-		}
 
 		if (inputs[0] > MCP3564_INTERNAL_VCM ||
-		    inputs[1] > MCP3564_INTERNAL_VCM) {
-			fwnode_handle_put(child);
+		    inputs[1] > MCP3564_INTERNAL_VCM)
 			return dev_err_probe(&indio_dev->dev, -EINVAL,
 					     "Channel index > %d, for %s\n",
 					     MCP3564_INTERNAL_VCM + 1,
 					     node_name);
-		}
 
 		chanspec.address = (inputs[0] << 4) | inputs[1];
 		chanspec.channel = inputs[0];
-- 
2.44.0

[PATCH v6 2/8] iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

A slightly less convincing usecase than many as all the failure paths
are wrapped up in a call to a per fwnode_handle utility function.
The complexity in that function is sufficient that it makes sense to
factor it out even if it this new auto cleanup would enable simpler
returns if the code was inline at the call site. Hence I've left it alone.

Cc: Marijn Suijten <marijn.suijten@somainline.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/qcom-spmi-adc5.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
index b6b612d733ff..9b69f40beed8 100644
--- a/drivers/iio/adc/qcom-spmi-adc5.c
+++ b/drivers/iio/adc/qcom-spmi-adc5.c
@@ -825,7 +825,6 @@ 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 fwnode_handle *child;
 	unsigned int index = 0;
 	int ret;
 
@@ -849,12 +848,10 @@ static int adc5_get_fw_data(struct adc5_chip *adc)
 	if (!adc->data)
 		adc->data = &adc5_data_pmic;
 
-	device_for_each_child_node(adc->dev, child) {
+	device_for_each_child_node_scoped(adc->dev, child) {
 		ret = adc5_get_fw_channel_data(adc, &prop, child, adc->data);
-		if (ret) {
-			fwnode_handle_put(child);
+		if (ret)
 			return ret;
-		}
 
 		prop.scale_fn_type =
 			adc->data->adc_chans[prop.channel].scale_fn_type;
-- 
2.44.0

Re: [PATCH v6 2/8] iio: adc: qcom-spmi-adc5: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

On Sat, 30 Mar 2024 18:52:59 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> A slightly less convincing usecase than many as all the failure paths
> are wrapped up in a call to a per fwnode_handle utility function.
> The complexity in that function is sufficient that it makes sense to
> factor it out even if it this new auto cleanup would enable simpler
> returns if the code was inline at the call site. Hence I've left it alone.
> 
> Cc: Marijn Suijten <marijn.suijten@somainline.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
I'm not sure why I didn't pick this one up before now given it has
an RB from Dmitry. Applied.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/qcom-spmi-adc5.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/qcom-spmi-adc5.c b/drivers/iio/adc/qcom-spmi-adc5.c
> index b6b612d733ff..9b69f40beed8 100644
> --- a/drivers/iio/adc/qcom-spmi-adc5.c
> +++ b/drivers/iio/adc/qcom-spmi-adc5.c
> @@ -825,7 +825,6 @@ 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 fwnode_handle *child;
>  	unsigned int index = 0;
>  	int ret;
>  
> @@ -849,12 +848,10 @@ static int adc5_get_fw_data(struct adc5_chip *adc)
>  	if (!adc->data)
>  		adc->data = &adc5_data_pmic;
>  
> -	device_for_each_child_node(adc->dev, child) {
> +	device_for_each_child_node_scoped(adc->dev, child) {
>  		ret = adc5_get_fw_channel_data(adc, &prop, child, adc->data);
> -		if (ret) {
> -			fwnode_handle_put(child);
> +		if (ret)
>  			return ret;
> -		}
>  
>  		prop.scale_fn_type =
>  			adc->data->adc_chans[prop.channel].scale_fn_type;

[PATCH v6 3/8] iio: adc: stm32: Fixing err code to not indicate success

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This path would result in returning 0 / success on an error path.

Cc: Olivier Moysan <olivier.moysan@foss.st.com>
Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Fixes: 95bc818404b2 ("iio: adc: stm32-adc: add support of generic channels binding")
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/stm32-adc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index b5d3c9cea5c4..283c20757106 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -2234,6 +2234,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 			if (vin[0] != val || vin[1] >= adc_info->max_channels) {
 				dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
 					vin[0], vin[1]);
+				ret = -EINVAL;
 				goto err;
 			}
 		} else if (ret != -EINVAL) {
-- 
2.44.0

Re: [PATCH v6 3/8] iio: adc: stm32: Fixing err code to not indicate success

[Fabrice Gasnier]

On 3/30/24 19:53, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This path would result in returning 0 / success on an error path.
> 
> Cc: Olivier Moysan <olivier.moysan@foss.st.com>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Fixes: 95bc818404b2 ("iio: adc: stm32-adc: add support of generic channels binding")
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/iio/adc/stm32-adc.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Hi Jonathan,

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

Best Regards,
Thanks,
Fabrice

> diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> index b5d3c9cea5c4..283c20757106 100644
> --- a/drivers/iio/adc/stm32-adc.c
> +++ b/drivers/iio/adc/stm32-adc.c
> @@ -2234,6 +2234,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
>  			if (vin[0] != val || vin[1] >= adc_info->max_channels) {
>  				dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
>  					vin[0], vin[1]);
> +				ret = -EINVAL;
>  				goto err;
>  			}
>  		} else if (ret != -EINVAL) {

Re: [PATCH v6 3/8] iio: adc: stm32: Fixing err code to not indicate success

[Jonathan Cameron]

On Fri, 12 Apr 2024 14:31:20 +0200
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:

> On 3/30/24 19:53, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > This path would result in returning 0 / success on an error path.
> > 
> > Cc: Olivier Moysan <olivier.moysan@foss.st.com>
> > Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> > Fixes: 95bc818404b2 ("iio: adc: stm32-adc: add support of generic channels binding")
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/iio/adc/stm32-adc.c | 1 +
> >  1 file changed, 1 insertion(+)
> >   
> 
> Hi Jonathan,
> 
> You can add my:
> Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Applied. I'll take this via the slow path and queue it for next merge window rather
than rushing it through as a fix.

Thanks

Jonathan


> 
> Best Regards,
> Thanks,
> Fabrice
> 
> > diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
> > index b5d3c9cea5c4..283c20757106 100644
> > --- a/drivers/iio/adc/stm32-adc.c
> > +++ b/drivers/iio/adc/stm32-adc.c
> > @@ -2234,6 +2234,7 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
> >  			if (vin[0] != val || vin[1] >= adc_info->max_channels) {
> >  				dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
> >  					vin[0], vin[1]);
> > +				ret = -EINVAL;
> >  				goto err;
> >  			}
> >  		} else if (ret != -EINVAL) {  
> 

[PATCH v6 4/8] iio: adc: stm32: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Note this would have made the bug fixed in the previous path much
less likely as it allows for direct returns.

Took advantage of dev_err_probe() to futher simplify things given no
longer a need for the goto err.

Cc: Olivier Moysan <olivier.moysan@foss.st.com>
Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Acked-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/stm32-adc.c | 62 ++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 38 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index 283c20757106..36add95212c3 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -2187,59 +2187,52 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 				       struct iio_chan_spec *channels)
 {
 	const struct stm32_adc_info *adc_info = adc->cfg->adc_info;
-	struct fwnode_handle *child;
+	struct device *dev = &indio_dev->dev;
 	const char *name;
 	int val, scan_index = 0, ret;
 	bool differential;
 	u32 vin[2];
 
-	device_for_each_child_node(&indio_dev->dev, child) {
+	device_for_each_child_node_scoped(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;
-		}
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Missing channel index\n");
 
 		ret = fwnode_property_read_string(child, "label", &name);
 		/* label is optional */
 		if (!ret) {
-			if (strlen(name) >= STM32_ADC_CH_SZ) {
-				dev_err(&indio_dev->dev, "Label %s exceeds %d characters\n",
-					name, STM32_ADC_CH_SZ);
-				ret = -EINVAL;
-				goto err;
-			}
+			if (strlen(name) >= STM32_ADC_CH_SZ)
+				return dev_err_probe(dev, -EINVAL,
+						     "Label %s exceeds %d characters\n",
+						     name, STM32_ADC_CH_SZ);
+
 			strscpy(adc->chan_name[val], name, STM32_ADC_CH_SZ);
 			ret = stm32_adc_populate_int_ch(indio_dev, name, val);
 			if (ret == -ENOENT)
 				continue;
 			else if (ret)
-				goto err;
+				return ret;
 		} else if (ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid label %d\n", ret);
-			goto err;
+			return dev_err_probe(dev, ret, "Invalid label\n");
 		}
 
-		if (val >= adc_info->max_channels) {
-			dev_err(&indio_dev->dev, "Invalid channel %d\n", val);
-			ret = -EINVAL;
-			goto err;
-		}
+		if (val >= adc_info->max_channels)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid channel %d\n", val);
 
 		differential = false;
 		ret = fwnode_property_read_u32_array(child, "diff-channels", vin, 2);
 		/* diff-channels is optional */
 		if (!ret) {
 			differential = true;
-			if (vin[0] != val || vin[1] >= adc_info->max_channels) {
-				dev_err(&indio_dev->dev, "Invalid channel in%d-in%d\n",
-					vin[0], vin[1]);
-				ret = -EINVAL;
-				goto err;
-			}
+			if (vin[0] != val || vin[1] >= adc_info->max_channels)
+				return dev_err_probe(dev, -EINVAL,
+						     "Invalid channel in%d-in%d\n",
+						     vin[0], vin[1]);
 		} else if (ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid diff-channels property %d\n", ret);
-			goto err;
+			return dev_err_probe(dev, ret,
+					     "Invalid diff-channels property\n");
 		}
 
 		stm32_adc_chan_init_one(indio_dev, &channels[scan_index], val,
@@ -2248,11 +2241,9 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 		val = 0;
 		ret = fwnode_property_read_u32(child, "st,min-sample-time-ns", &val);
 		/* st,min-sample-time-ns is optional */
-		if (ret && ret != -EINVAL) {
-			dev_err(&indio_dev->dev, "Invalid st,min-sample-time-ns property %d\n",
-				ret);
-			goto err;
-		}
+		if (ret && ret != -EINVAL)
+			return dev_err_probe(dev, ret,
+					     "Invalid st,min-sample-time-ns property\n");
 
 		stm32_adc_smpr_init(adc, channels[scan_index].channel, val);
 		if (differential)
@@ -2262,11 +2253,6 @@ static int stm32_adc_generic_chan_init(struct iio_dev *indio_dev,
 	}
 
 	return scan_index;
-
-err:
-	fwnode_handle_put(child);
-
-	return ret;
 }
 
 static int stm32_adc_chan_fw_init(struct iio_dev *indio_dev, bool timestamping)
-- 
2.44.0

Re: [PATCH v6 4/8] iio: adc: stm32: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

On Sat, 30 Mar 2024 18:53:01 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Note this would have made the bug fixed in the previous path much
> less likely as it allows for direct returns.
> 
> Took advantage of dev_err_probe() to futher simplify things given no
> longer a need for the goto err.
> 
> Cc: Olivier Moysan <olivier.moysan@foss.st.com>
> Cc: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Acked-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Applied,

Thanks,

[PATCH v6 5/8] iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads1015.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 6ae967e4d8fa..d3363d02f292 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -902,10 +902,9 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct ads1015_data *data = iio_priv(indio_dev);
 	struct device *dev = &client->dev;
-	struct fwnode_handle *node;
 	int i = -1;
 
-	device_for_each_child_node(dev, node) {
+	device_for_each_child_node_scoped(dev, node) {
 		u32 pval;
 		unsigned int channel;
 		unsigned int pga = ADS1015_DEFAULT_PGA;
@@ -927,7 +926,6 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 			pga = pval;
 			if (pga > 5) {
 				dev_err(dev, "invalid gain on %pfw\n", node);
-				fwnode_handle_put(node);
 				return -EINVAL;
 			}
 		}
@@ -936,7 +934,6 @@ static int ads1015_client_get_channels_config(struct i2c_client *client)
 			data_rate = pval;
 			if (data_rate > 7) {
 				dev_err(dev, "invalid data_rate on %pfw\n", node);
-				fwnode_handle_put(node);
 				return -EINVAL;
 			}
 		}
-- 
2.44.0

[PATCH v6 6/8] iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Tomislav Denis <tomislav.denis@avl.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/adc/ti-ads131e08.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index fcfc46254313..f653654f7c5d 100644
--- a/drivers/iio/adc/ti-ads131e08.c
+++ b/drivers/iio/adc/ti-ads131e08.c
@@ -694,7 +694,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 	struct ads131e08_channel_config *channel_config;
 	struct device *dev = &st->spi->dev;
 	struct iio_chan_spec *channels;
-	struct fwnode_handle *node;
 	unsigned int channel, tmp;
 	int num_channels, i, ret;
 
@@ -736,10 +735,10 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		return -ENOMEM;
 
 	i = 0;
-	device_for_each_child_node(dev, node) {
+	device_for_each_child_node_scoped(dev, node) {
 		ret = fwnode_property_read_u32(node, "reg", &channel);
 		if (ret)
-			goto err_child_out;
+			return ret;
 
 		ret = fwnode_property_read_u32(node, "ti,gain", &tmp);
 		if (ret) {
@@ -747,7 +746,7 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		} else {
 			ret = ads131e08_pga_gain_to_field_value(st, tmp);
 			if (ret < 0)
-				goto err_child_out;
+				return ret;
 
 			channel_config[i].pga_gain = tmp;
 		}
@@ -758,7 +757,7 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 		} else {
 			ret = ads131e08_validate_channel_mux(st, tmp);
 			if (ret)
-				goto err_child_out;
+				return ret;
 
 			channel_config[i].mux = tmp;
 		}
@@ -784,10 +783,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 	st->channel_config = channel_config;
 
 	return 0;
-
-err_child_out:
-	fwnode_handle_put(node);
-	return ret;
 }
 
 static void ads131e08_regulator_disable(void *data)
-- 
2.44.0

[PATCH v6 7/8] iio: dac: ad3552r: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Removing the goto err; statements also allows more extensive use of
dev_err_probe() further simplifying the code.

Cc: Mihail Chindris <mihail.chindris@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad3552r.c | 51 +++++++++++++++------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
index a492e8f2fc0f..e14a065b29ca 100644
--- a/drivers/iio/dac/ad3552r.c
+++ b/drivers/iio/dac/ad3552r.c
@@ -880,7 +880,6 @@ static void ad3552r_reg_disable(void *reg)
 static int ad3552r_configure_device(struct ad3552r_desc *dac)
 {
 	struct device *dev = &dac->spi->dev;
-	struct fwnode_handle *child;
 	struct regulator *vref;
 	int err, cnt = 0, voltage, delta = 100000;
 	u32 vals[2], val, ch;
@@ -949,53 +948,45 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 		return -ENODEV;
 	}
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		err = fwnode_property_read_u32(child, "reg", &ch);
-		if (err) {
-			dev_err(dev, "mandatory reg property missing\n");
-			goto put_child;
-		}
-		if (ch >= AD3552R_NUM_CH) {
-			dev_err(dev, "reg must be less than %d\n",
-				AD3552R_NUM_CH);
-			err = -EINVAL;
-			goto put_child;
-		}
+		if (err)
+			return dev_err_probe(dev, err,
+					     "mandatory reg property missing\n");
+		if (ch >= AD3552R_NUM_CH)
+			return dev_err_probe(dev, -EINVAL,
+					     "reg must be less than %d\n",
+					     AD3552R_NUM_CH);
 
 		if (fwnode_property_present(child, "adi,output-range-microvolt")) {
 			err = fwnode_property_read_u32_array(child,
 							     "adi,output-range-microvolt",
 							     vals,
 							     2);
-			if (err) {
-				dev_err(dev,
+			if (err)
+				return dev_err_probe(dev, err,
 					"adi,output-range-microvolt property could not be parsed\n");
-				goto put_child;
-			}
 
 			err = ad3552r_find_range(dac->chip_id, vals);
-			if (err < 0) {
-				dev_err(dev,
-					"Invalid adi,output-range-microvolt value\n");
-				goto put_child;
-			}
+			if (err < 0)
+				return dev_err_probe(dev, err,
+						     "Invalid adi,output-range-microvolt value\n");
+
 			val = err;
 			err = ad3552r_set_ch_value(dac,
 						   AD3552R_CH_OUTPUT_RANGE_SEL,
 						   ch, val);
 			if (err)
-				goto put_child;
+				return err;
 
 			dac->ch_data[ch].range = val;
 		} else if (dac->chip_id == AD3542R_ID) {
-			dev_err(dev,
-				"adi,output-range-microvolt is required for ad3542r\n");
-			err = -EINVAL;
-			goto put_child;
+			return dev_err_probe(dev, -EINVAL,
+					     "adi,output-range-microvolt is required for ad3542r\n");
 		} else {
 			err = ad3552r_configure_custom_gain(dac, child, ch);
 			if (err)
-				goto put_child;
+				return err;
 		}
 
 		ad3552r_calc_gain_and_offset(dac, ch);
@@ -1003,7 +994,7 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 
 		err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
 		if (err < 0)
-			goto put_child;
+			return err;
 
 		dac->channels[cnt] = AD3552R_CH_DAC(ch);
 		++cnt;
@@ -1021,10 +1012,6 @@ static int ad3552r_configure_device(struct ad3552r_desc *dac)
 	dac->num_ch = cnt;
 
 	return 0;
-put_child:
-	fwnode_handle_put(child);
-
-	return err;
 }
 
 static int ad3552r_init(struct ad3552r_desc *dac)
-- 
2.44.0

Re: [PATCH v6 7/8] iio: dac: ad3552r: Use device_for_each_child_node_scoped()

[Nuno Sá]

On Sat, 2024-03-30 at 18:53 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Removing the goto err; statements also allows more extensive use of
> dev_err_probe() further simplifying the code.
> 
> Cc: Mihail Chindris <mihail.chindris@analog.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Hmm, Mihail is no longer in ADI... I'll see what other drivers he's
"maintaining" and default them to me.

Anyways,

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad3552r.c | 51 +++++++++++++++------------------------
>  1 file changed, 19 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad3552r.c b/drivers/iio/dac/ad3552r.c
> index a492e8f2fc0f..e14a065b29ca 100644
> --- a/drivers/iio/dac/ad3552r.c
> +++ b/drivers/iio/dac/ad3552r.c
> @@ -880,7 +880,6 @@ static void ad3552r_reg_disable(void *reg)
>  static int ad3552r_configure_device(struct ad3552r_desc *dac)
>  {
>  	struct device *dev = &dac->spi->dev;
> -	struct fwnode_handle *child;
>  	struct regulator *vref;
>  	int err, cnt = 0, voltage, delta = 100000;
>  	u32 vals[2], val, ch;
> @@ -949,53 +948,45 @@ static int ad3552r_configure_device(struct ad3552r_desc
> *dac)
>  		return -ENODEV;
>  	}
>  
> -	device_for_each_child_node(dev, child) {
> +	device_for_each_child_node_scoped(dev, child) {
>  		err = fwnode_property_read_u32(child, "reg", &ch);
> -		if (err) {
> -			dev_err(dev, "mandatory reg property missing\n");
> -			goto put_child;
> -		}
> -		if (ch >= AD3552R_NUM_CH) {
> -			dev_err(dev, "reg must be less than %d\n",
> -				AD3552R_NUM_CH);
> -			err = -EINVAL;
> -			goto put_child;
> -		}
> +		if (err)
> +			return dev_err_probe(dev, err,
> +					     "mandatory reg property
> missing\n");
> +		if (ch >= AD3552R_NUM_CH)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "reg must be less than %d\n",
> +					     AD3552R_NUM_CH);
>  
>  		if (fwnode_property_present(child, "adi,output-range-
> microvolt")) {
>  			err = fwnode_property_read_u32_array(child,
>  							     "adi,output-
> range-microvolt",
>  							     vals,
>  							     2);
> -			if (err) {
> -				dev_err(dev,
> +			if (err)
> +				return dev_err_probe(dev, err,
>  					"adi,output-range-microvolt property
> could not be parsed\n");
> -				goto put_child;
> -			}
>  
>  			err = ad3552r_find_range(dac->chip_id, vals);
> -			if (err < 0) {
> -				dev_err(dev,
> -					"Invalid adi,output-range-microvolt
> value\n");
> -				goto put_child;
> -			}
> +			if (err < 0)
> +				return dev_err_probe(dev, err,
> +						     "Invalid adi,output-
> range-microvolt value\n");
> +
>  			val = err;
>  			err = ad3552r_set_ch_value(dac,
>  						  
> AD3552R_CH_OUTPUT_RANGE_SEL,
>  						   ch, val);
>  			if (err)
> -				goto put_child;
> +				return err;
>  
>  			dac->ch_data[ch].range = val;
>  		} else if (dac->chip_id == AD3542R_ID) {
> -			dev_err(dev,
> -				"adi,output-range-microvolt is required for
> ad3542r\n");
> -			err = -EINVAL;
> -			goto put_child;
> +			return dev_err_probe(dev, -EINVAL,
> +					     "adi,output-range-microvolt is
> required for ad3542r\n");
>  		} else {
>  			err = ad3552r_configure_custom_gain(dac, child, ch);
>  			if (err)
> -				goto put_child;
> +				return err;
>  		}
>  
>  		ad3552r_calc_gain_and_offset(dac, ch);
> @@ -1003,7 +994,7 @@ static int ad3552r_configure_device(struct ad3552r_desc
> *dac)
>  
>  		err = ad3552r_set_ch_value(dac, AD3552R_CH_SELECT, ch, 1);
>  		if (err < 0)
> -			goto put_child;
> +			return err;
>  
>  		dac->channels[cnt] = AD3552R_CH_DAC(ch);
>  		++cnt;
> @@ -1021,10 +1012,6 @@ static int ad3552r_configure_device(struct ad3552r_desc
> *dac)
>  	dac->num_ch = cnt;
>  
>  	return 0;
> -put_child:
> -	fwnode_handle_put(child);
> -
> -	return err;
>  }
>  
>  static int ad3552r_init(struct ad3552r_desc *dac)

Re: [PATCH v6 7/8] iio: dac: ad3552r: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

On Thu, 04 Apr 2024 11:14:17 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sat, 2024-03-30 at 18:53 +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Switching to the _scoped() version removes the need for manual
> > calling of fwnode_handle_put() in the paths where the code
> > exits the loop early. In this case that's all in error paths.
> > 
> > Removing the goto err; statements also allows more extensive use of
> > dev_err_probe() further simplifying the code.
> > 
> > Cc: Mihail Chindris <mihail.chindris@analog.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---  
> 
> Hmm, Mihail is no longer in ADI... I'll see what other drivers he's
> "maintaining" and default them to me.
Great.
> 
> Anyways,
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 
I'll carry on picking these series up piecemeal on basis of
reducing how many patches are outstanding as tags come in.

Applied.

[PATCH v6 8/8] iio: dac: ad5770r: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Switching to the _scoped() version removes the need for manual
calling of fwnode_handle_put() in the paths where the code
exits the loop early. In this case that's all in error paths.

Cc: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/dac/ad5770r.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/dac/ad5770r.c b/drivers/iio/dac/ad5770r.c
index f66d67402e43..c360ebf5297a 100644
--- a/drivers/iio/dac/ad5770r.c
+++ b/drivers/iio/dac/ad5770r.c
@@ -515,39 +515,32 @@ static int ad5770r_channel_config(struct ad5770r_state *st)
 {
 	int ret, tmp[2], min, max;
 	unsigned int num;
-	struct fwnode_handle *child;
 
 	num = device_get_child_node_count(&st->spi->dev);
 	if (num != AD5770R_MAX_CHANNELS)
 		return -EINVAL;
 
-	device_for_each_child_node(&st->spi->dev, child) {
+	device_for_each_child_node_scoped(&st->spi->dev, child) {
 		ret = fwnode_property_read_u32(child, "reg", &num);
 		if (ret)
-			goto err_child_out;
-		if (num >= AD5770R_MAX_CHANNELS) {
-			ret = -EINVAL;
-			goto err_child_out;
-		}
+			return ret;
+		if (num >= AD5770R_MAX_CHANNELS)
+			return -EINVAL;
 
 		ret = fwnode_property_read_u32_array(child,
 						     "adi,range-microamp",
 						     tmp, 2);
 		if (ret)
-			goto err_child_out;
+			return ret;
 
 		min = tmp[0] / 1000;
 		max = tmp[1] / 1000;
 		ret = ad5770r_store_output_range(st, min, max, num);
 		if (ret)
-			goto err_child_out;
+			return ret;
 	}
 
 	return 0;
-
-err_child_out:
-	fwnode_handle_put(child);
-	return ret;
 }
 
 static int ad5770r_init(struct ad5770r_state *st)
-- 
2.44.0

Re: [PATCH v6 8/8] iio: dac: ad5770r: Use device_for_each_child_node_scoped()

[Nuno Sá]

On Sat, 2024-03-30 at 18:53 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Switching to the _scoped() version removes the need for manual
> calling of fwnode_handle_put() in the paths where the code
> exits the loop early. In this case that's all in error paths.
> 
> Cc: Nuno Sá <nuno.sa@analog.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

Reviewed-by: Nuno Sa <nuno.sa@analog.com>

>  drivers/iio/dac/ad5770r.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/dac/ad5770r.c b/drivers/iio/dac/ad5770r.c
> index f66d67402e43..c360ebf5297a 100644
> --- a/drivers/iio/dac/ad5770r.c
> +++ b/drivers/iio/dac/ad5770r.c
> @@ -515,39 +515,32 @@ static int ad5770r_channel_config(struct ad5770r_state
> *st)
>  {
>  	int ret, tmp[2], min, max;
>  	unsigned int num;
> -	struct fwnode_handle *child;
>  
>  	num = device_get_child_node_count(&st->spi->dev);
>  	if (num != AD5770R_MAX_CHANNELS)
>  		return -EINVAL;
>  
> -	device_for_each_child_node(&st->spi->dev, child) {
> +	device_for_each_child_node_scoped(&st->spi->dev, child) {
>  		ret = fwnode_property_read_u32(child, "reg", &num);
>  		if (ret)
> -			goto err_child_out;
> -		if (num >= AD5770R_MAX_CHANNELS) {
> -			ret = -EINVAL;
> -			goto err_child_out;
> -		}
> +			return ret;
> +		if (num >= AD5770R_MAX_CHANNELS)
> +			return -EINVAL;
>  
>  		ret = fwnode_property_read_u32_array(child,
>  						     "adi,range-microamp",
>  						     tmp, 2);
>  		if (ret)
> -			goto err_child_out;
> +			return ret;
>  
>  		min = tmp[0] / 1000;
>  		max = tmp[1] / 1000;
>  		ret = ad5770r_store_output_range(st, min, max, num);
>  		if (ret)
> -			goto err_child_out;
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -err_child_out:
> -	fwnode_handle_put(child);
> -	return ret;
>  }
>  
>  static int ad5770r_init(struct ad5770r_state *st)

Re: [PATCH v6 8/8] iio: dac: ad5770r: Use device_for_each_child_node_scoped()

[Jonathan Cameron]

On Thu, 04 Apr 2024 11:11:11 +0200
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sat, 2024-03-30 at 18:53 +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Switching to the _scoped() version removes the need for manual
> > calling of fwnode_handle_put() in the paths where the code
> > exits the loop early. In this case that's all in error paths.
> > 
> > Cc: Nuno Sá <nuno.sa@analog.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---  
> 
> Reviewed-by: Nuno Sa <nuno.sa@analog.com>
> 
Applied.