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

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

[Jonathan Cameron]

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

Looking for review of these remaining patches from v6.
Chances since V6 (dropped the ones that I applied):

I'm slimming this series down by taking patches that have been reviewed
but nothing yet on these ones yet.  They are fairly simple so if anyone
has time to look that would be great.

Jonathan Cameron (2):
  iio: adc: mcp3564: Use device_for_each_child_node_scoped()
  iio: adc: ti-ads1015: Use device_for_each_child_node_scoped()

 drivers/iio/adc/mcp3564.c    | 16 +++++++---------
 drivers/iio/adc/ti-ads1015.c |  5 +----
 2 files changed, 8 insertions(+), 13 deletions(-)

-- 
2.44.0

[PATCH v6 1/2] 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/2] 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>
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
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

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

[Marius.Cristea]

Hi Jonathan,



On Sun, 2024-04-28 at 18:40 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> 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
> 


  The changes looks OK,

Reviewed-by: Marius Cristea <marius.cristea@microchip.com>


Thanks,
Marius

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

[Marcelo Schmitt]

Hi Jonathan,

This also looks good to me.

Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

On 04/28, 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: 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
> 
> 

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

[Jonathan Cameron]

On Sun, 28 Apr 2024 17:25:57 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> Hi Jonathan,
> 
> This also looks good to me.
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>
Thanks,

Applied.

1 to go. Someone must want to give the ads1015 patch the love
it needs? :)

Jonathan

> 
> On 04/28, 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: 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
> > 
> >   
> 

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

[Jonathan Cameron]

On Mon, 29 Apr 2024 20:56:23 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Sun, 28 Apr 2024 17:25:57 -0300
> Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:
> 
> > Hi Jonathan,
> > 
> > This also looks good to me.
> > 
> > Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>  
> Thanks,
> 
> Applied.
> 
> 1 to go. Someone must want to give the ads1015 patch the love
> it needs? :)
Ah. There was another patch for the same driver doing the same
thing. I picked that one up instead. Problem solved.

Jonathan

> 
> Jonathan
> 
> > 
> > On 04/28, 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: 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
> > > 
> > >     
> >   
>