On 21/03/2025 08:35, Matti Vaittinen wrote:
Hi dee Ho Laurent,
On 20/03/2025 16:26, Laurent Pinchart wrote:
Hi Matti,
On Thu, Mar 20, 2025 at 10:35:35AM +0200, Matti Vaittinen wrote:
It appears that the concept of available firmware nodes is not really
applicable to the scenarios where a specific name is required from a
node.
As explained[1] by Sakari:
"OF only enumerates available nodes via the fwnode API, software nodes
don't have the concept but on ACPI I guess you could have a difference
in nodes where you have device sub-nodes that aren't available. Still,
these ACPI device nodes don't have meaningful names in this context
(they're 4-character object names) so you wouldn't use them like this
anyway."
Use the fwnode_for_each_child_node() instead of the
fwnode_for_each_available_child_node() In order to make it clearly
visible that the 'availability' of the nodes does not need to be
considered here.
Why not ? Node availability is a concept that exists in DT, and this
driver has only been tested on DT-based systems.
I admit I need to study this then. I just took what Sakari said for
granted, without taking any further look at this.
I took a peek in the 'availability' concept and found:
https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/of/base.c#L468
So, the availability indeed has a well defined meaning in the DT,
boiling down to the value of the 'status' -property.
Then I took further look at the fwnode_for_each_child_node(), and if I'm
not mistaken, it calls:
fwnode_for_each_child_node()
fwnode_get_next_child_node()
fwnode_call_ptr_op(fwnode, get_next_child_node, child);
of_fwnode_get_next_child_node() (dt-based)
of_get_next_available_child() (dt-based)
where the of_get_next_available_child() skips all the disabled nodes.
So, in that regard I agree with Sakari. On DT based systems, the
fwnode_for_each_child_node() seems to equal the
fwnode_for_each_available_child_node().
And, since the 'thp7312' driver requires specific names for the nodes,
it indeed seems to me that only the device-tree use-case needs to be
considered.
After all this I'd say this patch is still valid - but the commit
message is misleading. If no one objects I'll rewrite the commit msg and
respin :)
Yours,
-- Matti
