diff mbox series

[09/13] iio: addac: ad74413r: Use __free(fwnode_handle) to replace fwnode_handle_put() calls

Message ID 20240114172009.179893-10-jic23@kernel.org (mailing list archive)
State Superseded
Headers show
Series device property / IIO: Use cleanup.h magic for fwnode_handle_put() handling. | expand

Commit Message

Jonathan Cameron Jan. 14, 2024, 5:20 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

This use of the new cleanup.h scope based freeing infrastructure allows
us to exit directly from error conditions within the
fwnode_for_each_available_child_node(dev, child) loop. On normal exit
from that loop no fwnode_handle reference will be held and the child
pointer will be NULL thus making the automatically run
fwnode_handle_put() a noop.

Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/iio/addac/ad74413r.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

Comments

Nuno Sá Jan. 15, 2024, 10:17 a.m. UTC | #1
On Sun, 2024-01-14 at 17:20 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> This use of the new cleanup.h scope based freeing infrastructure allows
> us to exit directly from error conditions within the
> fwnode_for_each_available_child_node(dev, child) loop. On normal exit
> from that loop no fwnode_handle reference will be held and the child
> pointer will be NULL thus making the automatically run
> fwnode_handle_put() a noop.
> 
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---

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

>  drivers/iio/addac/ad74413r.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> index 7af3e4b8fe3b..ec9a466e118d 100644
> --- a/drivers/iio/addac/ad74413r.c
> +++ b/drivers/iio/addac/ad74413r.c
> @@ -1255,21 +1255,16 @@ static int ad74413r_parse_channel_config(struct
> iio_dev *indio_dev,
>  static int ad74413r_parse_channel_configs(struct iio_dev *indio_dev)
>  {
>  	struct ad74413r_state *st = iio_priv(indio_dev);
> -	struct fwnode_handle *channel_node = NULL;
> +	struct fwnode_handle *channel_node __free(fwnode_handle) = NULL;
>  	int ret;
>  
>  	fwnode_for_each_available_child_node(dev_fwnode(st->dev),
> channel_node) {
>  		ret = ad74413r_parse_channel_config(indio_dev, channel_node);
>  		if (ret)
> -			goto put_channel_node;
> +			return ret;
>  	}
>  
>  	return 0;
> -
> -put_channel_node:
> -	fwnode_handle_put(channel_node);
> -
> -	return ret;
>  }
>  
>  static int ad74413r_setup_channels(struct iio_dev *indio_dev)
Jonathan Cameron Feb. 11, 2024, 6:53 p.m. UTC | #2
On Mon, 15 Jan 2024 11:17:12 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Sun, 2024-01-14 at 17:20 +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > This use of the new cleanup.h scope based freeing infrastructure allows
> > us to exit directly from error conditions within the
> > fwnode_for_each_available_child_node(dev, child) loop. On normal exit
> > from that loop no fwnode_handle reference will be held and the child
> > pointer will be NULL thus making the automatically run
> > fwnode_handle_put() a noop.
> > 
> > Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---  
> 
> Acked-by: Nuno Sa <nuno.sa@analog.com>
> 
> >  drivers/iio/addac/ad74413r.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
> > index 7af3e4b8fe3b..ec9a466e118d 100644
> > --- a/drivers/iio/addac/ad74413r.c
> > +++ b/drivers/iio/addac/ad74413r.c
> > @@ -1255,21 +1255,16 @@ static int ad74413r_parse_channel_config(struct
> > iio_dev *indio_dev,
> >  static int ad74413r_parse_channel_configs(struct iio_dev *indio_dev)
> >  {
> >  	struct ad74413r_state *st = iio_priv(indio_dev);
> > -	struct fwnode_handle *channel_node = NULL;
> > +	struct fwnode_handle *channel_node __free(fwnode_handle) = NULL;
> >  	int ret;
> >  
> >  	fwnode_for_each_available_child_node(dev_fwnode(st->dev),
This should have been
device_for_each_child_node() because that ultimately calls
of_fwnode_get_next_child_node() which calls the available form anyway.
https://lore.kernel.org/lkml/20211205190101.26de4a57@jic23-huawei/T/#u

So I'll just switch to the new
device_for_each_child_node_scoped() that I'm about to propose.


> > channel_node) {
> >  		ret = ad74413r_parse_channel_config(indio_dev, channel_node);
> >  		if (ret)
> > -			goto put_channel_node;
> > +			return ret;
> >  	}
> >  
> >  	return 0;
> > -
> > -put_channel_node:
> > -	fwnode_handle_put(channel_node);
> > -
> > -	return ret;
> >  }
> >  
> >  static int ad74413r_setup_channels(struct iio_dev *indio_dev)  
>
diff mbox series

Patch

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index 7af3e4b8fe3b..ec9a466e118d 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -1255,21 +1255,16 @@  static int ad74413r_parse_channel_config(struct iio_dev *indio_dev,
 static int ad74413r_parse_channel_configs(struct iio_dev *indio_dev)
 {
 	struct ad74413r_state *st = iio_priv(indio_dev);
-	struct fwnode_handle *channel_node = NULL;
+	struct fwnode_handle *channel_node __free(fwnode_handle) = NULL;
 	int ret;
 
 	fwnode_for_each_available_child_node(dev_fwnode(st->dev), channel_node) {
 		ret = ad74413r_parse_channel_config(indio_dev, channel_node);
 		if (ret)
-			goto put_channel_node;
+			return ret;
 	}
 
 	return 0;
-
-put_channel_node:
-	fwnode_handle_put(channel_node);
-
-	return ret;
 }
 
 static int ad74413r_setup_channels(struct iio_dev *indio_dev)