diff mbox series

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

Message ID 20240428174020.1832825-2-jic23@kernel.org (mailing list archive)
State Accepted
Headers show
Series IIO: Use device_for_each_child_scope() | expand

Commit Message

Jonathan Cameron April 28, 2024, 5:40 p.m. UTC 
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(-)

Comments

marius.cristea@microchip.com April 28, 2024, 8:25 p.m. UTC | #1 
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
Marcelo Schmitt April 28, 2024, 8:25 p.m. UTC | #2 
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
> 
>
Jonathan Cameron April 29, 2024, 7:56 p.m. UTC | #3 
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
> > 
> >   
>
Jonathan Cameron April 29, 2024, 8 p.m. UTC | #4 
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
> > > 
> > >     
> >   
>
diff mbox series

Patch

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