* [PATCH v4 0/2] device property: Add scoped fwnode child node iterators @ 2025-09-02 19:04 Jean-François Lessard 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard ` (2 more replies) 0 siblings, 3 replies; 21+ messages in thread From: Jean-François Lessard @ 2025-09-02 19:04 UTC (permalink / raw) To: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich Cc: linux-i2c, linux-kernel, linux-acpi This series adds scoped versions of fwnode iterator macros and converts existing manual implementation to use them. The first patch adds the infrastructure macros following existing patterns for scoped iterators in the kernel. The second patch demonstrates fwnode_for_each_child_node_scoped() usage by converting existing manual __free() usage in i2c-core-slave.c. This series introduces infrastructure for the TM16XX driver series, being the first user of fwnode_for_each_available_child_node_scoped(). See the related patch series: auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver Changes in v4: - drop the fwnode_for_each_named_child_node_scoped() variant (no user) - add Reviewed-by tag to commit message of patch 2/2 Changes in v3: - Split into separate patches as requested - Infrastructure addition in patch 1/2 - Usage example in patch 2/2 Changes in v2: - replace manual __free(fwnode_handle) of i2c-core-slave.c with fwnode_for_each_child_node_scoped Jean-François Lessard (2): device property: Add scoped fwnode child node iterators i2c: core: Use fwnode_for_each_child_node_scoped() drivers/i2c/i2c-core-slave.c | 3 +-- include/linux/property.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) -- 2.43.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-02 19:04 [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Jean-François Lessard @ 2025-09-02 19:04 ` Jean-François Lessard 2025-09-02 19:11 ` Danilo Krummrich ` (2 more replies) 2025-09-02 19:04 ` [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() Jean-François Lessard 2025-09-03 10:30 ` [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Andy Shevchenko 2 siblings, 3 replies; 21+ messages in thread From: Jean-François Lessard @ 2025-09-02 19:04 UTC (permalink / raw) To: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich Cc: linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard @ 2025-09-02 19:11 ` Danilo Krummrich 2025-09-03 10:29 ` Andy Shevchenko 2025-09-03 13:18 ` Sakari Ailus 2 siblings, 0 replies; 21+ messages in thread From: Danilo Krummrich @ 2025-09-02 19:11 UTC (permalink / raw) To: Jean-François Lessard Cc: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, linux-i2c, linux-kernel, linux-acpi 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> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard 2025-09-02 19:11 ` Danilo Krummrich @ 2025-09-03 10:29 ` Andy Shevchenko 2025-09-03 13:18 ` Sakari Ailus 2 siblings, 0 replies; 21+ messages in thread From: Andy Shevchenko @ 2025-09-03 10:29 UTC (permalink / raw) To: Jean-François Lessard Cc: Wolfram Sang, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard 2025-09-02 19:11 ` Danilo Krummrich 2025-09-03 10:29 ` Andy Shevchenko @ 2025-09-03 13:18 ` Sakari Ailus 2025-09-03 16:43 ` Jean-François Lessard 2025-09-03 17:22 ` Danilo Krummrich 2 siblings, 2 replies; 21+ messages in thread From: Sakari Ailus @ 2025-09-03 13:18 UTC (permalink / raw) To: Jean-François Lessard Cc: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-03 13:18 ` Sakari Ailus @ 2025-09-03 16:43 ` Jean-François Lessard 2025-09-03 17:22 ` Danilo Krummrich 1 sibling, 0 replies; 21+ messages in thread From: Jean-François Lessard @ 2025-09-03 16:43 UTC (permalink / raw) To: Sakari Ailus Cc: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi 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); >> > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-03 13:18 ` Sakari Ailus 2025-09-03 16:43 ` Jean-François Lessard @ 2025-09-03 17:22 ` Danilo Krummrich 2025-09-04 5:56 ` Sakari Ailus 1 sibling, 1 reply; 21+ messages in thread From: Danilo Krummrich @ 2025-09-03 17:22 UTC (permalink / raw) To: Sakari Ailus Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi (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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-03 17:22 ` Danilo Krummrich @ 2025-09-04 5:56 ` Sakari Ailus 2025-09-04 7:03 ` Danilo Krummrich 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2025-09-04 5:56 UTC (permalink / raw) To: Danilo Krummrich Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 5:56 ` Sakari Ailus @ 2025-09-04 7:03 ` Danilo Krummrich 2025-09-04 7:43 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Danilo Krummrich @ 2025-09-04 7:03 UTC (permalink / raw) To: Sakari Ailus Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 7:03 ` Danilo Krummrich @ 2025-09-04 7:43 ` Sakari Ailus 2025-09-04 8:51 ` Danilo Krummrich 0 siblings, 1 reply; 21+ messages in thread From: Sakari Ailus @ 2025-09-04 7:43 UTC (permalink / raw) To: Danilo Krummrich Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 7:43 ` Sakari Ailus @ 2025-09-04 8:51 ` Danilo Krummrich 2025-09-04 11:54 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Danilo Krummrich @ 2025-09-04 8:51 UTC (permalink / raw) To: Sakari Ailus Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 8:51 ` Danilo Krummrich @ 2025-09-04 11:54 ` Sakari Ailus 2025-09-04 12:39 ` Sakari Ailus 2025-09-04 16:14 ` Danilo Krummrich 0 siblings, 2 replies; 21+ messages in thread From: Sakari Ailus @ 2025-09-04 11:54 UTC (permalink / raw) To: Danilo Krummrich Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 11:54 ` Sakari Ailus @ 2025-09-04 12:39 ` Sakari Ailus 2025-09-04 16:14 ` Danilo Krummrich 1 sibling, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2025-09-04 12:39 UTC (permalink / raw) To: Danilo Krummrich Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators 2025-09-04 11:54 ` Sakari Ailus 2025-09-04 12:39 ` Sakari Ailus @ 2025-09-04 16:14 ` Danilo Krummrich 1 sibling, 0 replies; 21+ messages in thread From: Danilo Krummrich @ 2025-09-04 16:14 UTC (permalink / raw) To: Sakari Ailus Cc: Jean-François Lessard, Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Javier Carrasco, linux-i2c, linux-kernel, linux-acpi 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() 2025-09-02 19:04 [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Jean-François Lessard 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard @ 2025-09-02 19:04 ` Jean-François Lessard 2025-09-03 13:19 ` Sakari Ailus 2025-09-03 10:30 ` [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Andy Shevchenko 2 siblings, 1 reply; 21+ messages in thread From: Jean-François Lessard @ 2025-09-02 19:04 UTC (permalink / raw) To: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich Cc: linux-i2c, linux-kernel, linux-acpi Replace the manual __free(fwnode_handle) iterator declaration with the new scoped iterator macro for cleaner, less error-prone code. This eliminates the need for explicit iterator variable declaration with the cleanup attribute, making the code more consistent with other scoped iterator usage patterns in the kernel. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> --- drivers/i2c/i2c-core-slave.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/i2c/i2c-core-slave.c b/drivers/i2c/i2c-core-slave.c index 7ee6b992b..02ca55c22 100644 --- a/drivers/i2c/i2c-core-slave.c +++ b/drivers/i2c/i2c-core-slave.c @@ -112,10 +112,9 @@ bool i2c_detect_slave_mode(struct device *dev) struct fwnode_handle *fwnode = dev_fwnode(dev); if (is_of_node(fwnode)) { - struct fwnode_handle *child __free(fwnode_handle) = NULL; u32 reg; - fwnode_for_each_child_node(fwnode, child) { + fwnode_for_each_child_node_scoped(fwnode, child) { fwnode_property_read_u32(child, "reg", ®); if (reg & I2C_OWN_SLAVE_ADDRESS) return true; -- 2.43.0 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() 2025-09-02 19:04 ` [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() Jean-François Lessard @ 2025-09-03 13:19 ` Sakari Ailus 0 siblings, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2025-09-03 13:19 UTC (permalink / raw) To: Jean-François Lessard Cc: Wolfram Sang, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi On Tue, Sep 02, 2025 at 03:04:40PM -0400, Jean-François Lessard wrote: > Replace the manual __free(fwnode_handle) iterator declaration with the > new scoped iterator macro for cleaner, less error-prone code. > > This eliminates the need for explicit iterator variable declaration with > the cleanup attribute, making the code more consistent with other scoped > iterator usage patterns in the kernel. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> -- Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators 2025-09-02 19:04 [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Jean-François Lessard 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard 2025-09-02 19:04 ` [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() Jean-François Lessard @ 2025-09-03 10:30 ` Andy Shevchenko 2025-09-04 9:49 ` Wolfram Sang 2 siblings, 1 reply; 21+ messages in thread From: Andy Shevchenko @ 2025-09-03 10:30 UTC (permalink / raw) To: Jean-François Lessard Cc: Wolfram Sang, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi On Tue, Sep 02, 2025 at 03:04:38PM -0400, Jean-François Lessard wrote: > This series adds scoped versions of fwnode iterator macros and converts > existing manual implementation to use them. > > The first patch adds the infrastructure macros following existing patterns > for scoped iterators in the kernel. The second patch demonstrates > fwnode_for_each_child_node_scoped() usage by converting existing manual > __free() usage in i2c-core-slave.c. > > This series introduces infrastructure for the TM16XX driver series, > being the first user of fwnode_for_each_available_child_node_scoped(). > See the related patch series: > auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver It might be good to have an immutable branch for me from i2c core. Wolfram, can you provide a such if no objections? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators 2025-09-03 10:30 ` [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Andy Shevchenko @ 2025-09-04 9:49 ` Wolfram Sang 2025-09-04 9:59 ` Andy Shevchenko 0 siblings, 1 reply; 21+ messages in thread From: Wolfram Sang @ 2025-09-04 9:49 UTC (permalink / raw) To: Andy Shevchenko Cc: Jean-François Lessard, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi [-- Attachment #1: Type: text/plain, Size: 230 bytes --] > It might be good to have an immutable branch for me from i2c core. > Wolfram, can you provide a such if no objections? Sure thing, I can do that. But there is still discussion on patch 1, so I will wait for an outcome there. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators 2025-09-04 9:49 ` Wolfram Sang @ 2025-09-04 9:59 ` Andy Shevchenko 2025-09-04 10:13 ` Danilo Krummrich 0 siblings, 1 reply; 21+ messages in thread From: Andy Shevchenko @ 2025-09-04 9:59 UTC (permalink / raw) To: Wolfram Sang Cc: Jean-François Lessard, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, linux-i2c, linux-kernel, linux-acpi On Thu, Sep 04, 2025 at 11:49:25AM +0200, Wolfram Sang wrote: > > > It might be good to have an immutable branch for me from i2c core. > > Wolfram, can you provide a such if no objections? > > Sure thing, I can do that. But there is still discussion on patch 1, so > I will wait for an outcome there. But it seems that the discussion can be implemented in a followup? I think we are not in hurry anyway, so let see if it settles down soon. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators 2025-09-04 9:59 ` Andy Shevchenko @ 2025-09-04 10:13 ` Danilo Krummrich 2025-09-04 10:38 ` Sakari Ailus 0 siblings, 1 reply; 21+ messages in thread From: Danilo Krummrich @ 2025-09-04 10:13 UTC (permalink / raw) To: Andy Shevchenko Cc: Wolfram Sang, Jean-François Lessard, Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki, linux-i2c, linux-kernel, linux-acpi On Thu Sep 4, 2025 at 11:59 AM CEST, Andy Shevchenko wrote: > On Thu, Sep 04, 2025 at 11:49:25AM +0200, Wolfram Sang wrote: >> >> > It might be good to have an immutable branch for me from i2c core. >> > Wolfram, can you provide a such if no objections? >> >> Sure thing, I can do that. But there is still discussion on patch 1, so >> I will wait for an outcome there. > > But it seems that the discussion can be implemented in a followup? If Sakari attempts the rework, and we can prove this doesn't regress existing users, removing fwnode_for_each_available_child_node_scoped() in the context of the rework again should be trivial. Given that, I don't see a reason to stall people working with the existing semantics of the API in the meantime. So, AFAIC, my ACK is still valid. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators 2025-09-04 10:13 ` Danilo Krummrich @ 2025-09-04 10:38 ` Sakari Ailus 0 siblings, 0 replies; 21+ messages in thread From: Sakari Ailus @ 2025-09-04 10:38 UTC (permalink / raw) To: Danilo Krummrich Cc: Andy Shevchenko, Wolfram Sang, Jean-François Lessard, Daniel Scally, Heikki Krogerus, Greg Kroah-Hartman, Rafael J. Wysocki, linux-i2c, linux-kernel, linux-acpi Hi Danilo, others, On Thu, Sep 04, 2025 at 12:13:53PM +0200, Danilo Krummrich wrote: > On Thu Sep 4, 2025 at 11:59 AM CEST, Andy Shevchenko wrote: > > On Thu, Sep 04, 2025 at 11:49:25AM +0200, Wolfram Sang wrote: > >> > >> > It might be good to have an immutable branch for me from i2c core. > >> > Wolfram, can you provide a such if no objections? > >> > >> Sure thing, I can do that. But there is still discussion on patch 1, so > >> I will wait for an outcome there. > > > > But it seems that the discussion can be implemented in a followup? > > If Sakari attempts the rework, and we can prove this doesn't regress existing > users, removing fwnode_for_each_available_child_node_scoped() in the context > of the rework again should be trivial. It would perhaps be trivial but in this case I really wouldn't add it in the first place: it's unused. Either way, feel free to add: Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Given that, I don't see a reason to stall people working with the existing > semantics of the API in the meantime. -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-09-04 16:14 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-02 19:04 [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Jean-François Lessard 2025-09-02 19:04 ` [PATCH v4 1/2] " Jean-François Lessard 2025-09-02 19:11 ` Danilo Krummrich 2025-09-03 10:29 ` Andy Shevchenko 2025-09-03 13:18 ` Sakari Ailus 2025-09-03 16:43 ` Jean-François Lessard 2025-09-03 17:22 ` Danilo Krummrich 2025-09-04 5:56 ` Sakari Ailus 2025-09-04 7:03 ` Danilo Krummrich 2025-09-04 7:43 ` Sakari Ailus 2025-09-04 8:51 ` Danilo Krummrich 2025-09-04 11:54 ` Sakari Ailus 2025-09-04 12:39 ` Sakari Ailus 2025-09-04 16:14 ` Danilo Krummrich 2025-09-02 19:04 ` [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped() Jean-François Lessard 2025-09-03 13:19 ` Sakari Ailus 2025-09-03 10:30 ` [PATCH v4 0/2] device property: Add scoped fwnode child node iterators Andy Shevchenko 2025-09-04 9:49 ` Wolfram Sang 2025-09-04 9:59 ` Andy Shevchenko 2025-09-04 10:13 ` Danilo Krummrich 2025-09-04 10:38 ` Sakari Ailus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).