Re: [PATCH v2 3/3] hwmon: (ltc2992) Use fwnode_for_each_available_child_node_scoped()

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2024-05-27 at 17:30 +0300, Andy Shevchenko wrote:
> Sun, May 26, 2024 at 02:48:51PM +0100, Jonathan Cameron kirjoitti:
> > On Thu, 23 May 2024 17:47:16 +0200
> > Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote:
> > 
> > > The scoped version of the fwnode_for_each_available_child_node() macro
> > > automates object recfount decrement, avoiding possible memory leaks
> > > in new error paths inside the loop like it happened when
> > > commit '10b029020487 ("hwmon: (ltc2992) Avoid division by zero")'
> > > was added.
> > > 
> > > The new macro removes the need to manually call fwnode_handle_put() in
> > > the existing error paths and in any future addition. It also removes the
> > > need for the current child node declaration as well, as it is internally
> > > declared.
> > > 
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> > 
> > This looks like another instances of the lack of clarify about 
> > what device_for_each_child_node[_scoped]() guarantees about node
> > availability.
> > On DT it guarantees the node is available as ultimately calls
> > of_get_next_available_child()
> > 
> > On ACPI it doesn't (I think).
> > For swnode, there isn't an obvious concept of available.
> > 
> > It would be much better if we reached some agreement on this and
> > hence could avoid using the fwnode variants just to get the _available_ form
> > as done here.
> 
> > Or just add the device_for_each_available_child_node[_scoped]()
> > and call that in almost all cases.
> 
> device_for_each*() _implies_ availability. You need to talk to Rob about all
> this. The design of the device_for_each*() was exactly done in accordance with
> his suggestions...
> 
> > In generic code, do we ever want to walk unavailable child nodes?
> 
> ...which are most likely like your question here, i.e. why we ever need to
> traverse over unavailable nodes.
> 

I have some vague idea of Rob talking about CPUs being one of the reasons for
the current design. Don't remember for sure. At least (if not already) having
this clearly documented would be nice.

- Nuno Sá