[PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Andy Shevchenko]

When device_get_child_node_count() got split to the fwnode and device
respective APIs, the fwnode didn't inherit the ability to traverse over
the secondary fwnode. Hence any user, that switches from device to fwnode
API misses this feature. In particular, this was revealed by the commit
1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
that effectively broke the GPIO enumeration on Intel Galileo boards.
Fix this by moving the secondary lookup from device to fwnode API.

Note, in general no device_*() API should go into the depth of the fwnode
implementation.

Fixes: 114dbb4fa7c4 ("drivers property: When no children in primary, try secondary")
Cc: stable@vger.kernel.org
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/base/property.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 6a63860579dd..8d9a34be57fb 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -797,7 +797,18 @@ struct fwnode_handle *
 fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
 			   struct fwnode_handle *child)
 {
-	return fwnode_call_ptr_op(fwnode, get_next_child_node, child);
+	struct fwnode_handle *next;
+
+	if (IS_ERR_OR_NULL(fwnode))
+		return NULL;
+
+	/* Try to find a child in primary fwnode */
+	next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
+	if (next)
+		return next;
+
+	/* When no more children in primary, continue with secondary */
+	return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
 }
 EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
 
@@ -841,19 +852,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
 struct fwnode_handle *device_get_next_child_node(const struct device *dev,
 						 struct fwnode_handle *child)
 {
-	const struct fwnode_handle *fwnode = dev_fwnode(dev);
-	struct fwnode_handle *next;
-
-	if (IS_ERR_OR_NULL(fwnode))
-		return NULL;
-
-	/* Try to find a child in primary fwnode */
-	next = fwnode_get_next_child_node(fwnode, child);
-	if (next)
-		return next;
-
-	/* When no more children in primary, continue with secondary */
-	return fwnode_get_next_child_node(fwnode->secondary, child);
+	return fwnode_get_next_child_node(dev_fwnode(dev), child);
 }
 EXPORT_SYMBOL_GPL(device_get_next_child_node);
 
-- 
2.50.1

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Sakari Ailus]

Hi Andy,

Thanks for the patch.

On Tue, Feb 10, 2026 at 02:58:22PM +0100, Andy Shevchenko wrote:
> When device_get_child_node_count() got split to the fwnode and device
> respective APIs, the fwnode didn't inherit the ability to traverse over
> the secondary fwnode. Hence any user, that switches from device to fwnode
> API misses this feature. In particular, this was revealed by the commit
> 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> that effectively broke the GPIO enumeration on Intel Galileo boards.
> Fix this by moving the secondary lookup from device to fwnode API.
> 
> Note, in general no device_*() API should go into the depth of the fwnode
> implementation.
> 
> Fixes: 114dbb4fa7c4 ("drivers property: When no children in primary, try secondary")
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/base/property.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 6a63860579dd..8d9a34be57fb 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -797,7 +797,18 @@ struct fwnode_handle *
>  fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
>  			   struct fwnode_handle *child)
>  {
> -	return fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> +	struct fwnode_handle *next;
> +
> +	if (IS_ERR_OR_NULL(fwnode))
> +		return NULL;

This test is already being done by fwnode_call_ptr_op() (via
fwnode_has_op()) so I'd omit it here. That would probably be best put in
another patch though. Up to you.

> +
> +	/* Try to find a child in primary fwnode */
> +	next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> +	if (next)
> +		return next;
> +
> +	/* When no more children in primary, continue with secondary */
> +	return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
>  }
>  EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
>  
> @@ -841,19 +852,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
>  struct fwnode_handle *device_get_next_child_node(const struct device *dev,
>  						 struct fwnode_handle *child)
>  {
> -	const struct fwnode_handle *fwnode = dev_fwnode(dev);
> -	struct fwnode_handle *next;
> -
> -	if (IS_ERR_OR_NULL(fwnode))
> -		return NULL;
> -
> -	/* Try to find a child in primary fwnode */
> -	next = fwnode_get_next_child_node(fwnode, child);
> -	if (next)
> -		return next;
> -
> -	/* When no more children in primary, continue with secondary */
> -	return fwnode_get_next_child_node(fwnode->secondary, child);
> +	return fwnode_get_next_child_node(dev_fwnode(dev), child);

As the function becomes trivial, I'd move it to property.h.

>  }
>  EXPORT_SYMBOL_GPL(device_get_next_child_node);
>  

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Andy Shevchenko]

