* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 15:40 ` [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode() Bartosz Golaszewski
@ 2026-02-23 15:43 ` Rafael J. Wysocki
2026-02-23 17:23 ` Sakari Ailus
` (2 subsequent siblings)
3 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 15:43 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Len Brown, driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 4:41 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> reference to a specific fwnode, we only match it against the primary
> node of the controllers. Let's extend that to also the secondary node by
> reworking gpio_chip_match_by_fwnode()
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
> ---
> drivers/gpio/gpiolib.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..7fe1d9ab1281d6c5022b9bdd8909fef2cb74122e 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -11,6 +11,7 @@
> #include <linux/errno.h>
> #include <linux/file.h>
> #include <linux/fs.h>
> +#include <linux/fwnode.h>
> #include <linux/idr.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
>
> static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> {
> - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> + struct device *dev = &gc->gpiodev->dev;
> + struct fwnode_handle *node = dev_fwnode(dev);
> +
> + if (IS_ERR(fwnode))
> + return 0;
> +
> + if (device_match_fwnode(dev, fwnode))
> + return 1;
> +
> + return fwnode_is_primary(node) && node->secondary == fwnode;
> }
>
> /**
>
> --
> 2.47.3
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 15:40 ` [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode() Bartosz Golaszewski
2026-02-23 15:43 ` Rafael J. Wysocki
@ 2026-02-23 17:23 ` Sakari Ailus
2026-02-23 17:46 ` Rafael J. Wysocki
2026-02-23 19:45 ` Danilo Krummrich
2026-02-23 20:46 ` Rafael J. Wysocki
3 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2026-02-23 17:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
Hi Bartosz,
Thanks for the patch.
On Mon, Feb 23, 2026 at 04:40:53PM +0100, Bartosz Golaszewski wrote:
> In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> reference to a specific fwnode, we only match it against the primary
> node of the controllers. Let's extend that to also the secondary node by
> reworking gpio_chip_match_by_fwnode()
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> drivers/gpio/gpiolib.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..7fe1d9ab1281d6c5022b9bdd8909fef2cb74122e 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -11,6 +11,7 @@
> #include <linux/errno.h>
> #include <linux/file.h>
> #include <linux/fs.h>
> +#include <linux/fwnode.h>
> #include <linux/idr.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
>
> static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> {
> - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> + struct device *dev = &gc->gpiodev->dev;
> + struct fwnode_handle *node = dev_fwnode(dev);
> +
> + if (IS_ERR(fwnode))
> + return 0;
> +
> + if (device_match_fwnode(dev, fwnode))
Could device_match_fwnode() match secondary fwnode as well? AFAIU, the
secondary fwnode only exists because it originates from a different fwnode
backend, typically software node, while still representing the same node.
> + return 1;
> +
> + return fwnode_is_primary(node) && node->secondary == fwnode;
> }
>
> /**
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 17:23 ` Sakari Ailus
@ 2026-02-23 17:46 ` Rafael J. Wysocki
2026-02-23 22:07 ` Sakari Ailus
0 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 17:46 UTC (permalink / raw)
To: Sakari Ailus, Andy Shevchenko
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Danilo Krummrich,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Daniel Scally, Heikki Krogerus, Len Brown, driver-core,
linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 6:23 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Bartosz,
>
> Thanks for the patch.
>
> On Mon, Feb 23, 2026 at 04:40:53PM +0100, Bartosz Golaszewski wrote:
> > In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> > reference to a specific fwnode, we only match it against the primary
> > node of the controllers. Let's extend that to also the secondary node by
> > reworking gpio_chip_match_by_fwnode()
> >
> > Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > ---
> > drivers/gpio/gpiolib.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..7fe1d9ab1281d6c5022b9bdd8909fef2cb74122e 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -11,6 +11,7 @@
> > #include <linux/errno.h>
> > #include <linux/file.h>
> > #include <linux/fs.h>
> > +#include <linux/fwnode.h>
> > #include <linux/idr.h>
> > #include <linux/interrupt.h>
> > #include <linux/irq.h>
> > @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
> >
> > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > {
> > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > + struct device *dev = &gc->gpiodev->dev;
> > + struct fwnode_handle *node = dev_fwnode(dev);
> > +
> > + if (IS_ERR(fwnode))
> > + return 0;
> > +
> > + if (device_match_fwnode(dev, fwnode))
>
> Could device_match_fwnode() match secondary fwnode as well?
In the previous discussion on this, Andy was against doing that due to
the concern that it might introduce subtle bugs, which I agree with.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 17:46 ` Rafael J. Wysocki
@ 2026-02-23 22:07 ` Sakari Ailus
2026-02-24 8:47 ` Bartosz Golaszewski
0 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2026-02-23 22:07 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andy Shevchenko, Bartosz Golaszewski, Greg Kroah-Hartman,
Danilo Krummrich, Linus Walleij, Bartosz Golaszewski,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
Hi Rafael,
On Mon, Feb 23, 2026 at 06:46:38PM +0100, Rafael J. Wysocki wrote:
> On Mon, Feb 23, 2026 at 6:23 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Bartosz,
> >
> > Thanks for the patch.
> >
> > On Mon, Feb 23, 2026 at 04:40:53PM +0100, Bartosz Golaszewski wrote:
> > > In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> > > reference to a specific fwnode, we only match it against the primary
> > > node of the controllers. Let's extend that to also the secondary node by
> > > reworking gpio_chip_match_by_fwnode()
> > >
> > > Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > > ---
> > > drivers/gpio/gpiolib.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..7fe1d9ab1281d6c5022b9bdd8909fef2cb74122e 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -11,6 +11,7 @@
> > > #include <linux/errno.h>
> > > #include <linux/file.h>
> > > #include <linux/fs.h>
> > > +#include <linux/fwnode.h>
> > > #include <linux/idr.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/irq.h>
> > > @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
> > >
> > > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > > {
> > > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > > + struct device *dev = &gc->gpiodev->dev;
> > > + struct fwnode_handle *node = dev_fwnode(dev);
> > > +
> > > + if (IS_ERR(fwnode))
> > > + return 0;
> > > +
> > > + if (device_match_fwnode(dev, fwnode))
> >
> > Could device_match_fwnode() match secondary fwnode as well?
>
> In the previous discussion on this, Andy was against doing that due to
> the concern that it might introduce subtle bugs, which I agree with.
Could you elaborate or provide an example?
The function has some 27 users although few are individual drivers.
My understanding is that we only have the secondary fwnode for being able
to attach objects from different backend to the same node. The fwnode API
in the meantime generally tries to hide the existence of the secondary
fwnode; a rewrite (which ideally would have happened perhaps a few years
ago?) would probably make the fwnode a linked list instead so we'd lose
that secondary pointer in the process.
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 22:07 ` Sakari Ailus
@ 2026-02-24 8:47 ` Bartosz Golaszewski
2026-02-24 8:56 ` Sakari Ailus
0 siblings, 1 reply; 28+ messages in thread
From: Bartosz Golaszewski @ 2026-02-24 8:47 UTC (permalink / raw)
To: Sakari Ailus
Cc: Rafael J. Wysocki, Andy Shevchenko, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> > > >
> > > > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > > > {
> > > > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > > > + struct device *dev = &gc->gpiodev->dev;
> > > > + struct fwnode_handle *node = dev_fwnode(dev);
> > > > +
> > > > + if (IS_ERR(fwnode))
> > > > + return 0;
> > > > +
> > > > + if (device_match_fwnode(dev, fwnode))
> > >
> > > Could device_match_fwnode() match secondary fwnode as well?
> >
> > In the previous discussion on this, Andy was against doing that due to
> > the concern that it might introduce subtle bugs, which I agree with.
>
> Could you elaborate or provide an example?
>
> The function has some 27 users although few are individual drivers.
>
> My understanding is that we only have the secondary fwnode for being able
> to attach objects from different backend to the same node. The fwnode API
> in the meantime generally tries to hide the existence of the secondary
> fwnode; a rewrite (which ideally would have happened perhaps a few years
> ago?) would probably make the fwnode a linked list instead so we'd lose
> that secondary pointer in the process.
>
It already is a (singly) linked list. Ideally it would be a
doubly-linked list moved into struct device with struct fwnode_handle
having no concept of primary and secondary nodes.
Bartosz
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-24 8:47 ` Bartosz Golaszewski
@ 2026-02-24 8:56 ` Sakari Ailus
2026-02-24 9:43 ` Andy Shevchenko
0 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2026-02-24 8:56 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Rafael J. Wysocki, Andy Shevchenko, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
Hi Bartosz,
On Tue, Feb 24, 2026 at 09:47:57AM +0100, Bartosz Golaszewski wrote:
> On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > > > >
> > > > > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > > > > {
> > > > > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > > > > + struct device *dev = &gc->gpiodev->dev;
> > > > > + struct fwnode_handle *node = dev_fwnode(dev);
> > > > > +
> > > > > + if (IS_ERR(fwnode))
> > > > > + return 0;
> > > > > +
> > > > > + if (device_match_fwnode(dev, fwnode))
> > > >
> > > > Could device_match_fwnode() match secondary fwnode as well?
> > >
> > > In the previous discussion on this, Andy was against doing that due to
> > > the concern that it might introduce subtle bugs, which I agree with.
> >
> > Could you elaborate or provide an example?
> >
> > The function has some 27 users although few are individual drivers.
> >
> > My understanding is that we only have the secondary fwnode for being able
> > to attach objects from different backend to the same node. The fwnode API
> > in the meantime generally tries to hide the existence of the secondary
> > fwnode; a rewrite (which ideally would have happened perhaps a few years
> > ago?) would probably make the fwnode a linked list instead so we'd lose
> > that secondary pointer in the process.
> >
>
> It already is a (singly) linked list. Ideally it would be a
With two entries at most.
> doubly-linked list moved into struct device with struct fwnode_handle
> having no concept of primary and secondary nodes.
I'd think we had that list in struct fwnode_handle, which will still
represent nodes. But let's see the details when someone gets to implement
it. :-)
--
Kind regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-24 8:56 ` Sakari Ailus
@ 2026-02-24 9:43 ` Andy Shevchenko
2026-02-25 7:39 ` Sakari Ailus
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2026-02-24 9:43 UTC (permalink / raw)
To: Sakari Ailus
Cc: Bartosz Golaszewski, Rafael J. Wysocki, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
On Tue, Feb 24, 2026 at 10:56:16AM +0200, Sakari Ailus wrote:
> On Tue, Feb 24, 2026 at 09:47:57AM +0100, Bartosz Golaszewski wrote:
> > On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
...
> > > > > > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > > > > > {
> > > > > > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > > > > > + struct device *dev = &gc->gpiodev->dev;
> > > > > > + struct fwnode_handle *node = dev_fwnode(dev);
> > > > > > +
> > > > > > + if (IS_ERR(fwnode))
> > > > > > + return 0;
> > > > > > +
> > > > > > + if (device_match_fwnode(dev, fwnode))
> > > > >
> > > > > Could device_match_fwnode() match secondary fwnode as well?
> > > >
> > > > In the previous discussion on this, Andy was against doing that due to
> > > > the concern that it might introduce subtle bugs, which I agree with.
> > >
> > > Could you elaborate or provide an example?
I believe you ask me. Okay, the sophisticated case I have in mind is the
intel_quark_i2c_gpio.c which provides a GPIO device with a list of children.
First of all, it seems broken as it rewrites the secondary link for the
I²C device. (Which makes me think that we need to have a copy of the
[primary] fwnode in the children devices of MFD, but I don't know how
to refcount that properly). The gpiolib-acpi-core.c has a matching function
via ACPI_HANDLE(). So it might be not affected by this.
What I don't know is USB Type-C and USB DWC3 code where it's much more
complicated. And I'm not in a position to state that the change won't
affect those.
> > > The function has some 27 users although few are individual drivers.
> > >
> > > My understanding is that we only have the secondary fwnode for being able
> > > to attach objects from different backend to the same node. The fwnode API
> > > in the meantime generally tries to hide the existence of the secondary
> > > fwnode; a rewrite (which ideally would have happened perhaps a few years
> > > ago?) would probably make the fwnode a linked list instead so we'd lose
> > > that secondary pointer in the process.
> >
> > It already is a (singly) linked list. Ideally it would be a
>
> With two entries at most.
There is no technical limitation based on the data type.
> > doubly-linked list moved into struct device with struct fwnode_handle
> > having no concept of primary and secondary nodes.
>
> I'd think we had that list in struct fwnode_handle, which will still
> represent nodes. But let's see the details when someone gets to implement
> it. :-)
In the case above single or double linked list doesn't solve the issue of
the corrupted (parent) fwnode. We need also to have a siblings list so it
looks more like a tree.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-24 9:43 ` Andy Shevchenko
@ 2026-02-25 7:39 ` Sakari Ailus
2026-02-25 9:38 ` Andy Shevchenko
0 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2026-02-25 7:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, Rafael J. Wysocki, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
Hi Andy,
On Tue, Feb 24, 2026 at 11:43:39AM +0200, Andy Shevchenko wrote:
> On Tue, Feb 24, 2026 at 10:56:16AM +0200, Sakari Ailus wrote:
> > On Tue, Feb 24, 2026 at 09:47:57AM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
> > > <sakari.ailus@linux.intel.com> wrote:
>
> ...
>
> > > > > > > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > > > > > > {
> > > > > > > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > > > > > > + struct device *dev = &gc->gpiodev->dev;
> > > > > > > + struct fwnode_handle *node = dev_fwnode(dev);
> > > > > > > +
> > > > > > > + if (IS_ERR(fwnode))
> > > > > > > + return 0;
> > > > > > > +
> > > > > > > + if (device_match_fwnode(dev, fwnode))
> > > > > >
> > > > > > Could device_match_fwnode() match secondary fwnode as well?
> > > > >
> > > > > In the previous discussion on this, Andy was against doing that due to
> > > > > the concern that it might introduce subtle bugs, which I agree with.
> > > >
> > > > Could you elaborate or provide an example?
>
> I believe you ask me. Okay, the sophisticated case I have in mind is the
> intel_quark_i2c_gpio.c which provides a GPIO device with a list of children.
>
> First of all, it seems broken as it rewrites the secondary link for the
> I²C device. (Which makes me think that we need to have a copy of the
> [primary] fwnode in the children devices of MFD, but I don't know how
> to refcount that properly). The gpiolib-acpi-core.c has a matching function
> via ACPI_HANDLE(). So it might be not affected by this.
>
> What I don't know is USB Type-C and USB DWC3 code where it's much more
> complicated. And I'm not in a position to state that the change won't
> affect those.
Any idea who has the hardware in these cases? There aren't that many users
of this function out there and I think at some point we do need to fix
this.
What we could also do is that we add another function that only cares about
the very fwnode you have at hand, switch the dubious cases to use that and
have the proper function test both available fwnodes. That'd get us on the
right path to fix this eventually, if not now.
>
> > > > The function has some 27 users although few are individual drivers.
> > > >
> > > > My understanding is that we only have the secondary fwnode for being able
> > > > to attach objects from different backend to the same node. The fwnode API
> > > > in the meantime generally tries to hide the existence of the secondary
> > > > fwnode; a rewrite (which ideally would have happened perhaps a few years
> > > > ago?) would probably make the fwnode a linked list instead so we'd lose
> > > > that secondary pointer in the process.
> > >
> > > It already is a (singly) linked list. Ideally it would be a
> >
> > With two entries at most.
>
> There is no technical limitation based on the data type.
There aren't any, no, but the current implementation assumes this, and I
wouldn't change this without changing the data structure as well.
>
> > > doubly-linked list moved into struct device with struct fwnode_handle
> > > having no concept of primary and secondary nodes.
> >
> > I'd think we had that list in struct fwnode_handle, which will still
> > represent nodes. But let's see the details when someone gets to implement
> > it. :-)
>
> In the case above single or double linked list doesn't solve the issue of
> the corrupted (parent) fwnode. We need also to have a siblings list so it
> looks more like a tree.
>
--
Regards,
Sakari Ailus
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-25 7:39 ` Sakari Ailus
@ 2026-02-25 9:38 ` Andy Shevchenko
2026-02-25 10:07 ` Heikki Krogerus
0 siblings, 1 reply; 28+ messages in thread
From: Andy Shevchenko @ 2026-02-25 9:38 UTC (permalink / raw)
To: Sakari Ailus
Cc: Bartosz Golaszewski, Rafael J. Wysocki, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Heikki Krogerus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
On Wed, Feb 25, 2026 at 09:39:12AM +0200, Sakari Ailus wrote:
> On Tue, Feb 24, 2026 at 11:43:39AM +0200, Andy Shevchenko wrote:
> > On Tue, Feb 24, 2026 at 10:56:16AM +0200, Sakari Ailus wrote:
> > > On Tue, Feb 24, 2026 at 09:47:57AM +0100, Bartosz Golaszewski wrote:
> > > > On Mon, Feb 23, 2026 at 11:07 PM Sakari Ailus
> > > > <sakari.ailus@linux.intel.com> wrote:
...
> > > > > > > Could device_match_fwnode() match secondary fwnode as well?
> > > > > >
> > > > > > In the previous discussion on this, Andy was against doing that due to
> > > > > > the concern that it might introduce subtle bugs, which I agree with.
> > > > >
> > > > > Could you elaborate or provide an example?
> >
> > I believe you ask me. Okay, the sophisticated case I have in mind is the
> > intel_quark_i2c_gpio.c which provides a GPIO device with a list of children.
> >
> > First of all, it seems broken as it rewrites the secondary link for the
> > I²C device. (Which makes me think that we need to have a copy of the
> > [primary] fwnode in the children devices of MFD, but I don't know how
> > to refcount that properly). The gpiolib-acpi-core.c has a matching function
> > via ACPI_HANDLE(). So it might be not affected by this.
> >
> > What I don't know is USB Type-C and USB DWC3 code where it's much more
> > complicated. And I'm not in a position to state that the change won't
> > affect those.
>
> Any idea who has the hardware in these cases? There aren't that many users
> of this function out there and I think at some point we do need to fix
> this.
Ask Heikki?
> What we could also do is that we add another function that only cares about
> the very fwnode you have at hand, switch the dubious cases to use that and
> have the proper function test both available fwnodes. That'd get us on the
> right path to fix this eventually, if not now.
>
> > > > > The function has some 27 users although few are individual drivers.
> > > > >
> > > > > My understanding is that we only have the secondary fwnode for being able
> > > > > to attach objects from different backend to the same node. The fwnode API
> > > > > in the meantime generally tries to hide the existence of the secondary
> > > > > fwnode; a rewrite (which ideally would have happened perhaps a few years
> > > > > ago?) would probably make the fwnode a linked list instead so we'd lose
> > > > > that secondary pointer in the process.
> > > >
> > > > It already is a (singly) linked list. Ideally it would be a
> > >
> > > With two entries at most.
> >
> > There is no technical limitation based on the data type.
>
> There aren't any, no, but the current implementation assumes this, and I
> wouldn't change this without changing the data structure as well.
How does it assume? A caller may crawl via the list pretending that each of
fwnode is "the head of the single linked list".
I would agree with you if the struct fwnode_handle was opaque, but it doesn't.
> > > > doubly-linked list moved into struct device with struct fwnode_handle
> > > > having no concept of primary and secondary nodes.
> > >
> > > I'd think we had that list in struct fwnode_handle, which will still
> > > represent nodes. But let's see the details when someone gets to implement
> > > it. :-)
> >
> > In the case above single or double linked list doesn't solve the issue of
> > the corrupted (parent) fwnode. We need also to have a siblings list so it
> > looks more like a tree.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-25 9:38 ` Andy Shevchenko
@ 2026-02-25 10:07 ` Heikki Krogerus
0 siblings, 0 replies; 28+ messages in thread
From: Heikki Krogerus @ 2026-02-25 10:07 UTC (permalink / raw)
To: Sakari Ailus, Andy Shevchenko
Cc: Bartosz Golaszewski, Rafael J. Wysocki, Bartosz Golaszewski,
Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij,
Dmitry Torokhov, Daniel Scally, Len Brown, driver-core,
linux-kernel, linux-gpio, linux-acpi
Hi Sakari, Andy,
> > > What I don't know is USB Type-C and USB DWC3 code where it's much more
> > > complicated. And I'm not in a position to state that the change won't
> > > affect those.
> >
> > Any idea who has the hardware in these cases? There aren't that many users
> > of this function out there and I think at some point we do need to fix
> > this.
>
> Ask Heikki?
I can test any fixes or changes that you guys want to propose for
this, np.
Br,
--
heikki
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 15:40 ` [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode() Bartosz Golaszewski
2026-02-23 15:43 ` Rafael J. Wysocki
2026-02-23 17:23 ` Sakari Ailus
@ 2026-02-23 19:45 ` Danilo Krummrich
2026-02-23 19:55 ` Rafael J. Wysocki
2026-02-23 20:46 ` Rafael J. Wysocki
3 siblings, 1 reply; 28+ messages in thread
From: Danilo Krummrich @ 2026-02-23 19:45 UTC (permalink / raw)
To: Bartosz Golaszewski, Rafael J. Wysocki
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon Feb 23, 2026 at 4:40 PM CET, Bartosz Golaszewski wrote:
> static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> {
> - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> + struct device *dev = &gc->gpiodev->dev;
> + struct fwnode_handle *node = dev_fwnode(dev);
> +
> + if (IS_ERR(fwnode))
> + return 0;
> +
> + if (device_match_fwnode(dev, fwnode))
> + return 1;
> +
> + return fwnode_is_primary(node) && node->secondary == fwnode;
> }
Rafael, I understand [1] as you agree with my point, but object to introduce
device_match_fwnode_ext() (or whatever name we would pick eventually :)
regardless because only the GPIO code would need it as by now.
IIUC, I wonder if exposing fwnode_is_primary() instead is a good trade.
[1] https://lore.kernel.org/driver-core/CAJZ5v0jUCtKTW-g-C0pKu0DQqOkyfSz=upXwbtYeV_=rMBUMyg@mail.gmail.com/
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 19:45 ` Danilo Krummrich
@ 2026-02-23 19:55 ` Rafael J. Wysocki
2026-02-23 20:00 ` Danilo Krummrich
0 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 19:55 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Bartosz Golaszewski, Rafael J. Wysocki, Greg Kroah-Hartman,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Len Brown, driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 8:45 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon Feb 23, 2026 at 4:40 PM CET, Bartosz Golaszewski wrote:
> > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> > {
> > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> > + struct device *dev = &gc->gpiodev->dev;
> > + struct fwnode_handle *node = dev_fwnode(dev);
> > +
> > + if (IS_ERR(fwnode))
> > + return 0;
> > +
> > + if (device_match_fwnode(dev, fwnode))
> > + return 1;
> > +
> > + return fwnode_is_primary(node) && node->secondary == fwnode;
> > }
>
> Rafael, I understand [1] as you agree with my point, but object to introduce
> device_match_fwnode_ext() (or whatever name we would pick eventually :)
> regardless because only the GPIO code would need it as by now.
This is a preference, not a strong objection, but yes.
> IIUC, I wonder if exposing fwnode_is_primary() instead is a good trade.
Well, there is the secondary pointer in struct fwnode_handle, so it is
kind of exported anyway and it could be documented as "a secondary
fwnode_handle supplying additional properties or an error pointer", so
exposing this static inline doesn't change much IMV.
What's your specific concern about exposing it?
> [1] https://lore.kernel.org/driver-core/CAJZ5v0jUCtKTW-g-C0pKu0DQqOkyfSz=upXwbtYeV_=rMBUMyg@mail.gmail.com/
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 19:55 ` Rafael J. Wysocki
@ 2026-02-23 20:00 ` Danilo Krummrich
2026-02-23 20:04 ` Rafael J. Wysocki
0 siblings, 1 reply; 28+ messages in thread
From: Danilo Krummrich @ 2026-02-23 20:00 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Linus Walleij,
Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Len Brown,
driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon Feb 23, 2026 at 8:55 PM CET, Rafael J. Wysocki wrote:
> On Mon, Feb 23, 2026 at 8:45 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Mon Feb 23, 2026 at 4:40 PM CET, Bartosz Golaszewski wrote:
>> > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
>> > {
>> > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
>> > + struct device *dev = &gc->gpiodev->dev;
>> > + struct fwnode_handle *node = dev_fwnode(dev);
>> > +
>> > + if (IS_ERR(fwnode))
>> > + return 0;
>> > +
>> > + if (device_match_fwnode(dev, fwnode))
>> > + return 1;
>> > +
>> > + return fwnode_is_primary(node) && node->secondary == fwnode;
>> > }
>>
>> Rafael, I understand [1] as you agree with my point, but object to introduce
>> device_match_fwnode_ext() (or whatever name we would pick eventually :)
>> regardless because only the GPIO code would need it as by now.
>
> This is a preference, not a strong objection, but yes.
>
>> IIUC, I wonder if exposing fwnode_is_primary() instead is a good trade.
>
> Well, there is the secondary pointer in struct fwnode_handle, so it is
> kind of exported anyway and it could be documented as "a secondary
> fwnode_handle supplying additional properties or an error pointer", so
> exposing this static inline doesn't change much IMV.
>
> What's your specific concern about exposing it?
No concern with either approach from my side, I was just curious. :)
Maybe it makes sense to add a comment to gpio_chip_match_by_fwnode() hinting to
move this into common code once there's another occurance of this pattern.
But either way, this is
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
>> [1] https://lore.kernel.org/driver-core/CAJZ5v0jUCtKTW-g-C0pKu0DQqOkyfSz=upXwbtYeV_=rMBUMyg@mail.gmail.com/
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 20:00 ` Danilo Krummrich
@ 2026-02-23 20:04 ` Rafael J. Wysocki
0 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 20:04 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Rafael J. Wysocki, Bartosz Golaszewski, Greg Kroah-Hartman,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Len Brown, driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 9:00 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon Feb 23, 2026 at 8:55 PM CET, Rafael J. Wysocki wrote:
> > On Mon, Feb 23, 2026 at 8:45 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Mon Feb 23, 2026 at 4:40 PM CET, Bartosz Golaszewski wrote:
> >> > static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> >> > {
> >> > - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> >> > + struct device *dev = &gc->gpiodev->dev;
> >> > + struct fwnode_handle *node = dev_fwnode(dev);
> >> > +
> >> > + if (IS_ERR(fwnode))
> >> > + return 0;
> >> > +
> >> > + if (device_match_fwnode(dev, fwnode))
> >> > + return 1;
> >> > +
> >> > + return fwnode_is_primary(node) && node->secondary == fwnode;
> >> > }
> >>
> >> Rafael, I understand [1] as you agree with my point, but object to introduce
> >> device_match_fwnode_ext() (or whatever name we would pick eventually :)
> >> regardless because only the GPIO code would need it as by now.
> >
> > This is a preference, not a strong objection, but yes.
> >
> >> IIUC, I wonder if exposing fwnode_is_primary() instead is a good trade.
> >
> > Well, there is the secondary pointer in struct fwnode_handle, so it is
> > kind of exported anyway and it could be documented as "a secondary
> > fwnode_handle supplying additional properties or an error pointer", so
> > exposing this static inline doesn't change much IMV.
> >
> > What's your specific concern about exposing it?
>
> No concern with either approach from my side, I was just curious. :)
>
> Maybe it makes sense to add a comment to gpio_chip_match_by_fwnode() hinting to
> move this into common code once there's another occurance of this pattern.
A comment like that might help, yes.
> But either way, this is
>
> Reviewed-by: Danilo Krummrich <dakr@kernel.org>
>
> >> [1] https://lore.kernel.org/driver-core/CAJZ5v0jUCtKTW-g-C0pKu0DQqOkyfSz=upXwbtYeV_=rMBUMyg@mail.gmail.com/
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()
2026-02-23 15:40 ` [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode() Bartosz Golaszewski
` (2 preceding siblings ...)
2026-02-23 19:45 ` Danilo Krummrich
@ 2026-02-23 20:46 ` Rafael J. Wysocki
3 siblings, 0 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 20:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Len Brown, driver-core, linux-kernel, linux-gpio, linux-acpi
On Mon, Feb 23, 2026 at 4:41 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> In GPIOLIB, during fwnode lookup, after having resolved the consumer's
> reference to a specific fwnode, we only match it against the primary
> node of the controllers. Let's extend that to also the secondary node by
> reworking gpio_chip_match_by_fwnode()
>
> Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
> drivers/gpio/gpiolib.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..7fe1d9ab1281d6c5022b9bdd8909fef2cb74122e 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -11,6 +11,7 @@
> #include <linux/errno.h>
> #include <linux/file.h>
> #include <linux/fs.h>
> +#include <linux/fwnode.h>
> #include <linux/idr.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> @@ -1395,7 +1396,16 @@ EXPORT_SYMBOL_GPL(gpio_device_find_by_label);
>
> static int gpio_chip_match_by_fwnode(struct gpio_chip *gc, const void *fwnode)
> {
> - return device_match_fwnode(&gc->gpiodev->dev, fwnode);
> + struct device *dev = &gc->gpiodev->dev;
> + struct fwnode_handle *node = dev_fwnode(dev);
> +
> + if (IS_ERR(fwnode))
> + return 0;
> +
> + if (device_match_fwnode(dev, fwnode))
> + return 1;
> +
> + return fwnode_is_primary(node) && node->secondary == fwnode;
Actually, you can replace the above statement with
return node && node->secondary == fwnode;
because fwnode_is_primary(node) expands to (node &&
!IS_ERR(node->secondary)), but since you compare node->secondary to
fwnode, the IS_ERR() check on it is redundant (fwnode is known to be
non-error already at this point).
That would allow you to avoid exporting fwnode_is_primary() (which is
kind of useful given the review comments).
> }
>
> /**
>
> --
^ permalink raw reply [flat|nested] 28+ messages in thread