[PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Jean-François Lessard]

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

[PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Jean-François Lessard]

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

[PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped()

[Jean-François Lessard]

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", &reg);
 			if (reg & I2C_OWN_SLAVE_ADDRESS)
 				return true;
-- 
2.43.0

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

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>

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Andy Shevchenko]

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

Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Andy Shevchenko]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Sakari Ailus]

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

Re: [PATCH v4 2/2] i2c: core: Use fwnode_for_each_child_node_scoped()

[Sakari Ailus]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Jean-François Lessard]

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);
>>  
>

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

(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?

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Sakari Ailus]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

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?

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Sakari Ailus]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

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?

Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Wolfram Sang]

[-- 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 --]

Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Andy Shevchenko]

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

Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

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.

Re: [PATCH v4 0/2] device property: Add scoped fwnode child node iterators

[Sakari Ailus]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Sakari Ailus]

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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[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

Re: [PATCH v4 1/2] device property: Add scoped fwnode child node iterators

[Danilo Krummrich]

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.