[PATCH v2 0/2] driver core: provide and use device_match_fwnode_ext()

[PATCH v2 0/2] driver core: provide and use device_match_fwnode_ext()

[Bartosz Golaszewski]

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
exposing fwnode_is_primary() to drivers and reworking
gpio_chip_match_by_fwnode().

Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@google.com/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v2:
- Fold the code initially put into driver code into GPIOLIB as advised
  by Rafael
- Rework the logic as suggested by Andy
- To that end: make fwnode_is_primary() public
- Link to v1: https://patch.msgid.link/20260219-device-match-secondary-fwnode-v1-0-a64e8d4754bc@oss.qualcomm.com

---
Bartosz Golaszewski (2):
      driver core: make fwnode_is_primary() public
      gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

 drivers/base/core.c    |  5 -----
 drivers/gpio/gpiolib.c | 12 +++++++++++-
 include/linux/fwnode.h |  5 +++++
 3 files changed, 16 insertions(+), 6 deletions(-)
---
base-commit: 50f68cc7be0a2cbf54d8f6aaf17df32fb01acc3f
change-id: 20260219-device-match-secondary-fwnode-4cc163ec47ee

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Bartosz Golaszewski]

Export fwnode_is_primary() in fwnode.h for use in driver code.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/base/core.c    | 5 -----
 include/linux/fwnode.h | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index f599a1384eec90c104601422b04dc2b4c19d4382..2e551bbe591b09c66b113b419ba63f17e8bea94a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5122,11 +5122,6 @@ int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(dev_warn_probe);
 