On Wed, Feb 11, 2026 at 12:02:57AM +0200, Sakari Ailus wrote:
> On Tue, Feb 10, 2026 at 02:58:22PM +0100, Andy Shevchenko wrote:
> > When device_get_child_node_count() got split to the fwnode and device
> > respective APIs, the fwnode didn't inherit the ability to traverse over
> > the secondary fwnode. Hence any user, that switches from device to fwnode
> > API misses this feature. In particular, this was revealed by the commit
> > 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> > that effectively broke the GPIO enumeration on Intel Galileo boards.
> > Fix this by moving the secondary lookup from device to fwnode API.
> > 
> > Note, in general no device_*() API should go into the depth of the fwnode
> > implementation.

Thanks for the review, my answers below.

...

> > +	if (IS_ERR_OR_NULL(fwnode))
> > +		return NULL;
> 
> This test is already being done by fwnode_call_ptr_op() (via
> fwnode_has_op()) so I'd omit it here. That would probably be best put in
> another patch though. Up to you.

I would like to keep this as is for the matter of backporting.
With that done, I can clean up further.

...

> As the function becomes trivial, I'd move it to property.h.

Yes, but the same applies to many functions in the property.c. I don't want to
treat this specially:
- exceptionally for this function (what about the rest?)
- for the matters of backporting

...

TL;DR: I would like to move this patch forward as is. After that I will
consider cleaning up as suggested taking into account other places.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Sakari Ailus]

Hi Andy,

On Wed, Feb 11, 2026 at 10:14:59AM +0200, Andy Shevchenko wrote:
> On Wed, Feb 11, 2026 at 12:02:57AM +0200, Sakari Ailus wrote:
> > On Tue, Feb 10, 2026 at 02:58:22PM +0100, Andy Shevchenko wrote:
> > > When device_get_child_node_count() got split to the fwnode and device
> > > respective APIs, the fwnode didn't inherit the ability to traverse over
> > > the secondary fwnode. Hence any user, that switches from device to fwnode
> > > API misses this feature. In particular, this was revealed by the commit
> > > 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> > > that effectively broke the GPIO enumeration on Intel Galileo boards.
> > > Fix this by moving the secondary lookup from device to fwnode API.
> > > 
> > > Note, in general no device_*() API should go into the depth of the fwnode
> > > implementation.
> 
> Thanks for the review, my answers below.
> 
> ...
> 
> > > +	if (IS_ERR_OR_NULL(fwnode))
> > > +		return NULL;
> > 
> > This test is already being done by fwnode_call_ptr_op() (via
> > fwnode_has_op()) so I'd omit it here. That would probably be best put in
> > another patch though. Up to you.
> 
> I would like to keep this as is for the matter of backporting.
> With that done, I can clean up further.
> 
> ...
> 
> > As the function becomes trivial, I'd move it to property.h.
> 
> Yes, but the same applies to many functions in the property.c. I don't want to
> treat this specially:
> - exceptionally for this function (what about the rest?)
> - for the matters of backporting

There are other similar functions in property.h already. Moving the other
trivial ones there, too, wouldn't hurt.

> 
> ...
> 
> TL;DR: I would like to move this patch forward as is. After that I will
> consider cleaning up as suggested taking into account other places.

Sounds good to me.

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Andy Shevchenko]

On Wed, Feb 11, 2026 at 10:27:28AM +0200, Sakari Ailus wrote:
> On Wed, Feb 11, 2026 at 10:14:59AM +0200, Andy Shevchenko wrote:
> > On Wed, Feb 11, 2026 at 12:02:57AM +0200, Sakari Ailus wrote:
> > > On Tue, Feb 10, 2026 at 02:58:22PM +0100, Andy Shevchenko wrote:

...

> > > As the function becomes trivial, I'd move it to property.h.
> > 
> > Yes, but the same applies to many functions in the property.c. I don't want to
> > treat this specially:
> > - exceptionally for this function (what about the rest?)
> > - for the matters of backporting
> 
> There are other similar functions in property.h already. Moving the other
> trivial ones there, too, wouldn't hurt.

There is actually a potential issue that I would like to avoid. Id est
the device.h is a mess and first of all I want to split it to a few other
headers (one of which for the fwnode stuff), and only after that reshuffle
this, because blindly moving everything to the header is not a good
strategy in long term.

...

> > TL;DR: I would like to move this patch forward as is. After that I will
> > consider cleaning up as suggested taking into account other places.
> 
> Sounds good to me.
> 
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Rafael J. Wysocki]

Cc: driver-core

