Re: fwnode_for_each_child_node() and OF backend discrepancy

From: Michael Walle
Date: Tue Jun 28 2022 - 07:36:51 EST


Am 2022-06-28 13:10, schrieb Andy Shevchenko:
On Mon, Jun 27, 2022 at 02:49:51PM +0200, 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.

Any thoughts?

It was discussed at least twice this year (in regard to some new IIO drivers)
and Rob told that iterating over disabled (not available) nodes in OF kinda
legacy/design mistake. That's why device_for_each_child_node() goes only
over available nodes only.

Mh, but then the fwnode_for_each_child_node() is very misleading, esp.
with the presence of fwnode_for_each_available_child_node().

So, why do you need to iterate over disabled ones?

I was trying to fix the lan966x driver [1] which doesn't work if there
are disabled nodes in between. My steps would have been:
(1) change fwnode_for_each_available_child_node() to
fwnode_for_each_child_node(), maybe with a fixes tag, as it's
easy to backport
(2) introduce new compatibles and deduce the number of ports
according to the compatible string and not by counting
the child nodes.
(3) keep the old behavior for the legacy compatible and mark it
as deprecated in the binding
(4) move the device tree over to the new compatible string

-michael

[1] https://elixir.bootlin.com/linux/v5.19-rc4/source/drivers/net/ethernet/microchip/lan966x/lan966x_main.c#L1029