-static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
-{
-	return fwnode && !IS_ERR(fwnode->secondary);
-}
-
 /**
  * set_primary_fwnode - Change the primary firmware node of a given device.
  * @dev: Device to handle.
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 097be89487bf5c5a96f01d569c1691088db4f0ca..04db7f3ea50cceb9025c82c6449ba342d0e1b4a4 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
 void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
 bool fw_devlink_is_strict(void);
 
+static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
+{
+	return fwnode && !IS_ERR(fwnode->secondary);
+}
+
 #endif

-- 
2.47.3

[PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Bartosz Golaszewski]

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;
 }
 
 /**

-- 
2.47.3

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Rafael J. Wysocki]

On Mon, Feb 23, 2026 at 4:41 PM Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> wrote:
>
> Export fwnode_is_primary() in fwnode.h for use in driver code.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

> ---
>  drivers/base/core.c    | 5 -----
>  include/linux/fwnode.h | 5 +++++
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index f599a1384eec90c104601422b04dc2b4c19d4382..2e551bbe591b09c66b113b419ba63f17e8bea94a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -5122,11 +5122,6 @@ int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL_GPL(dev_warn_probe);
>
> -static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> -{
> -       return fwnode && !IS_ERR(fwnode->secondary);
> -}
> -
>  /**
>   * set_primary_fwnode - Change the primary firmware node of a given device.
>   * @dev: Device to handle.
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 097be89487bf5c5a96f01d569c1691088db4f0ca..04db7f3ea50cceb9025c82c6449ba342d0e1b4a4 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
>  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  bool fw_devlink_is_strict(void);
>
> +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> +{
> +       return fwnode && !IS_ERR(fwnode->secondary);
> +}
> +
>  #endif
>
> --
> 2.47.3
>
>

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

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

Re: [PATCH v2 0/2] driver core: provide and use device_match_fwnode_ext()

[Bartosz Golaszewski]

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
> exposing fwnode_is_primary() to drivers and reworking
> gpio_chip_match_by_fwnode().
>
> Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@google.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---

Forgot to pick up Linus' R-b:

Reviewed-by: Linus Walleij <linusw@kernel.org>

Rafael: is it fine if I take it through the GPIO tree?

Bart

Re: [PATCH v2 0/2] driver core: provide and use device_match_fwnode_ext()

[Rafael J. Wysocki]

On Mon, Feb 23, 2026 at 4:46 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> 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
> > exposing fwnode_is_primary() to drivers and reworking
> > gpio_chip_match_by_fwnode().
> >
> > Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@google.com/
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > ---
>
> Forgot to pick up Linus' R-b:
>
> Reviewed-by: Linus Walleij <linusw@kernel.org>
>
> Rafael: is it fine if I take it through the GPIO tree?

Sure, no problem.

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Sakari Ailus]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

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.

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Dmitry Torokhov]

Hi Bartosz,

On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> Export fwnode_is_primary() in fwnode.h for use in driver code.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/base/core.c    | 5 -----
>  include/linux/fwnode.h | 5 +++++
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index f599a1384eec90c104601422b04dc2b4c19d4382..2e551bbe591b09c66b113b419ba63f17e8bea94a 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -5122,11 +5122,6 @@ int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
>  }
>  EXPORT_SYMBOL_GPL(dev_warn_probe);
>  
> -static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> -{
> -	return fwnode && !IS_ERR(fwnode->secondary);
> -}
> -
>  /**
>   * set_primary_fwnode - Change the primary firmware node of a given device.
>   * @dev: Device to handle.
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index 097be89487bf5c5a96f01d569c1691088db4f0ca..04db7f3ea50cceb9025c82c6449ba342d0e1b4a4 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
>  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  bool fw_devlink_is_strict(void);
>  
> +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> +{
> +	return fwnode && !IS_ERR(fwnode->secondary);
> +}

I think this is a bad API to be exported for wider use. It is surprising
that a standalone node is not considered to be a primary. It is also not
great that the argument is not constant pointer.

I would suggest having something like

bool fwnode_has_secondary(const struct fwnode_handle *fwnode);

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> Export fwnode_is_primary() in fwnode.h for use in driver code.

...

> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
>  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
>  bool fw_devlink_is_strict(void);
>  
> +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> +{
> +	return fwnode && !IS_ERR(fwnode->secondary);
> +}

This is inconsistent. Please, split out fwnode stuff from device.h to
device/fwnode.h and share it there.

This reminds me to look what I have locally in development...


(With your patch it will be in device.h and fwnode.h and in the latter
 it's even not properly grouped with other non-fwdevlink related stuff.)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Bartosz Golaszewski]

On Mon, Feb 23, 2026 at 6:54 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> > Export fwnode_is_primary() in fwnode.h for use in driver code.
>
> ...
>
> > --- a/include/linux/fwnode.h
> > +++ b/include/linux/fwnode.h
> > @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
> >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> >  bool fw_devlink_is_strict(void);
> >
> > +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > +{
> > +     return fwnode && !IS_ERR(fwnode->secondary);
> > +}
>
> This is inconsistent. Please, split out fwnode stuff from device.h to
> device/fwnode.h and share it there.
>
> This reminds me to look what I have locally in development...
>
>
> (With your patch it will be in device.h and fwnode.h and in the latter
>  it's even not properly grouped with other non-fwdevlink related stuff.)

Please rephrase the entire email because I have no idea what you mean. :(

Bart

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Rafael J. Wysocki]

On Mon, Feb 23, 2026 at 7:29 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Mon, Feb 23, 2026 at 6:54 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> > > Export fwnode_is_primary() in fwnode.h for use in driver code.
> >
> > ...
> >
> > > --- a/include/linux/fwnode.h
> > > +++ b/include/linux/fwnode.h
> > > @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
> > >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > >  bool fw_devlink_is_strict(void);
> > >
> > > +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > > +{
> > > +     return fwnode && !IS_ERR(fwnode->secondary);
> > > +}
> >
> > This is inconsistent. Please, split out fwnode stuff from device.h to
> > device/fwnode.h and share it there.
> >
> > This reminds me to look what I have locally in development...
> >
> >
> > (With your patch it will be in device.h and fwnode.h and in the latter
> >  it's even not properly grouped with other non-fwdevlink related stuff.)
>
> Please rephrase the entire email because I have no idea what you mean. :(

I thought Andy wanted fwnode_is_primary() to go into fwnode.h, but
that's where it is already going.  Confused. :-/

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 07:28:48PM +0100, Bartosz Golaszewski wrote:
> On Mon, Feb 23, 2026 at 6:54 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> > > Export fwnode_is_primary() in fwnode.h for use in driver code.

...

> > > --- a/include/linux/fwnode.h
> > > +++ b/include/linux/fwnode.h
> > > @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
> > >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > >  bool fw_devlink_is_strict(void);
> > >
> > > +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > > +{
> > > +     return fwnode && !IS_ERR(fwnode->secondary);
> > > +}
> >
> > This is inconsistent. Please, split out fwnode stuff from device.h to
> > device/fwnode.h and share it there.
> >
> > This reminds me to look what I have locally in development...
> >
> >
> > (With your patch it will be in device.h and fwnode.h and in the latter
> >  it's even not properly grouped with other non-fwdevlink related stuff.)
> 
> Please rephrase the entire email because I have no idea what you mean. :(

The primary/secondary and other device-fwnode related stuff is currently
exposed via include/linux/device.h. The problem is that device.h is overloaded
and starves for more splitting, which I'm doing (very slowly, though).
The idea is to have all device-fwnode  (and maybe of_node) stuff to be gathered in
include/linux/device/fwnode.h

You, guys, missed the keyword 'device' in the pathname for the proposed
[include/linux/device/]fwnode.h.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Danilo Krummrich]

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/

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Rafael J. Wysocki]

On Mon, Feb 23, 2026 at 8:32 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Feb 23, 2026 at 07:28:48PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Feb 23, 2026 at 6:54 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> > > > Export fwnode_is_primary() in fwnode.h for use in driver code.
>
> ...
>
> > > > --- a/include/linux/fwnode.h
> > > > +++ b/include/linux/fwnode.h
> > > > @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
> > > >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > > >  bool fw_devlink_is_strict(void);
> > > >
> > > > +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > > > +{
> > > > +     return fwnode && !IS_ERR(fwnode->secondary);
> > > > +}
> > >
> > > This is inconsistent. Please, split out fwnode stuff from device.h to
> > > device/fwnode.h and share it there.
> > >
> > > This reminds me to look what I have locally in development...
> > >
> > >
> > > (With your patch it will be in device.h and fwnode.h and in the latter
> > >  it's even not properly grouped with other non-fwdevlink related stuff.)
> >
> > Please rephrase the entire email because I have no idea what you mean. :(
>
> The primary/secondary and other device-fwnode related stuff is currently
> exposed via include/linux/device.h. The problem is that device.h is overloaded
> and starves for more splitting, which I'm doing (very slowly, though).
> The idea is to have all device-fwnode  (and maybe of_node) stuff to be gathered in
> include/linux/device/fwnode.h

I don't see "struct device" anywhere in fwnode_is_primary().  This
check is only about whether or not the given fwnode has a valid
secondary fwnode.

> You, guys, missed the keyword 'device' in the pathname for the proposed
> [include/linux/device/]fwnode.h.

Why do you think we missed it?

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

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/

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Danilo Krummrich]

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/

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

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/

Re: [PATCH v2 1/2] driver core: make fwnode_is_primary() public

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 08:45:42PM +0100, Rafael J. Wysocki wrote:
> On Mon, Feb 23, 2026 at 8:32 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Feb 23, 2026 at 07:28:48PM +0100, Bartosz Golaszewski wrote:
> > > On Mon, Feb 23, 2026 at 6:54 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Mon, Feb 23, 2026 at 04:40:52PM +0100, Bartosz Golaszewski wrote:
> > > > > Export fwnode_is_primary() in fwnode.h for use in driver code.

...

> > > > > --- a/include/linux/fwnode.h
> > > > > +++ b/include/linux/fwnode.h
> > > > > @@ -230,4 +230,9 @@ void fwnode_links_purge(struct fwnode_handle *fwnode);
> > > > >  void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode);
> > > > >  bool fw_devlink_is_strict(void);
> > > > >
> > > > > +static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
> > > > > +{
> > > > > +     return fwnode && !IS_ERR(fwnode->secondary);
> > > > > +}
> > > >
> > > > This is inconsistent. Please, split out fwnode stuff from device.h to
> > > > device/fwnode.h and share it there.
> > > >
> > > > This reminds me to look what I have locally in development...
> > > >
> > > > (With your patch it will be in device.h and fwnode.h and in the latter
> > > >  it's even not properly grouped with other non-fwdevlink related stuff.)
> > >
> > > Please rephrase the entire email because I have no idea what you mean. :(
> >
> > The primary/secondary and other device-fwnode related stuff is currently
> > exposed via include/linux/device.h. The problem is that device.h is overloaded
> > and starves for more splitting, which I'm doing (very slowly, though).
> > The idea is to have all device-fwnode  (and maybe of_node) stuff to be gathered in
> > include/linux/device/fwnode.h
> 
> I don't see "struct device" anywhere in fwnode_is_primary().  This
> check is only about whether or not the given fwnode has a valid
> secondary fwnode.

I am talking about splitting device-fwnode related API to
include/linux/device/fwnode.h. The idea of primary/secondary comes from the
upper layer (device) as struct fwnode_handle just defines a 'secondary' member
for a single linked list. It doesn't seem to limit anyhow the list.

My understanding is that the fwnode_is_primary() belongs to the device layer more
than to fwnode one. fwnode layer doesn't (clearly?) define the use cases
and in my opinion should not, fwnode_handle is more abstract and shouldn't
be limited to primary/secondary division that is currently related to the
device. Maybe I am missing something obvious... but to me spreading these
APIs into fwnode.h sounds like layering violation (especially if we think
of the future decoupling fwnode from struct device and making it a separate
entity).

> > You, guys, missed the keyword 'device' in the pathname for the proposed
> > [include/linux/device/]fwnode.h.
> 
> Why do you think we missed it?

Because of the previous comment that suggest that I wanted to move the code to
fwnode.h, but I was talking about device/fwnode.h (which is currently absent).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Rafael J. Wysocki]

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

>  }
>
>  /**
>
> --

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Sakari Ailus]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Bartosz Golaszewski]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Sakari Ailus]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Andy Shevchenko]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Sakari Ailus]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Andy Shevchenko]

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

Re: [PATCH v2 2/2] gpiolib: match secondary fwnode too in gpio_device_find_by_fwnode()

[Heikki Krogerus]

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