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

[PATCH] 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 check to gpiod_find_by_fwnode() to try a software-node lookup on
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>
---
I found this issue while testing "platform/x86: x86-android-tablets:
convert wm1502 devices to GPIO references":
https://lore.kernel.org/platform-driver-x86/20250810-x86-andoroid-tablet-v2-7-9c7a1b3c32b2@gmail.com/
which adds a software node with GPIO lookup info the a spi-10WM5102:00
device which has an ACPI fwnode as primary fwnode.
---
 drivers/gpio/gpiolib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0d2b470a252e..b619fea498c8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4601,6 +4601,12 @@ static struct gpio_desc *gpiod_find_by_fwnode(struct fwnode_handle *fwnode,
 		desc = swnode_find_gpio(fwnode, con_id, idx, lookupflags);
 	}
 
+	if (desc == ERR_PTR(-ENOENT) && fwnode && is_software_node(fwnode->secondary)) {
+		dev_dbg(consumer, "using secondary-swnode '%pfw' for '%s' GPIO lookup\n",
+			fwnode->secondary, name);
+		desc = swnode_find_gpio(fwnode->secondary, con_id, idx, lookupflags);
+	}
+
 	return desc;
 }
 
-- 
2.51.0

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

[Andy Shevchenko]

On Sat, Sep 13, 2025 at 9:43 PM Hans de Goede <hansg@kernel.org> 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 check to gpiod_find_by_fwnode() to try a software-node lookup on
> the secondary fwnode if the GPIO was not found in the primary fwnode.

> ---
> I found this issue while testing "platform/x86: x86-android-tablets:
> convert wm1502 devices to GPIO references":
> https://lore.kernel.org/platform-driver-x86/20250810-x86-andoroid-tablet-v2-7-9c7a1b3c32b2@gmail.com/
> which adds a software node with GPIO lookup info the a spi-10WM5102:00
> device which has an ACPI fwnode as primary fwnode.

While this is a quick fix, the long term one should be in a full
redesigning of the fwnode concept in the kernel. The limitation of the
linked list to two sooner or later strikes us. Besides that, the list
of fwnodes conceptually is not property of the fwnode itself. The
struct device may have struct list_head swnodes; besides possible
other users. In particular this also will allow to have OF and ACPI
nodes along with swnodes. You can say "are you crazy?", but look at
the DSA development and other interesting PCI devices that are
basically computers-as-a-card. The floating patch series is to enable
OF enumeration for the devices on that type of cards even on ACPI
based platforms. Which make above mentioned use-case not so
theoretical.


-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Sun, Sep 14, 2025 at 4:34 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Sep 13, 2025 at 9:43 PM Hans de Goede <hansg@kernel.org> wrote:

...

> the long term one should be in a full
> redesigning of the fwnode concept in the kernel. The limitation of the
> linked list to two sooner or later strikes us. Besides that, the list
> of fwnodes conceptually is not property of the fwnode itself. The
> struct device may have struct list_head swnodes; besides possible
> other users. In particular this also will allow to have OF and ACPI
> nodes along with swnodes. You can say "are you crazy?", but look at
> the DSA development and other interesting PCI devices that are
> basically computers-as-a-card. The floating patch series is to enable
> OF enumeration for the devices on that type of cards even on ACPI
> based platforms. Which make above mentioned use-case not so
> theoretical.

FWIW, that's why I for more than a year require people to use
dev_fwnode() and dev_of_node() (and similar) APIs instead of
dereferencing them directly in order to make less churn in case of the
above ever happens.


-- 
With Best Regards,
Andy Shevchenko

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

[Dmitry Torokhov]

Hi Hans,

On Sat, Sep 13, 2025 at 08:43:09PM +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 check to gpiod_find_by_fwnode() to try a software-node lookup on
> the secondary fwnode if the GPIO was not found in the primary fwnode.

Thanks for catching this. I think it would be better if we added
handling of the secondary node to gpiod_find_and_request(). This way the
fallback will work for all kind of combinations, even if secondary node
happens to be an OF or ACPI one.

Thanks.

-- 
Dmitry

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

[Hans de Goede]

Hi,

