Re: [PATCH v2 03/14] device property: Introduce device_for_each_child_node_scoped()

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

On Mon, 12 Feb 2024 14:10:57 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Sun, Feb 11, 2024 at 07:25:29PM +0000, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > Similar to recently propose for_each_child_of_node_scoped() this
> > new version of the loop macro instantiates a new local
> > struct fwnode_handle * that uses the __free(fwnode_handle) auto
> > cleanup handling so that if a reference to a node is held on early
> > exit from the loop the reference will be released. If the loop
> > runs to completion, the child pointer will be NULL and no action will
> > be taken.
> > 
> > The reason this is useful is that it removes the need for
> > fwnode_handle_put() on early loop exits.  If there is a need
> > to retain the reference, then return_ptr(child) or no_free_ptr(child)
> > may be used to safely disable the auto cleanup.  
> 
> ...
> 
> > +#define device_for_each_child_node_scoped(dev, child)\  
> 
> Missing space before backslash, but I would rather to make them to be TABed to
> the same column.

Oops. I spotted I messed this up bug clearly failed to fix it before sending out.

> 
> > +	for (struct fwnode_handle *child __free(fwnode_handle) = \
> > +	     device_get_next_child_node(dev, NULL); child; \  
> 
> Please, move child to a separate line, so we will easily see the all three
> parameters of the for-loop. That said, indent the assignment to the right as
> well.
Indent makes sense - but (to save another respin) how far?
Next tab stop will be a bit random looking but I guess nothing else
makes more sense.

> 
> > +	     child = device_get_next_child_node(dev, child))  
> 
> With the above addressed,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Thanks,
>