On 19/03/2025 17:23, Sakari Ailus wrote:
On Wed, Mar 19, 2025 at 08:02:24AM +0200, Matti Vaittinen wrote:
On 18/03/2025 17:24, Sakari Ailus wrote:
On Mon, Mar 17, 2025 at 05:50:38PM +0200, Matti Vaittinen wrote:
There are a few use-cases where child nodes with a specific name need to
be parsed. Code like:
...
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -167,10 +167,18 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
for (child = fwnode_get_next_child_node(fwnode, NULL); child; \
child = fwnode_get_next_child_node(fwnode, child))
+#define fwnode_for_each_named_child_node(fwnode, child, name) \
+ fwnode_for_each_child_node(fwnode, child) \
+ if (!fwnode_name_eq(child, name)) { } else
+
#define fwnode_for_each_available_child_node(fwnode, child) \
for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
child = fwnode_get_next_available_child_node(fwnode, child))
+#define fwnode_for_each_available_named_child_node(fwnode, child, name) \
+ fwnode_for_each_available_child_node(fwnode, child) \
+ if (!fwnode_name_eq(child, name)) { } else
+
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.
I believe you have far better understanding on these concepts than I do. The
reason behind adding fwnode_for_each_available_child_node() was the patch
10/10:
- fwnode_for_each_available_child_node(sensors, node) {
- if (fwnode_name_eq(node, "sensor")) {
- if (!thp7312_sensor_parse_dt(thp7312, node))
- num_sensors++;
- }
+ fwnode_for_each_available_named_child_node(sensors, node, "sensor") {
+ if (!thp7312_sensor_parse_dt(thp7312, node))
+ num_sensors++;
}
So my question is: is it useful to provide this besides
fwnode_for_each_named_child_node(), given that both are effectively the
same?
So, I suppose you're saying the existing thp7312 -driver has no real reason
to use the 'fwnode_for_each_available_child_node()', but it could be using
fwnode_for_each_child_node() instead?
If so, I am Ok with dropping the
'fwnode_for_each_available_named_child_node()' and changing the 10/10 to:
- fwnode_for_each_available_child_node(sensors, node) {
- if (fwnode_name_eq(node, "sensor")) {
- if (!thp7312_sensor_parse_dt(thp7312, node))
- num_sensors++;
- }
+ fwnode_for_each_named_child_node(sensors, node, "sensor") {
+ if (!thp7312_sensor_parse_dt(thp7312, node))
+ num_sensors++;
}
Do you think that'd be correct?
I'd say so. Feel free to cc me to the last patch as well.
Thanks. I'll drop the fwnode_for_each_available_named_child_node() then.
I guess one way to make this clearer is to switch to
fwnode_for_each_child_node() in a separate patch before
fwnode_for_each_named_child_node() conversion.
I suppose this makes sense.
I think this series can't make it to 6.15-rc1. Meaning, these
*_named_*() APIs perhaps land in 6.16-rc1. I assume these *_named_*()
APIs will go through the IIO. This rather simple IIO driver's review
took longer than I predicted, with more versions I intended (as always)
- and I kind of dislike respinning the whole series, with this large
audience, when changes are not interesting to the most.
Maybe it is simplest to drop the thp7312 (and gianfar) from this series,
and respin them only when the 6.16-rc1 is out. It's going to be couple
of months though - so there's always a risk that I forget.
The proposed change for the thp7312, from
fwnode_for_each_available_child_node() to fwnode_for_each_child_node()
can be done earlier though.
There are also just a handful of users of
fwnode_for_each_available_child_node() and I guess these could be
converted, too, but I think it's outside the scope of the set.
Definitely not in the scope of the bd79124 support :)
Yours,
-- Matti
