[PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[Hans de Goede]

When a software-node gets added to a device which already has another
fwnode as primary node it will become the secondary fwnode for that
device.

Currently if a software-node with GPIO properties ends up as the secondary
fwnode then gpiod_find_by_fwnode() will fail to find the GPIOs.

Add a new gpiod_fwnode_lookup() helper which falls back to calling
gpiod_find_by_fwnode() with the secondary fwnode if the GPIO was not
found in the primary fwnode.

Fixes: e7f9ff5dc90c ("gpiolib: add support for software nodes")
Cc: stable@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Hans de Goede <hansg@kernel.org>
---
Changes in v2:
- Add a new gpiod_fwnode_lookup() helper instead of putting the secondary
  fwnode check inside gpiod_find_by_fwnode()
---
 drivers/gpio/gpiolib.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0d2b470a252e..74d54513730a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4604,6 +4604,23 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
 	return desc;
 }
 
+static struct gpio_desc *gpiod_fwnode_lookup(struct fwnode_handle *fwnode,
+					     struct device *consumer,
+					     const char *con_id,
+					     unsigned int idx,
+					     enum gpiod_flags *flags,
+					     unsigned long *lookupflags)
+{
+	struct gpio_desc *desc;
+
+	desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
+	if (gpiod_not_found(desc) && !IS_ERR_OR_NULL(fwnode))
+		desc = gpiod_find_by_fwnode(fwnode->secondary, consumer, con_id,
+					    idx, flags, lookupflags);
+
+	return desc;
+}
+
 struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 					 struct fwnode_handle *fwnode,
 					 const char *con_id,
