Re: [PATCH v2 01/14] device property: Add cleanup.h based fwnode_handle_put() scope based cleanup.

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 12 Feb 2024 08:49:23 +0000
Sakari Ailus <sakari.ailus@linux.intel.com> wrote:

> Hi Jonathan,
> 
> On Sun, Feb 11, 2024 at 07:25:27PM +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Useful where the fwnode_handle was obtained from a call such as
> > fwnode_find_reference() as it will safely do nothing if IS_ERR() is true
> > and will automatically release the reference on the variable leaving
> > scope.
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  include/linux/property.h | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index e6516d0b7d52..bcda028f1a33 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -12,6 +12,7 @@
> >  
> >  #include <linux/args.h>
> >  #include <linux/bits.h>
> > +#include <linux/cleanup.h>
> >  #include <linux/fwnode.h>
> >  #include <linux/stddef.h>
> >  #include <linux/types.h>
> > @@ -188,6 +189,8 @@ struct fwnode_handle *device_get_named_child_node(const struct device *dev,
> >  
> >  struct fwnode_handle *fwnode_handle_get(struct fwnode_handle *fwnode);
> >  void fwnode_handle_put(struct fwnode_handle *fwnode);
> > +DEFINE_FREE(fwnode_handle, struct fwnode_handle *,
> > +	    if (!IS_ERR_OR_NULL(_T)) fwnode_handle_put(_T))  
> 
> fwnode_handle_put() can be safely called on NULL or error pointer fwnode so
> you can remove the check.

Was discussed in the RFC thread (where i didn't have this protection)

https://lore.kernel.org/linux-iio/20240108125117.000010fb@Huawei.com/
includes a reference to Linus Torvald's view on this.

All comes down to compiler visibility and optimization opportunities, which are improved
if the check is in the macro definition.

Jonathan
> 
> >  
> >  int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index);
> >  int fwnode_irq_get_byname(const struct fwnode_handle *fwnode, const char *name);  
>