On Tue, Feb 10, 2026 at 2:58 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> When device_get_child_node_count() got split to the fwnode and device
> respective APIs, the fwnode didn't inherit the ability to traverse over
> the secondary fwnode. Hence any user, that switches from device to fwnode
> API misses this feature. In particular, this was revealed by the commit
> 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> that effectively broke the GPIO enumeration on Intel Galileo boards.
> Fix this by moving the secondary lookup from device to fwnode API.
>
> Note, in general no device_*() API should go into the depth of the fwnode
> implementation.
>
> Fixes: 114dbb4fa7c4 ("drivers property: When no children in primary, try secondary")
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
>  drivers/base/property.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 6a63860579dd..8d9a34be57fb 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -797,7 +797,18 @@ struct fwnode_handle *
>  fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
>                            struct fwnode_handle *child)
>  {
> -       return fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> +       struct fwnode_handle *next;
> +
> +       if (IS_ERR_OR_NULL(fwnode))
> +               return NULL;
> +
> +       /* Try to find a child in primary fwnode */
> +       next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> +       if (next)
> +               return next;
> +
> +       /* When no more children in primary, continue with secondary */
> +       return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
>  }
>  EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
>
> @@ -841,19 +852,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_next_available_child_node);
>  struct fwnode_handle *device_get_next_child_node(const struct device *dev,
>                                                  struct fwnode_handle *child)
>  {
> -       const struct fwnode_handle *fwnode = dev_fwnode(dev);
> -       struct fwnode_handle *next;
> -
> -       if (IS_ERR_OR_NULL(fwnode))
> -               return NULL;
> -
> -       /* Try to find a child in primary fwnode */
> -       next = fwnode_get_next_child_node(fwnode, child);
> -       if (next)
> -               return next;
> -
> -       /* When no more children in primary, continue with secondary */
> -       return fwnode_get_next_child_node(fwnode->secondary, child);
> +       return fwnode_get_next_child_node(dev_fwnode(dev), child);
>  }
>  EXPORT_SYMBOL_GPL(device_get_next_child_node);
>
> --
> 2.50.1
>

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Danilo Krummrich]

On Tue Feb 10, 2026 at 2:58 PM CET, Andy Shevchenko wrote:
> When device_get_child_node_count() got split to the fwnode and device
> respective APIs, the fwnode didn't inherit the ability to traverse over
> the secondary fwnode. Hence any user, that switches from device to fwnode
> API misses this feature. In particular, this was revealed by the commit
> 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> that effectively broke the GPIO enumeration on Intel Galileo boards.
> Fix this by moving the secondary lookup from device to fwnode API.
>
> Note, in general no device_*() API should go into the depth of the fwnode
> implementation.
>
> Fixes: 114dbb4fa7c4 ("drivers property: When no children in primary, try secondary")
> Cc: stable@vger.kernel.org
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to driver-core-linus, thanks!

Re: [PATCH v1 1/1] device property: Allow secondary lookup in fwnode_get_next_child_node()

[Peter Shier]

On Tue Feb 10, 2026 at 2:58 PM CET, Andy Shevchenko wrote:
> When device_get_child_node_count() got split to the fwnode and device
> respective APIs, the fwnode didn't inherit the ability to traverse over
> the secondary fwnode. Hence any user, that switches from device to fwnode
> API misses this feature. In particular, this was revealed by the commit
> 1490cbb9dbfd ("device property: Split fwnode_get_child_node_count()")
> that effectively broke the GPIO enumeration on Intel Galileo boards.
> Fix this by moving the secondary lookup from device to fwnode API.

I am not familiar with this code at all but from a sashiko AI review I
found the following comments.
Does this need to be addressed?

> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 837d77e3af2b..4217d00c76fd 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -759,7 +759,18 @@
>  fwnode_get_next_child_node(const struct fwnode_handle *fwnode,
>     struct fwnode_handle *child)
>  {
> - return fwnode_call_ptr_op(fwnode, get_next_child_node, child);
> + struct fwnode_handle *next;
> +
> + if (IS_ERR_OR_NULL(fwnode))
> + return NULL;
> +
> + /* Try to find a child in primary fwnode */
> + next = fwnode_call_ptr_op(fwnode, get_next_child_node, child);

If the primary fwnode is a Devicetree node (OF) and it returns NULL because
`child` was its last child, `of_fwnode_get_next_child_node()` will have
already called `of_node_put()` on `child`. Will passing this already-freed
`child` pointer to the secondary fwnode below cause a double-put when
`software_node_get_next_child()` also calls `fwnode_handle_put(child)`?

If `child` is a node from the secondary fwnode, passing it to the primary
OF fwnode's `get_next_child_node` operation will result in `to_of_node(child)`
evaluating to NULL. This causes `of_get_next_available_child()` to treat it
as the start of the iteration and return the first primary child. Does this
create an infinite loop where the iteration loops back to the primary children
after reaching a secondary child?

Furthermore, when the primary OF fwnode's `get_next_child_node` treats the
secondary `child` as NULL, it does not drop the reference to `child`. Does
this leak the secondary fwnode child references during the infinite loop?

> + if (next)
> + return next;
> +
> + /* When no more children in primary, continue with secondary */
> + return fwnode_call_ptr_op(fwnode->secondary, get_next_child_node, child);
>  }