Re: fwnode_for_each_child_node() and OF backend discrepancy

[Rafael J. Wysocki]

On Tue, Jun 28, 2022 at 12:32 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 27/06/2022 15:33, Rafael J. Wysocki wrote:
> > On Mon, Jun 27, 2022 at 3:08 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@xxxxxxxxxx> wrote:
> >>
> >> On 27/06/2022 14:49, Michael Walle wrote:
> >>> Hi,
> >>>
> >>> I tired to iterate over all child nodes, regardless if they are
> >>> available
> >>> or not. Now there is that handy fwnode_for_each_child_node() (and the
> >>> fwnode_for_each_available_child_node()). The only thing is the OF
> >>> backend
> >>> already skips disabled nodes [1], making fwnode_for_each_child_node()
> >>> and
> >>> fwnode_for_each_available_child_node() behave the same with the OF
> >>> backend.
> >>>
> >>> Doesn't seem to be noticed by anyone for now. I'm not sure how to fix
> >>> that
> >>> one. fwnode_for_each_child_node() and also fwnode_get_next_child_node()
> >>> are
> >>> used by a handful of drivers. I've looked at some, but couldn't decide
> >>> whether they really want to iterate over all child nodes or just the
> >>> enabled
> >>> ones.
> >>
> >> If I get it correctly, this was introduced  by 8a0662d9ed29 ("Driver
> >> core: Unified interface for firmware node properties")
> >> .
> >
> > Originally it was, but then it has been reworked a few times.
> >
> > The backend callbacks were introduced by Sakari, in particular.
>
> I see you as an author of 8a0662d9ed29 which adds
> device_get_next_child_node() and uses of_get_next_available_child()
> instead of of_get_next_child(). Although it was back in 2014, so maybe
> it will be tricky to get original intention. :)

The OF part of this was based on Grant's suggestions.  My
understanding at that time was that this was the right thing to do for
OF and nobody told me otherwise.

> Which commit do you mean when you refer to Sakari's work?

3708184afc77 device property: Move FW type specific functionality to
FW specific files

However, it didn't change the "available" vs "any" behavior for OF.

> >
> >> The question to Rafael - what was your intention when you added
> >> device_get_next_child_node() looking only for available nodes?
> >
> > That depends on the backend.
>
> We talk about OF backend. In your commit device_get_next_child_node for
> OF uses explicitly available node, not any node.

Yes, it does.

If that doesn't match the cases in which it is used, I guess it can be adjusted.

> > fwnode_for_each_available_child_node() is more specific and IIRC it
> > was introduced for fw_devlink (CC Saravana).
> >
> >> My understanding is that this implementation should be consistent with
> >> OF implementation, so fwnode_get_next_child_node=get any child.
> >
> > IIUC, the OF implementation is not consistent with the
> > fwnode_get_next_child_node=get any child thing.
> >
> >> However maybe ACPI treats it somehow differently?
> >
> > acpi_get_next_subnode() simply returns the next subnode it can find.

I guess that the confusion is related to what "available" means for ACPI and OF.

In the ACPI case it means "this a device object corresponding to a
device that is present".  In OF it is related to the "status" property
AFAICS.