Add scoped versions of fwnode child node iterators that automatically
handle reference counting cleanup using the __free() attribute:
- fwnode_for_each_child_node_scoped()
- fwnode_for_each_available_child_node_scoped()
These macros follow the same pattern as existing scoped iterators in the
kernel, ensuring fwnode references are automatically released when the
iterator variable goes out of scope. This prevents resource leaks and
eliminates the need for manual cleanup in error paths.
The implementation mirrors the non-scoped variants but uses
__free(fwnode_handle) for automatic resource management, providing a
safer and more convenient interface for drivers iterating over firmware
node children.
Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
Notes:
checkpatch reports false positives that are intentionally ignored:
MACRO_ARG_REUSE, MACRO_ARG_PRECEDENCE
This is a standard iterator pattern following kernel conventions.
include/linux/property.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/linux/property.h b/include/linux/property.h
index 82f0cb3ab..862e20813 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -176,6 +176,16 @@ struct fwnode_handle *fwnode_get_next_available_child_node(
for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\
child = fwnode_get_next_available_child_node(fwnode, child))
+#define fwnode_for_each_child_node_scoped(fwnode, child) \
+ for (struct fwnode_handle *child __free(fwnode_handle) = \
+ fwnode_get_next_child_node(fwnode, NULL); \
+ child; child = fwnode_get_next_child_node(fwnode, child))
+
+#define fwnode_for_each_available_child_node_scoped(fwnode, child) \
+ for (struct fwnode_handle *child __free(fwnode_handle) = \
+ fwnode_get_next_available_child_node(fwnode, NULL); \
+ child; child = fwnode_get_next_available_child_node(fwnode, child))
+
struct fwnode_handle *device_get_next_child_node(const struct device *dev,
struct fwnode_handle *child);
--
2.43.0
On Tue, Sep 02, 2025 at 03:04:39PM -0400, Jean-François Lessard wrote: > Add scoped versions of fwnode child node iterators that automatically > handle reference counting cleanup using the __free() attribute: > > - fwnode_for_each_child_node_scoped() > - fwnode_for_each_available_child_node_scoped() > > These macros follow the same pattern as existing scoped iterators in the > kernel, ensuring fwnode references are automatically released when the > iterator variable goes out of scope. This prevents resource leaks and > eliminates the need for manual cleanup in error paths. > > The implementation mirrors the non-scoped variants but uses > __free(fwnode_handle) for automatic resource management, providing a > safer and more convenient interface for drivers iterating over firmware > node children. > > Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> Applied to for-next, thanks!
Hi Jean-François, On Tue, Sep 02, 2025 at 03:04:39PM -0400, Jean-François Lessard wrote: > Add scoped versions of fwnode child node iterators that automatically > handle reference counting cleanup using the __free() attribute: > > - fwnode_for_each_child_node_scoped() > - fwnode_for_each_available_child_node_scoped() > > These macros follow the same pattern as existing scoped iterators in the > kernel, ensuring fwnode references are automatically released when the > iterator variable goes out of scope. This prevents resource leaks and > eliminates the need for manual cleanup in error paths. > > The implementation mirrors the non-scoped variants but uses > __free(fwnode_handle) for automatic resource management, providing a > safer and more convenient interface for drivers iterating over firmware > node children. > > Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> > --- > > Notes: > checkpatch reports false positives that are intentionally ignored: > MACRO_ARG_REUSE, MACRO_ARG_PRECEDENCE > This is a standard iterator pattern following kernel conventions. > > include/linux/property.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/linux/property.h b/include/linux/property.h > index 82f0cb3ab..862e20813 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -176,6 +176,16 @@ struct fwnode_handle *fwnode_get_next_available_child_node( > for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\ > child = fwnode_get_next_available_child_node(fwnode, child)) > > +#define fwnode_for_each_child_node_scoped(fwnode, child) \ > + for (struct fwnode_handle *child __free(fwnode_handle) = \ > + fwnode_get_next_child_node(fwnode, NULL); \ > + child; child = fwnode_get_next_child_node(fwnode, child)) > + > +#define fwnode_for_each_available_child_node_scoped(fwnode, child) \ > + for (struct fwnode_handle *child __free(fwnode_handle) = \ > + fwnode_get_next_available_child_node(fwnode, NULL); \ > + child; child = fwnode_get_next_available_child_node(fwnode, child)) > + Do we really need the available variant? Please see <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. I'll post a patch to remove fwnode_get_next_available_child_node(), too. > struct fwnode_handle *device_get_next_child_node(const struct device *dev, > struct fwnode_handle *child); > -- Kind regards, Sakari Ailus
(Cc: Javier) On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: > Do we really need the available variant? > > Please see > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. > > I'll post a patch to remove fwnode_get_next_available_child_node(), too. Either I'm missing something substantial or the link does indeed not provide an obvious justification of why you want to send a patch to remove fwnode_get_next_available_child_node(). Do you mean to say that all fwnode backends always return true for device_is_available() and hence the fwnode API should not make this distinction? I.e. are you referring to the fact that of_fwnode_get_next_child_node() always calls of_get_next_available_child() and swnode has no device_is_available() callback and hence is always available? What about ACPI?
Hi Danilo, On Wed, Sep 03, 2025 at 07:22:29PM +0200, Danilo Krummrich wrote: > (Cc: Javier) > > On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: > > Do we really need the available variant? > > > > Please see > > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. > > > > I'll post a patch to remove fwnode_get_next_available_child_node(), too. > > Either I'm missing something substantial or the link does indeed not provide an > obvious justification of why you want to send a patch to remove > fwnode_get_next_available_child_node(). > > Do you mean to say that all fwnode backends always return true for > device_is_available() and hence the fwnode API should not make this distinction? > > I.e. are you referring to the fact that of_fwnode_get_next_child_node() always > calls of_get_next_available_child() and swnode has no device_is_available() > callback and hence is always available? What about ACPI? On ACPI there's no such concept on ACPI data nodes so all data nodes are considered to be available. So effectively the fwnode_*available*() is always the same as the variant without _available(). -- Regards, Sakari Ailus
On Thu Sep 4, 2025 at 7:56 AM CEST, Sakari Ailus wrote: > Hi Danilo, > > On Wed, Sep 03, 2025 at 07:22:29PM +0200, Danilo Krummrich wrote: >> (Cc: Javier) >> >> On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: >> > Do we really need the available variant? >> > >> > Please see >> > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. >> > >> > I'll post a patch to remove fwnode_get_next_available_child_node(), too. >> >> Either I'm missing something substantial or the link does indeed not provide an >> obvious justification of why you want to send a patch to remove >> fwnode_get_next_available_child_node(). >> >> Do you mean to say that all fwnode backends always return true for >> device_is_available() and hence the fwnode API should not make this distinction? >> >> I.e. are you referring to the fact that of_fwnode_get_next_child_node() always >> calls of_get_next_available_child() and swnode has no device_is_available() >> callback and hence is always available? What about ACPI? > > On ACPI there's no such concept on ACPI data nodes so all data nodes are > considered to be available. So effectively the fwnode_*available*() is > always the same as the variant without _available(). What about acpi_fwnode_device_is_available()? Is it guaranteed to always evaluate to true? If so, to you plan to remove device_is_available() from struct fwnode_operations and fixup all users of fwnode_get_next_available_child_node() and fwnode_for_each_available_child_node() as well?
Hi Danilo, On Thu, Sep 04, 2025 at 09:03:44AM +0200, Danilo Krummrich wrote: > On Thu Sep 4, 2025 at 7:56 AM CEST, Sakari Ailus wrote: > > Hi Danilo, > > > > On Wed, Sep 03, 2025 at 07:22:29PM +0200, Danilo Krummrich wrote: > >> (Cc: Javier) > >> > >> On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: > >> > Do we really need the available variant? > >> > > >> > Please see > >> > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. > >> > > >> > I'll post a patch to remove fwnode_get_next_available_child_node(), too. > >> > >> Either I'm missing something substantial or the link does indeed not provide an > >> obvious justification of why you want to send a patch to remove > >> fwnode_get_next_available_child_node(). > >> > >> Do you mean to say that all fwnode backends always return true for > >> device_is_available() and hence the fwnode API should not make this distinction? > >> > >> I.e. are you referring to the fact that of_fwnode_get_next_child_node() always > >> calls of_get_next_available_child() and swnode has no device_is_available() > >> callback and hence is always available? What about ACPI? > > > > On ACPI there's no such concept on ACPI data nodes so all data nodes are > > considered to be available. So effectively the fwnode_*available*() is > > always the same as the variant without _available(). > > What about acpi_fwnode_device_is_available()? Is it guaranteed to always > evaluate to true? acpi_fwnode_device_is_available() is different as it works on ACPI device nodes having availability information. > > If so, to you plan to remove device_is_available() from struct > fwnode_operations and fixup all users of fwnode_get_next_available_child_node() > and fwnode_for_each_available_child_node() as well? The device_is_available() callback needs to stay; it has valid uses elsewhere. Technically it is possible that fwnode_*child_node() functions could return device nodes that aren't available, but it is unlikely any caller would want to enumerate device nodes this way. Even so, I think it'd be the best to add an explicit availability check on ACPI side as well so only available nodes would be returned. The fact that none of the drivers using the two available variants acting on child nodes had ACPI ID table suggests that the use of the variants was motivated solely to use a function named similarly to the OF version. -- Kind regards, Sakari Ailus
On Thu Sep 4, 2025 at 9:43 AM CEST, Sakari Ailus wrote: > Hi Danilo, > > On Thu, Sep 04, 2025 at 09:03:44AM +0200, Danilo Krummrich wrote: >> On Thu Sep 4, 2025 at 7:56 AM CEST, Sakari Ailus wrote: >> > Hi Danilo, >> > >> > On Wed, Sep 03, 2025 at 07:22:29PM +0200, Danilo Krummrich wrote: >> >> (Cc: Javier) >> >> >> >> On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: >> >> > Do we really need the available variant? >> >> > >> >> > Please see >> >> > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. >> >> > >> >> > I'll post a patch to remove fwnode_get_next_available_child_node(), too. >> >> >> >> Either I'm missing something substantial or the link does indeed not provide an >> >> obvious justification of why you want to send a patch to remove >> >> fwnode_get_next_available_child_node(). >> >> >> >> Do you mean to say that all fwnode backends always return true for >> >> device_is_available() and hence the fwnode API should not make this distinction? >> >> >> >> I.e. are you referring to the fact that of_fwnode_get_next_child_node() always >> >> calls of_get_next_available_child() and swnode has no device_is_available() >> >> callback and hence is always available? What about ACPI? >> > >> > On ACPI there's no such concept on ACPI data nodes so all data nodes are >> > considered to be available. So effectively the fwnode_*available*() is >> > always the same as the variant without _available(). >> >> What about acpi_fwnode_device_is_available()? Is it guaranteed to always >> evaluate to true? > > acpi_fwnode_device_is_available() is different as it works on ACPI device > nodes having availability information. Well, it works on both data and device nodes, so considering data nodes only isn't enough, no? So, we can't just say fwnode_get_next_available_child_node() and fwnode_get_next_child_node() can be used interchangeably. >> If so, to you plan to remove device_is_available() from struct >> fwnode_operations and fixup all users of fwnode_get_next_available_child_node() >> and fwnode_for_each_available_child_node() as well? > > The device_is_available() callback needs to stay; it has valid uses > elsewhere. > > Technically it is possible that fwnode_*child_node() functions could return > device nodes that aren't available, but it is unlikely any caller would > want to enumerate device nodes this way. Even so, I think it'd be the best > to add an explicit availability check on ACPI side as well so only > available nodes would be returned. Fair enough, but that's an entirely different rationale than the one you gave above. I.e. "all iterators should only ever provide available ones" vs. the "they're all available anyways" argument above. (Quote: "So effectively the fwnode_*available*() is always the same as the variant without _available().") I see quite some drivers using fwnode_for_each_child_node() without any availability check. However, they may just rely on implementation details, such as knowing it's an OF node or ACPI data node, etc. So, before you remove fwnode_get_next_available_child_node(), do you plan to change the semantics of the get_next_child_node() callback accordingly, including adding the availability check on the ACPI side? How do we ensure there are no existing drivers relying on iterating also unavailble nodes? Above you say it's unlikely anyone actually wants this, but are we sure?
Hi Danilo, On Thu, Sep 04, 2025 at 10:51:15AM +0200, Danilo Krummrich wrote: > On Thu Sep 4, 2025 at 9:43 AM CEST, Sakari Ailus wrote: > > Hi Danilo, > > > > On Thu, Sep 04, 2025 at 09:03:44AM +0200, Danilo Krummrich wrote: > >> On Thu Sep 4, 2025 at 7:56 AM CEST, Sakari Ailus wrote: > >> > Hi Danilo, > >> > > >> > On Wed, Sep 03, 2025 at 07:22:29PM +0200, Danilo Krummrich wrote: > >> >> (Cc: Javier) > >> >> > >> >> On Wed Sep 3, 2025 at 3:18 PM CEST, Sakari Ailus wrote: > >> >> > Do we really need the available variant? > >> >> > > >> >> > Please see > >> >> > <URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. > >> >> > > >> >> > I'll post a patch to remove fwnode_get_next_available_child_node(), too. > >> >> > >> >> Either I'm missing something substantial or the link does indeed not provide an > >> >> obvious justification of why you want to send a patch to remove > >> >> fwnode_get_next_available_child_node(). > >> >> > >> >> Do you mean to say that all fwnode backends always return true for > >> >> device_is_available() and hence the fwnode API should not make this distinction? > >> >> > >> >> I.e. are you referring to the fact that of_fwnode_get_next_child_node() always > >> >> calls of_get_next_available_child() and swnode has no device_is_available() > >> >> callback and hence is always available? What about ACPI? > >> > > >> > On ACPI there's no such concept on ACPI data nodes so all data nodes are > >> > considered to be available. So effectively the fwnode_*available*() is > >> > always the same as the variant without _available(). > >> > >> What about acpi_fwnode_device_is_available()? Is it guaranteed to always > >> evaluate to true? > > > > acpi_fwnode_device_is_available() is different as it works on ACPI device > > nodes having availability information. > > Well, it works on both data and device nodes, so considering data nodes only > isn't enough, no? > > So, we can't just say fwnode_get_next_available_child_node() and > fwnode_get_next_child_node() can be used interchangeably. > > >> If so, to you plan to remove device_is_available() from struct > >> fwnode_operations and fixup all users of fwnode_get_next_available_child_node() > >> and fwnode_for_each_available_child_node() as well? > > > > The device_is_available() callback needs to stay; it has valid uses > > elsewhere. > > > > Technically it is possible that fwnode_*child_node() functions could return > > device nodes that aren't available, but it is unlikely any caller would > > want to enumerate device nodes this way. Even so, I think it'd be the best > > to add an explicit availability check on ACPI side as well so only > > available nodes would be returned. > > Fair enough, but that's an entirely different rationale than the one you gave > above. I.e. "all iterators should only ever provide available ones" vs. the > "they're all available anyways" argument above. > > (Quote: "So effectively the fwnode_*available*() is always the same as the > variant without _available().") This was perhaps a simplification. The word "effectively" is crucial here. > > I see quite some drivers using fwnode_for_each_child_node() without any > availability check. However, they may just rely on implementation details, such > as knowing it's an OF node or ACPI data node, etc. > > So, before you remove fwnode_get_next_available_child_node(), do you plan to > change the semantics of the get_next_child_node() callback accordingly, > including adding the availability check on the ACPI side? > > How do we ensure there are no existing drivers relying on iterating also > unavailble nodes? Above you say it's unlikely anyone actually wants this, but > are we sure? If you're concerned of the use on ACPI platforms, none of the drivers using the two available variants list any ACPI IDs, signifying they're not used on ACPI systems -- I don't think they ever have been. I noticed there's also no availability check for the OF graph nodes. That's likely an accidental omission. -- Kind regards, Sakari Ailus
On Thu Sep 4, 2025 at 1:54 PM CEST, Sakari Ailus wrote: > If you're concerned of the use on ACPI platforms, none of the drivers using > the two available variants list any ACPI IDs, signifying they're not used > on ACPI systems -- I don't think they ever have been. Great -- sounds reasonable then.
Hi Danilo, On Thu, Sep 04, 2025 at 06:14:31PM +0200, Danilo Krummrich wrote: > On Thu Sep 4, 2025 at 1:54 PM CEST, Sakari Ailus wrote: > > If you're concerned of the use on ACPI platforms, none of the drivers using > > the two available variants list any ACPI IDs, signifying they're not used > > on ACPI systems -- I don't think they ever have been. > > Great -- sounds reasonable then. I opted to add availability check on the ACPI side as well so that DT and ACPI are aligned in not returning unavailable child devices. -- Regards, Sakari Ailus
On Thu, Sep 04, 2025 at 02:54:40PM +0300, Sakari Ailus wrote: > I noticed there's also no availability check for the OF graph nodes. That's > likely an accidental omission. After doing some further research, this seems to be correct. In OF, the status is defined for device nodes only. A child node could be a device whereas graph endpoints are not device nodes, so the lack of a check there is reasonable. -- Sakari Ailus
Hi Sakari, Le 3 septembre 2025 09 h 18 min 32 s HAE, Sakari Ailus <sakari.ailus@linux.intel.com> a écrit : >Hi Jean-François, > >On Tue, Sep 02, 2025 at 03:04:39PM -0400, Jean-François Lessard wrote: >> Add scoped versions of fwnode child node iterators that automatically >> handle reference counting cleanup using the __free() attribute: >> >> - fwnode_for_each_child_node_scoped() >> - fwnode_for_each_available_child_node_scoped() >> >> These macros follow the same pattern as existing scoped iterators in the >> kernel, ensuring fwnode references are automatically released when the >> iterator variable goes out of scope. This prevents resource leaks and >> eliminates the need for manual cleanup in error paths. >> >> The implementation mirrors the non-scoped variants but uses >> __free(fwnode_handle) for automatic resource management, providing a >> safer and more convenient interface for drivers iterating over firmware >> node children. >> >> Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> >> --- >> >> Notes: >> checkpatch reports false positives that are intentionally ignored: >> MACRO_ARG_REUSE, MACRO_ARG_PRECEDENCE >> This is a standard iterator pattern following kernel conventions. >> >> include/linux/property.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/linux/property.h b/include/linux/property.h >> index 82f0cb3ab..862e20813 100644 >> --- a/include/linux/property.h >> +++ b/include/linux/property.h >> @@ -176,6 +176,16 @@ struct fwnode_handle *fwnode_get_next_available_child_node( >> for (child = fwnode_get_next_available_child_node(fwnode, NULL); child;\ >> child = fwnode_get_next_available_child_node(fwnode, child)) >> >> +#define fwnode_for_each_child_node_scoped(fwnode, child) \ >> + for (struct fwnode_handle *child __free(fwnode_handle) = \ >> + fwnode_get_next_child_node(fwnode, NULL); \ >> + child; child = fwnode_get_next_child_node(fwnode, child)) >> + >> +#define fwnode_for_each_available_child_node_scoped(fwnode, child) \ >> + for (struct fwnode_handle *child __free(fwnode_handle) = \ >> + fwnode_get_next_available_child_node(fwnode, NULL); \ >> + child; child = fwnode_get_next_available_child_node(fwnode, child)) >> + > >Do we really need the available variant? > >Please see ><URL:https://lore.kernel.org/linux-acpi/Zwj12J5bTNUEnxA0@kekkonen.localdomain/>. > >I'll post a patch to remove fwnode_get_next_available_child_node(), too. > Thanks for the link to the discussion. I see you're planning to remove fwnode_get_next_available_child_node() entirely. In that context, adding a scoped version doesn't make sense. For my driver use case, I can handle the status checking manually if the _available_ variant is being deprecated. Should I drop the _available_ variant and submit v5 with only fwnode_for_each_child_node_scoped()? >> struct fwnode_handle *device_get_next_child_node(const struct device *dev, >> struct fwnode_handle *child); >> >
On Tue, Sep 02, 2025 at 03:04:39PM -0400, Jean-François Lessard wrote: > Add scoped versions of fwnode child node iterators that automatically > handle reference counting cleanup using the __free() attribute: > > - fwnode_for_each_child_node_scoped() > - fwnode_for_each_available_child_node_scoped() > > These macros follow the same pattern as existing scoped iterators in the > kernel, ensuring fwnode references are automatically released when the > iterator variable goes out of scope. This prevents resource leaks and > eliminates the need for manual cleanup in error paths. > > The implementation mirrors the non-scoped variants but uses > __free(fwnode_handle) for automatic resource management, providing a > safer and more convenient interface for drivers iterating over firmware > node children. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko
On 9/2/25 9:04 PM, Jean-François Lessard wrote: > Add scoped versions of fwnode child node iterators that automatically > handle reference counting cleanup using the __free() attribute: > > - fwnode_for_each_child_node_scoped() > - fwnode_for_each_available_child_node_scoped() > > These macros follow the same pattern as existing scoped iterators in the > kernel, ensuring fwnode references are automatically released when the > iterator variable goes out of scope. This prevents resource leaks and > eliminates the need for manual cleanup in error paths. > > The implementation mirrors the non-scoped variants but uses > __free(fwnode_handle) for automatic resource management, providing a > safer and more convenient interface for drivers iterating over firmware > node children. > > Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> Acked-by: Danilo Krummrich <dakr@kernel.org>
© 2016 - 2025 Red Hat, Inc.