@@ -4622,8 +4639,8 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 	int ret = 0;
 
 	scoped_guard(srcu, &gpio_devices_srcu) {
-		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
-					    &flags, &lookupflags);
+		desc = gpiod_fwnode_lookup(fwnode, consumer, con_id, idx,
+					   &flags, &lookupflags);
 		if (gpiod_not_found(desc) && platform_lookup_allowed) {
 			/*
 			 * Either we are not using DT or ACPI, or their lookup
-- 
2.51.0

Re: [PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[Dmitry Torokhov]

On Sat, Sep 20, 2025 at 10:09:55PM +0200, Hans de Goede wrote:
> When a software-node gets added to a device which already has another
> fwnode as primary node it will become the secondary fwnode for that
> device.
> 
> Currently if a software-node with GPIO properties ends up as the secondary
> fwnode then gpiod_find_by_fwnode() will fail to find the GPIOs.
> 
> Add a new gpiod_fwnode_lookup() helper which falls back to calling
> gpiod_find_by_fwnode() with the secondary fwnode if the GPIO was not
> found in the primary fwnode.
> 
> Fixes: e7f9ff5dc90c ("gpiolib: add support for software nodes")
> Cc: stable@vger.kernel.org
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Hans de Goede <hansg@kernel.org>

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> Changes in v2:
> - Add a new gpiod_fwnode_lookup() helper instead of putting the secondary
>   fwnode check inside gpiod_find_by_fwnode()
> ---
>  drivers/gpio/gpiolib.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 0d2b470a252e..74d54513730a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4604,6 +4604,23 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
>  	return desc;
>  }
>  
> +static struct gpio_desc *gpiod_fwnode_lookup(struct fwnode_handle *fwnode,
> +					     struct device *consumer,
> +					     const char *con_id,
> +					     unsigned int idx,
> +					     enum gpiod_flags *flags,
> +					     unsigned long *lookupflags)
> +{
> +	struct gpio_desc *desc;
> +
> +	desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
> +	if (gpiod_not_found(desc) && !IS_ERR_OR_NULL(fwnode))
> +		desc = gpiod_find_by_fwnode(fwnode->secondary, consumer, con_id,
> +					    idx, flags, lookupflags);
> +
> +	return desc;

Bikeshedding for later. Maybe do it like this in case we can have more
than 2 nodes at some point?

        do {
		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
		if (!gpiod_not_found(desc))
			return desc;

		fwnode = fwnode->secondary;
	} while (!IS_ERR_OR_NULL(fwnode));

	return ERR_PTR(-ENOENT);

> +}
> +
>  struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>  					 struct fwnode_handle *fwnode,
>  					 const char *con_id,
> @@ -4622,8 +4639,8 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>  	int ret = 0;
>  
>  	scoped_guard(srcu, &gpio_devices_srcu) {
> -		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
> -					    &flags, &lookupflags);
> +		desc = gpiod_fwnode_lookup(fwnode, consumer, con_id, idx,
> +					   &flags, &lookupflags);
>  		if (gpiod_not_found(desc) && platform_lookup_allowed) {
>  			/*
>  			 * Either we are not using DT or ACPI, or their lookup

Thanks.

-- 
Dmitry

Re: [PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[Hans de Goede]

On 20-Sep-25 11:27 PM, Dmitry Torokhov wrote:
> On Sat, Sep 20, 2025 at 10:09:55PM +0200, Hans de Goede wrote:
>> When a software-node gets added to a device which already has another
>> fwnode as primary node it will become the secondary fwnode for that
>> device.
>>
>> Currently if a software-node with GPIO properties ends up as the secondary
>> fwnode then gpiod_find_by_fwnode() will fail to find the GPIOs.
>>
>> Add a new gpiod_fwnode_lookup() helper which falls back to calling
>> gpiod_find_by_fwnode() with the secondary fwnode if the GPIO was not
>> found in the primary fwnode.
>>
>> Fixes: e7f9ff5dc90c ("gpiolib: add support for software nodes")
>> Cc: stable@vger.kernel.org
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Signed-off-by: Hans de Goede <hansg@kernel.org>
> 
> Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
>> ---
>> Changes in v2:
>> - Add a new gpiod_fwnode_lookup() helper instead of putting the secondary
>>   fwnode check inside gpiod_find_by_fwnode()
>> ---
>>  drivers/gpio/gpiolib.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 0d2b470a252e..74d54513730a 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -4604,6 +4604,23 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
>>  	return desc;
>>  }
>>  
>> +static struct gpio_desc *gpiod_fwnode_lookup(struct fwnode_handle *fwnode,
>> +					     struct device *consumer,
>> +					     const char *con_id,
>> +					     unsigned int idx,
>> +					     enum gpiod_flags *flags,
>> +					     unsigned long *lookupflags)
>> +{
>> +	struct gpio_desc *desc;
>> +
>> +	desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
>> +	if (gpiod_not_found(desc) && !IS_ERR_OR_NULL(fwnode))
>> +		desc = gpiod_find_by_fwnode(fwnode->secondary, consumer, con_id,
>> +					    idx, flags, lookupflags);
>> +
>> +	return desc;
> 
> Bikeshedding for later. Maybe do it like this in case we can have more
> than 2 nodes at some point?
> 
>         do {
> 		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
> 		if (!gpiod_not_found(desc))
> 			return desc;
> 
> 		fwnode = fwnode->secondary;
> 	} while (!IS_ERR_OR_NULL(fwnode));
> 
> 	return ERR_PTR(-ENOENT);

At a minimum this would need to a regular while () {} loop then,
the initial fwnode may also be NULL and we don't want to deref that.

Andy did mention turning the fwnode-s into a regular linked-list
in the future, but I think that would be using <linux/list.h> then,
replacing the secondary pointer with a list head ?

Regards,

Hans






> 
>> +}
>> +
>>  struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>>  					 struct fwnode_handle *fwnode,
>>  					 const char *con_id,
>> @@ -4622,8 +4639,8 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>>  	int ret = 0;
>>  
>>  	scoped_guard(srcu, &gpio_devices_srcu) {
>> -		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
>> -					    &flags, &lookupflags);
>> +		desc = gpiod_fwnode_lookup(fwnode, consumer, con_id, idx,
>> +					   &flags, &lookupflags);
>>  		if (gpiod_not_found(desc) && platform_lookup_allowed) {
>>  			/*
>>  			 * Either we are not using DT or ACPI, or their lookup
> 
> Thanks.
> 

Re: [PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[Andy Shevchenko]

On Sun, Sep 21, 2025 at 4:00 PM Hans de Goede <hansg@kernel.org> wrote:
> On 20-Sep-25 11:27 PM, Dmitry Torokhov wrote:
> > On Sat, Sep 20, 2025 at 10:09:55PM +0200, Hans de Goede wrote:

...

> > Bikeshedding for later. Maybe do it like this in case we can have more
> > than 2 nodes at some point?
> >
> >         do {
> >               desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx, flags, lookupflags);
> >               if (!gpiod_not_found(desc))
> >                       return desc;
> >
> >               fwnode = fwnode->secondary;
> >       } while (!IS_ERR_OR_NULL(fwnode));
> >
> >       return ERR_PTR(-ENOENT);
>
> At a minimum this would need to a regular while () {} loop then,
> the initial fwnode may also be NULL and we don't want to deref that.
>
> Andy did mention turning the fwnode-s into a regular linked-list
> in the future, but I think that would be using <linux/list.h> then,
> replacing the secondary pointer with a list head ?

Dropping secondary in the struct fwnode_handle and use struct
list_head somewhere else, e.g., struct device.

> >> +}


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] gpiolib: Extend software-node support to support secondary software-nodes

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Sat, 20 Sep 2025 22:09:55 +0200, Hans de Goede wrote:
> When a software-node gets added to a device which already has another
> fwnode as primary node it will become the secondary fwnode for that
> device.
> 
> Currently if a software-node with GPIO properties ends up as the secondary
> fwnode then gpiod_find_by_fwnode() will fail to find the GPIOs.
> 
> [...]

Applied, thanks!

[1/1] gpiolib: Extend software-node support to support secondary software-nodes
      https://git.kernel.org/brgl/linux/c/c6ccc4dde17676dfe617b9a37bd9ba19a8fc87ee

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>