On 15-Sep-25 3:21 AM, Dmitry Torokhov wrote:
> Hi Hans,
> 
> On Sat, Sep 13, 2025 at 08:43:09PM +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 check to gpiod_find_by_fwnode() to try a software-node lookup on
>> the secondary fwnode if the GPIO was not found in the primary fwnode.
> 
> Thanks for catching this. I think it would be better if we added
> handling of the secondary node to gpiod_find_and_request(). This way the
> fallback will work for all kind of combinations, even if secondary node
> happens to be an OF or ACPI one.

IOW something like this:

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b619fea498c8..1a3b5ca00554 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4630,6 +4630,13 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 	scoped_guard(srcu, &gpio_devices_srcu) {
 		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
 					    &flags, &lookupflags);
+
+		if (gpiod_not_found(desc) && fwnode) {
+			dev_dbg(consumer, "trying secondary fwnode for GPIO lookup\n");
+			desc = gpiod_find_by_fwnode(fwnode->secondary, 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

That should work too, but if there is an OF or ACPI node it should always
be the primary one. So my original patch id fine as is.

Either way works for me. If you prefer the above approach instead of my
original patch let me know and I'll give it a test-run and then post a v2.

Regards,

Hans

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

[Andy Shevchenko]

On Mon, Sep 15, 2025 at 8:49 PM Hans de Goede <hansg@kernel.org> wrote:
> On 15-Sep-25 3:21 AM, Dmitry Torokhov wrote:
> > On Sat, Sep 13, 2025 at 08:43:09PM +0200, Hans de Goede wrote:

...

> > Thanks for catching this. I think it would be better if we added
> > handling of the secondary node to gpiod_find_and_request(). This way the
> > fallback will work for all kind of combinations, even if secondary node
> > happens to be an OF or ACPI one.
>
> IOW something like this:

> That should work too, but if there is an OF or ACPI node it should always
> be the primary one. So my original patch id fine as is.

I barely remember one discussion with Heikki a few years ago and the
subject of the discussion was the order inversion of the ACPI/OF vs.
SW in the fwnode list. But it might be that it was just for something
that never appeared upstream. In any case, the proposed by Dmitry is
more flexible (we won't need to change this again if needed in the
future).


> Either way works for me. If you prefer the above approach instead of my
> original patch let me know and I'll give it a test-run and then post a v2.

+1 for v2.

-- 
With Best Regards,
Andy Shevchenko

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

[Bartosz Golaszewski]

On Mon, 15 Sep 2025 19:49:16 +0200, Hans de Goede <hansg@kernel.org> said:
> Hi,
>
> On 15-Sep-25 3:21 AM, Dmitry Torokhov wrote:
>> Hi Hans,
>>
>> On Sat, Sep 13, 2025 at 08:43:09PM +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 check to gpiod_find_by_fwnode() to try a software-node lookup on
>>> the secondary fwnode if the GPIO was not found in the primary fwnode.
>>
>> Thanks for catching this. I think it would be better if we added
>> handling of the secondary node to gpiod_find_and_request(). This way the
>> fallback will work for all kind of combinations, even if secondary node
>> happens to be an OF or ACPI one.
>
> IOW something like this:
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index b619fea498c8..1a3b5ca00554 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4630,6 +4630,13 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
>  	scoped_guard(srcu, &gpio_devices_srcu) {
>  		desc = gpiod_find_by_fwnode(fwnode, consumer, con_id, idx,
>  					    &flags, &lookupflags);
> +
> +		if (gpiod_not_found(desc) && fwnode) {
> +			dev_dbg(consumer, "trying secondary fwnode for GPIO lookup\n");
> +			desc = gpiod_find_by_fwnode(fwnode->secondary, 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
>
> That should work too, but if there is an OF or ACPI node it should always
> be the primary one. So my original patch id fine as is.
>
> Either way works for me. If you prefer the above approach instead of my
> original patch let me know and I'll give it a test-run and then post a v2.
>

I'm fine either way but if you go with the above (looks like it's the
consensus) then please wrap it in its own helper, possibly called something
like gpiod_fwnode_lookup() and put this primary-secondary logic in there to
move it out of gpiod_find_and_request().

Bartosz