[PATCH 0/2] IIO: Use of device_for_each_child_node_scoped() in ti-ads131e08

[PATCH 0/2] IIO: Use of device_for_each_child_node_scoped() in ti-ads131e08

[Lorenzo Bertin Salvador]

This series is making use of device_for_each_child_node_scoped() to avoid
the need to remember to call fwnode_handle_put() in early exits from
loops over the child nodes. It is also correcting a minor style problem
in drivers/iio/adc/ti-ads131e08.c

Lorenzo Bertin Salvador (2):
  iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped() to
    simplify error paths.
  iio: adc: ti-ads131e08: Fix a style problem in struct declaration.

 drivers/iio/adc/ti-ads131e08.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 1/2] iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped() to simplify error paths.

[Lorenzo Bertin Salvador]

This loop definition automatically releases the handle on early exit
reducing the chance of bugs that cause resource leaks.

Co-developed-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
Signed-off-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
Signed-off-by: Lorenzo Bertin Salvador <lorenzobs@usp.br>
---
 drivers/iio/adc/ti-ads131e08.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index fcfc46254..cb04a29b3 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;
 		}
@@ -785,9 +784,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
 
 	return 0;
 
-err_child_out:
-	fwnode_handle_put(node);
-	return ret;
 }
 
 static void ads131e08_regulator_disable(void *data)
-- 
2.25.1

[PATCH 2/2] iio: adc: ti-ads131e08: Fix a style problem in struct declaration.

[Lorenzo Bertin Salvador]

There was a blank line missing after declarations in struct tmp_buf.

Co-developed-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
Signed-off-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
Signed-off-by: Lorenzo Bertin Salvador <lorenzobs@usp.br>
---
 drivers/iio/adc/ti-ads131e08.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
index cb04a29b3..fcc0276bf 100644
--- a/drivers/iio/adc/ti-ads131e08.c
+++ b/drivers/iio/adc/ti-ads131e08.c
@@ -102,6 +102,7 @@ struct ads131e08_state {
 	struct completion completion;
 	struct {
 		u8 data[ADS131E08_NUM_DATA_BYTES_MAX];
+
 		s64 ts __aligned(8);
 	} tmp_buf;
 
-- 
2.25.1

Re: [PATCH 2/2] iio: adc: ti-ads131e08: Fix a style problem in struct declaration.

[Jonathan Cameron]

On Sat, 20 Apr 2024 15:27:44 -0300
Lorenzo Bertin Salvador <lorenzobs@usp.br> wrote:

> There was a blank line missing after declarations in struct tmp_buf.
> 
> Co-developed-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
> Signed-off-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
> Signed-off-by: Lorenzo Bertin Salvador <lorenzobs@usp.br>
That's a checkpatch.pl (I assume) bug.
These are both declarations, just the second one is more complex than most :)

Feel free to try and fix check patch but this isn't something we are going
to 'fix' in code.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ti-ads131e08.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
> index cb04a29b3..fcc0276bf 100644
> --- a/drivers/iio/adc/ti-ads131e08.c
> +++ b/drivers/iio/adc/ti-ads131e08.c
> @@ -102,6 +102,7 @@ struct ads131e08_state {
>  	struct completion completion;
>  	struct {
>  		u8 data[ADS131E08_NUM_DATA_BYTES_MAX];
> +
>  		s64 ts __aligned(8);
>  	} tmp_buf;
>  

Re: [PATCH 0/2] IIO: Use of device_for_each_child_node_scoped() in ti-ads131e08

[Jonathan Cameron]

On Sat, 20 Apr 2024 15:27:42 -0300
Lorenzo Bertin Salvador <lorenzobs@usp.br> wrote:

> This series is making use of device_for_each_child_node_scoped() to avoid
> the need to remember to call fwnode_handle_put() in early exits from
> loops over the child nodes. It is also correcting a minor style problem
> in drivers/iio/adc/ti-ads131e08.c
> 
> Lorenzo Bertin Salvador (2):
>   iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped() to
>     simplify error paths.
>   iio: adc: ti-ads131e08: Fix a style problem in struct declaration.

The title of the cover letter only reflects one of these patches and
needs to cover them both.  However, for tidying up type actions a vague
title like iio: adc: ti-ads131e08: code style improvements
would be fine as long as the description gives a more detailed listing
of what is going on.  That's covered by your description above.

Thanks,

Jonathan

> 
>  drivers/iio/adc/ti-ads131e08.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 

Re: [PATCH 1/2] iio: adc: ti-ads131e08: Use device_for_each_child_node_scoped() to simplify error paths.

[Jonathan Cameron]

On Sat, 20 Apr 2024 15:27:43 -0300
Lorenzo Bertin Salvador <lorenzobs@usp.br> wrote:

> This loop definition automatically releases the handle on early exit
> reducing the chance of bugs that cause resource leaks.
> 
> Co-developed-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
> Signed-off-by: Briza Mel Dias de Sousa <brizamel.dias@usp.br>
> Signed-off-by: Lorenzo Bertin Salvador <lorenzobs@usp.br>
Good patch.
Applied to the togreg branch of iio.git, but I'll only push it out
as testing today so that 0-day (an autobuilder) can see if it can
find anything that we have missed.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ti-ads131e08.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads131e08.c b/drivers/iio/adc/ti-ads131e08.c
> index fcfc46254..cb04a29b3 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;
>  		}
> @@ -785,9 +784,6 @@ static int ads131e08_alloc_channels(struct iio_dev *indio_dev)
>  
>  	return 0;
>  
> -err_child_out:
> -	fwnode_handle_put(node);
> -	return ret;
>  }
>  
>  static void ads131e08_regulator_disable(void *data)