* [PATCH 0/2] driver core: provide and use device_match_fwnode_ext()
@ 2026-02-19 16:31 Bartosz Golaszewski
2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2026-02-19 16:31 UTC (permalink / raw)
To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov,
Andy Shevchenko
Cc: driver-core, linux-kernel, linux-gpio, 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
providing a new variant of device_match_fwnode() and using it in GPIO
core.
Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@google.com/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Bartosz Golaszewski (2):
driver core: provide device_match_fwnode_ext()
gpiolib: use device_match_fwnode_ext()
drivers/base/core.c | 14 ++++++++++++++
drivers/gpio/gpiolib.c | 2 +-
include/linux/device/bus.h | 1 +
3 files changed, 16 insertions(+), 1 deletion(-)
---
base-commit: 50f68cc7be0a2cbf54d8f6aaf17df32fb01acc3f
change-id: 20260219-device-match-secondary-fwnode-4cc163ec47ee
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:31 [PATCH 0/2] driver core: provide and use device_match_fwnode_ext() Bartosz Golaszewski @ 2026-02-19 16:31 ` Bartosz Golaszewski 2026-02-19 16:36 ` Greg Kroah-Hartman 2026-02-19 19:58 ` Andy Shevchenko 2026-02-19 16:31 ` [PATCH 2/2] gpiolib: use device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 17:32 ` [PATCH 0/2] driver core: provide and " Linus Walleij 2 siblings, 2 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 16:31 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko Cc: driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski Provide an extended variant of device_match_fwnode() that also tries to match the device's secondary fwnode. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- drivers/base/core.c | 14 ++++++++++++++ include/linux/device/bus.h | 1 + 2 files changed, 15 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) } EXPORT_SYMBOL_GPL(device_match_fwnode); +int device_match_fwnode_ext(struct device *dev, const void *fwnode) +{ + struct fwnode_handle *dev_node = dev_fwnode(dev); + + if (!fwnode) + return 0; + + if (dev_node == fwnode) + return 1; + + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; +} +EXPORT_SYMBOL_GPL(device_match_fwnode_ext); + int device_match_devt(struct device *dev, const void *pdevt) { return dev->devt == *(dev_t *)pdevt; diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h index 99c3c83ea520876ab3577ffa76f159f89f4f86c5..bb6ed08c39aee497feabdba82d2e8f89ca1c8b5f 100644 --- a/include/linux/device/bus.h +++ b/include/linux/device/bus.h @@ -137,6 +137,7 @@ int device_match_name(struct device *dev, const void *name); int device_match_type(struct device *dev, const void *type); int device_match_of_node(struct device *dev, const void *np); int device_match_fwnode(struct device *dev, const void *fwnode); +int device_match_fwnode_ext(struct device *dev, const void *fwnode); int device_match_devt(struct device *dev, const void *pdevt); int device_match_acpi_dev(struct device *dev, const void *adev); int device_match_acpi_handle(struct device *dev, const void *handle); -- 2.47.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski @ 2026-02-19 16:36 ` Greg Kroah-Hartman 2026-02-19 16:39 ` Bartosz Golaszewski 2026-02-19 19:58 ` Andy Shevchenko 1 sibling, 1 reply; 29+ messages in thread From: Greg Kroah-Hartman @ 2026-02-19 16:36 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > Provide an extended variant of device_match_fwnode() that also tries to > match the device's secondary fwnode. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > --- > drivers/base/core.c | 14 ++++++++++++++ > include/linux/device/bus.h | 1 + > 2 files changed, 15 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) > } > EXPORT_SYMBOL_GPL(device_match_fwnode); > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) No kernel doc to explain what this function does? :( ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:36 ` Greg Kroah-Hartman @ 2026-02-19 16:39 ` Bartosz Golaszewski 2026-02-19 16:50 ` Greg Kroah-Hartman ` (2 more replies) 0 siblings, 3 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 16:39 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > Provide an extended variant of device_match_fwnode() that also tries to > > match the device's secondary fwnode. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > > --- > > drivers/base/core.c | 14 ++++++++++++++ > > include/linux/device/bus.h | 1 + > > 2 files changed, 15 insertions(+) > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 > > --- a/drivers/base/core.c > > +++ b/drivers/base/core.c > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) > > } > > EXPORT_SYMBOL_GPL(device_match_fwnode); > > > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > No kernel doc to explain what this function does? > > :( > It's not like any of the other variants from this file were documented but ok, I can add it in v2. Still, I'd like to hear if this even makes sense. Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:39 ` Bartosz Golaszewski @ 2026-02-19 16:50 ` Greg Kroah-Hartman 2026-02-19 16:54 ` Dmitry Torokhov 2026-02-19 16:55 ` Danilo Krummrich 2 siblings, 0 replies; 29+ messages in thread From: Greg Kroah-Hartman @ 2026-02-19 16:50 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: > On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > > Provide an extended variant of device_match_fwnode() that also tries to > > > match the device's secondary fwnode. > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > > > --- > > > drivers/base/core.c | 14 ++++++++++++++ > > > include/linux/device/bus.h | 1 + > > > 2 files changed, 15 insertions(+) > > > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 > > > --- a/drivers/base/core.c > > > +++ b/drivers/base/core.c > > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) > > > } > > > EXPORT_SYMBOL_GPL(device_match_fwnode); > > > > > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > > > No kernel doc to explain what this function does? > > > > :( > > > > It's not like any of the other variants from this file were documented > but ok, I can add it in v2. Still, I'd like to hear if this even makes > sense. I agree, most of them don't have documentation, but I can't figure out what _ext() means here so a bit of a hint would be nice :) I don't know if it makes sense, I'll let the others argue about that. thanks, greg k-h ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:39 ` Bartosz Golaszewski 2026-02-19 16:50 ` Greg Kroah-Hartman @ 2026-02-19 16:54 ` Dmitry Torokhov 2026-02-19 21:15 ` Bartosz Golaszewski 2026-02-19 16:55 ` Danilo Krummrich 2 siblings, 1 reply; 29+ messages in thread From: Dmitry Torokhov @ 2026-02-19 16:54 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Andy Shevchenko, driver-core, linux-kernel, linux-gpio Hi Bartosz, On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: > On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: > > > > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > > Provide an extended variant of device_match_fwnode() that also tries to > > > match the device's secondary fwnode. > > > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > > > --- > > > drivers/base/core.c | 14 ++++++++++++++ > > > include/linux/device/bus.h | 1 + > > > 2 files changed, 15 insertions(+) > > > > > > diff --git a/drivers/base/core.c b/drivers/base/core.c > > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 > > > --- a/drivers/base/core.c > > > +++ b/drivers/base/core.c > > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) > > > } > > > EXPORT_SYMBOL_GPL(device_match_fwnode); > > > > > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > > > No kernel doc to explain what this function does? > > > > :( > > > > It's not like any of the other variants from this file were documented > but ok, I can add it in v2. Still, I'd like to hear if this even makes > sense. I think it really needs a good explanation given how it goes through secondaries on one side but not on the other (but maybe it should? Why one would not want to match secondary?) I also not fond of the name and still not quite sure if we need a special variant or if the normal device_match_fwnode() should be adjusted and its callers (there are about 20) should be audited (and maybe if the actually rely on current behavior we can introduce device_match_primary_fwnode() instead). And it should be a 'bool' (and we should fix the rest of them as well). Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:54 ` Dmitry Torokhov @ 2026-02-19 21:15 ` Bartosz Golaszewski 2026-02-20 0:47 ` Dmitry Torokhov 0 siblings, 1 reply; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 21:15 UTC (permalink / raw) To: Dmitry Torokhov Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, 19 Feb 2026 17:54:24 +0100, Dmitry Torokhov <dmitry.torokhov@gmail.com> said: > Hi Bartosz, > > On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: >> On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >> > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >> > > Provide an extended variant of device_match_fwnode() that also tries to >> > > match the device's secondary fwnode. >> > > >> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >> > > --- >> > > drivers/base/core.c | 14 ++++++++++++++ >> > > include/linux/device/bus.h | 1 + >> > > 2 files changed, 15 insertions(+) >> > > >> > > diff --git a/drivers/base/core.c b/drivers/base/core.c >> > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 >> > > --- a/drivers/base/core.c >> > > +++ b/drivers/base/core.c >> > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) >> > > } >> > > EXPORT_SYMBOL_GPL(device_match_fwnode); >> > > >> > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >> > >> > No kernel doc to explain what this function does? >> > >> > :( >> > >> >> It's not like any of the other variants from this file were documented >> but ok, I can add it in v2. Still, I'd like to hear if this even makes >> sense. > > I think it really needs a good explanation given how it goes through > secondaries on one side but not on the other (but maybe it should? Why > one would not want to match secondary?) > I don't think it should. You have one, concrete fwnode and you want to match it against a struct device: in this variant both its primary and secondary nodes. I don't think we should do a four-way matching. Bart > I also not fond of the name and still not quite sure if we need a > special variant or if the normal device_match_fwnode() should be > adjusted and its callers (there are about 20) should be audited (and > maybe if the actually rely on current behavior we can introduce > device_match_primary_fwnode() instead). > > And it should be a 'bool' (and we should fix the rest of them as well). > > Thanks. > > -- > Dmitry > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 21:15 ` Bartosz Golaszewski @ 2026-02-20 0:47 ` Dmitry Torokhov 2026-02-20 7:27 ` Andy Shevchenko 2026-02-20 11:06 ` Bartosz Golaszewski 0 siblings, 2 replies; 29+ messages in thread From: Dmitry Torokhov @ 2026-02-20 0:47 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 03:15:53PM -0600, Bartosz Golaszewski wrote: > On Thu, 19 Feb 2026 17:54:24 +0100, Dmitry Torokhov > <dmitry.torokhov@gmail.com> said: > > Hi Bartosz, > > > > On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: > >> On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman > >> <gregkh@linuxfoundation.org> wrote: > >> > > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > >> > > Provide an extended variant of device_match_fwnode() that also tries to > >> > > match the device's secondary fwnode. > >> > > > >> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > >> > > --- > >> > > drivers/base/core.c | 14 ++++++++++++++ > >> > > include/linux/device/bus.h | 1 + > >> > > 2 files changed, 15 insertions(+) > >> > > > >> > > diff --git a/drivers/base/core.c b/drivers/base/core.c > >> > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 > >> > > --- a/drivers/base/core.c > >> > > +++ b/drivers/base/core.c > >> > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) > >> > > } > >> > > EXPORT_SYMBOL_GPL(device_match_fwnode); > >> > > > >> > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > >> > > >> > No kernel doc to explain what this function does? > >> > > >> > :( > >> > > >> > >> It's not like any of the other variants from this file were documented > >> but ok, I can add it in v2. Still, I'd like to hear if this even makes > >> sense. > > > > I think it really needs a good explanation given how it goes through > > secondaries on one side but not on the other (but maybe it should? Why > > one would not want to match secondary?) > > > > I don't think it should. You have one, concrete fwnode and you want to match > it against a struct device: in this variant both its primary and secondary > nodes. I don't think we should do a four-way matching. I wonder why you consider these 2 distinct fwnodes instead of a single object that has multiple components? After all in device we have a pointer to fwnode, and not list of fwnodes.... Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 0:47 ` Dmitry Torokhov @ 2026-02-20 7:27 ` Andy Shevchenko 2026-02-20 11:06 ` Bartosz Golaszewski 1 sibling, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2026-02-20 7:27 UTC (permalink / raw) To: Dmitry Torokhov Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 04:47:59PM -0800, Dmitry Torokhov wrote: > On Thu, Feb 19, 2026 at 03:15:53PM -0600, Bartosz Golaszewski wrote: > > On Thu, 19 Feb 2026 17:54:24 +0100, Dmitry Torokhov > > <dmitry.torokhov@gmail.com> said: > > > On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: ... > > > I think it really needs a good explanation given how it goes through > > > secondaries on one side but not on the other (but maybe it should? Why > > > one would not want to match secondary?) > > > > > > > I don't think it should. You have one, concrete fwnode and you want to match > > it against a struct device: in this variant both its primary and secondary > > nodes. I don't think we should do a four-way matching. > > I wonder why you consider these 2 distinct fwnodes instead of a single > object that has multiple components? After all in device we have a > pointer to fwnode, and not list of fwnodes.... For the matter of fact the struct fwnode_handle is a single-linked list with a limitation to have up to two entries. And the second one is a problematic design as these years showed. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 0:47 ` Dmitry Torokhov 2026-02-20 7:27 ` Andy Shevchenko @ 2026-02-20 11:06 ` Bartosz Golaszewski 1 sibling, 0 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-20 11:06 UTC (permalink / raw) To: Dmitry Torokhov Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Fri, 20 Feb 2026 01:47:59 +0100, Dmitry Torokhov <dmitry.torokhov@gmail.com> said: > On Thu, Feb 19, 2026 at 03:15:53PM -0600, Bartosz Golaszewski wrote: >> On Thu, 19 Feb 2026 17:54:24 +0100, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> said: >> > Hi Bartosz, >> > >> > On Thu, Feb 19, 2026 at 05:39:47PM +0100, Bartosz Golaszewski wrote: >> >> On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman >> >> <gregkh@linuxfoundation.org> wrote: >> >> > >> >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >> >> > > Provide an extended variant of device_match_fwnode() that also tries to >> >> > > match the device's secondary fwnode. >> >> > > >> >> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >> >> > > --- >> >> > > drivers/base/core.c | 14 ++++++++++++++ >> >> > > include/linux/device/bus.h | 1 + >> >> > > 2 files changed, 15 insertions(+) >> >> > > >> >> > > diff --git a/drivers/base/core.c b/drivers/base/core.c >> >> > > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 >> >> > > --- a/drivers/base/core.c >> >> > > +++ b/drivers/base/core.c >> >> > > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) >> >> > > } >> >> > > EXPORT_SYMBOL_GPL(device_match_fwnode); >> >> > > >> >> > > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >> >> > >> >> > No kernel doc to explain what this function does? >> >> > >> >> > :( >> >> > >> >> >> >> It's not like any of the other variants from this file were documented >> >> but ok, I can add it in v2. Still, I'd like to hear if this even makes >> >> sense. >> > >> > I think it really needs a good explanation given how it goes through >> > secondaries on one side but not on the other (but maybe it should? Why >> > one would not want to match secondary?) >> > >> >> I don't think it should. You have one, concrete fwnode and you want to match >> it against a struct device: in this variant both its primary and secondary >> nodes. I don't think we should do a four-way matching. > > I wonder why you consider these 2 distinct fwnodes instead of a single > object that has multiple components? After all in device we have a > pointer to fwnode, and not list of fwnodes.... > Yes, and we've run into problems due to that. I can't find the series in question but someone had a device which in DT looked like this: parent { compatible = "foo,bar"; child@1 { ... }; child@1 { ... }; }; And children would inherit the firmware node of the parent but the user wanted to attach a separate software node to each child and that can't be done right now so ideally we'd move to having a list of firmware nodes in struct device and remove the secondary pointer from struct fwnode_handle. And to that end: I'd like to limit the usage of secondary fwnodes. In this case we have an fwnode address we get from fwnode_get_reference_args() and we match it against all fwnodes of a struct device. Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:39 ` Bartosz Golaszewski 2026-02-19 16:50 ` Greg Kroah-Hartman 2026-02-19 16:54 ` Dmitry Torokhov @ 2026-02-19 16:55 ` Danilo Krummrich 2026-02-19 19:27 ` Andy Shevchenko 2026-02-19 21:16 ` Bartosz Golaszewski 2 siblings, 2 replies; 29+ messages in thread From: Danilo Krummrich @ 2026-02-19 16:55 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: > On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> >> On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >> > Provide an extended variant of device_match_fwnode() that also tries to >> > match the device's secondary fwnode. >> > >> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >> > --- >> > drivers/base/core.c | 14 ++++++++++++++ >> > include/linux/device/bus.h | 1 + >> > 2 files changed, 15 insertions(+) >> > >> > diff --git a/drivers/base/core.c b/drivers/base/core.c >> > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 >> > --- a/drivers/base/core.c >> > +++ b/drivers/base/core.c >> > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) >> > } >> > EXPORT_SYMBOL_GPL(device_match_fwnode); >> > >> > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >> >> No kernel doc to explain what this function does? >> >> :( >> > > It's not like any of the other variants from this file were documented > but ok, I can add it in v2. Still, I'd like to hear if this even makes > sense. I'd argue that the other ones are very obvious, as they just encapsulate a single operation rather than any logic, whereas this one does have some logic. Also, is there a reason why we need both device_match_fwnode() *and* device_match_fwnode_ext()? ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:55 ` Danilo Krummrich @ 2026-02-19 19:27 ` Andy Shevchenko 2026-02-19 21:18 ` Bartosz Golaszewski 2026-02-19 21:16 ` Bartosz Golaszewski 1 sibling, 1 reply; 29+ messages in thread From: Andy Shevchenko @ 2026-02-19 19:27 UTC (permalink / raw) To: Danilo Krummrich Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 05:55:20PM +0100, Danilo Krummrich wrote: > On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: ... > Also, is there a reason why we need both device_match_fwnode() *and* > device_match_fwnode_ext()? Yes. We don't want (at least for now) to dive into bug hunting in a 2+ years horizon if something goes wrong with [currently] working drivers that use device_match_fwnode() against the cases when there are primary and secondary fwnodes present. I won't put my bet that extending device_match_fwnode() won't break anything. And I don't want to invest (waste?) my time to learn each of the existing cases. The proposed way is robust and safest. And for the record, I will be the first person to push back device_match_fwnode() upgrade without a comprehensive testing on real (affected) HW. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 19:27 ` Andy Shevchenko @ 2026-02-19 21:18 ` Bartosz Golaszewski 2026-02-20 0:21 ` Danilo Krummrich 2026-02-20 7:42 ` Andy Shevchenko 0 siblings, 2 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 21:18 UTC (permalink / raw) To: Andy Shevchenko Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Danilo Krummrich On Thu, 19 Feb 2026 20:27:51 +0100, Andy Shevchenko <andriy.shevchenko@linux.intel.com> said: > On Thu, Feb 19, 2026 at 05:55:20PM +0100, Danilo Krummrich wrote: >> On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: > > ... > >> Also, is there a reason why we need both device_match_fwnode() *and* >> device_match_fwnode_ext()? > > Yes. We don't want (at least for now) to dive into bug hunting in a 2+ years > horizon if something goes wrong with [currently] working drivers that use > device_match_fwnode() against the cases when there are primary and secondary > fwnodes present. > > I won't put my bet that extending device_match_fwnode() won't break anything. > And I don't want to invest (waste?) my time to learn each of the existing cases. > > The proposed way is robust and safest. And for the record, I will be the first > person to push back device_match_fwnode() upgrade without a comprehensive testing > on real (affected) HW. > Who's got the final word here? I responded to Danilo's email saying I can fold the new code into the existing function but you are against it. Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 21:18 ` Bartosz Golaszewski @ 2026-02-20 0:21 ` Danilo Krummrich 2026-02-20 7:42 ` Andy Shevchenko 1 sibling, 0 replies; 29+ messages in thread From: Danilo Krummrich @ 2026-02-20 0:21 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Andy Shevchenko, Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio On Thu Feb 19, 2026 at 10:18 PM CET, Bartosz Golaszewski wrote: > On Thu, 19 Feb 2026 20:27:51 +0100, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> said: >> On Thu, Feb 19, 2026 at 05:55:20PM +0100, Danilo Krummrich wrote: >>> On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: >> >> ... >> >>> Also, is there a reason why we need both device_match_fwnode() *and* >>> device_match_fwnode_ext()? >> >> Yes. We don't want (at least for now) to dive into bug hunting in a 2+ years >> horizon if something goes wrong with [currently] working drivers that use >> device_match_fwnode() against the cases when there are primary and secondary >> fwnodes present. >> >> I won't put my bet that extending device_match_fwnode() won't break anything. >> And I don't want to invest (waste?) my time to learn each of the existing cases. >> >> The proposed way is robust and safest. And for the record, I will be the first >> person to push back device_match_fwnode() upgrade without a comprehensive testing >> on real (affected) HW. >> > > Who's got the final word here? I responded to Danilo's email saying I can fold > the new code into the existing function but you are against it. I asked this question because it does not seem unlikely it could just work for all drivers. (Reading other threads, it seems like I'm not the only one asking this.) If someone is up to investigate, why not? If not, that's fine too, then we take the safe path. In case of the latter, I suggest to rename it to device_match_fwnode_full(). Thanks, Danilo ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 21:18 ` Bartosz Golaszewski 2026-02-20 0:21 ` Danilo Krummrich @ 2026-02-20 7:42 ` Andy Shevchenko 2026-02-20 11:21 ` Bartosz Golaszewski 1 sibling, 1 reply; 29+ messages in thread From: Andy Shevchenko @ 2026-02-20 7:42 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Danilo Krummrich On Thu, Feb 19, 2026 at 03:18:24PM -0600, Bartosz Golaszewski wrote: > On Thu, 19 Feb 2026 20:27:51 +0100, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> said: > > On Thu, Feb 19, 2026 at 05:55:20PM +0100, Danilo Krummrich wrote: > >> On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: ... > >> Also, is there a reason why we need both device_match_fwnode() *and* > >> device_match_fwnode_ext()? > > > > Yes. We don't want (at least for now) to dive into bug hunting in a 2+ years > > horizon if something goes wrong with [currently] working drivers that use > > device_match_fwnode() against the cases when there are primary and secondary > > fwnodes present. > > > > I won't put my bet that extending device_match_fwnode() won't break anything. > > And I don't want to invest (waste?) my time to learn each of the existing cases. > > > > The proposed way is robust and safest. And for the record, I will be the first > > person to push back device_match_fwnode() upgrade without a comprehensive testing > > on real (affected) HW. > > Who's got the final word here? I responded to Danilo's email saying I can fold > the new code into the existing function but you are against it. Of course I am not a maintainer, but as I said, I will be not okay without proven tests on the real HW. It's non-trivial change as it may lead to a problematic behaviour that one may not observe immediately (it might affect 1 out of 100s platforms). So, it will be hidden till unknown point in time in the future. I prefer safest way. And then we can convert case-by-case without hurry, which is the usual cause of the subtle bugs. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 7:42 ` Andy Shevchenko @ 2026-02-20 11:21 ` Bartosz Golaszewski 0 siblings, 0 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-20 11:21 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Danilo Krummrich, Bartosz Golaszewski On Fri, 20 Feb 2026 08:42:00 +0100, Andy Shevchenko <andriy.shevchenko@linux.intel.com> said: > On Thu, Feb 19, 2026 at 03:18:24PM -0600, Bartosz Golaszewski wrote: >> On Thu, 19 Feb 2026 20:27:51 +0100, Andy Shevchenko >> <andriy.shevchenko@linux.intel.com> said: >> > On Thu, Feb 19, 2026 at 05:55:20PM +0100, Danilo Krummrich wrote: >> >> On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: > > ... > >> >> Also, is there a reason why we need both device_match_fwnode() *and* >> >> device_match_fwnode_ext()? >> > >> > Yes. We don't want (at least for now) to dive into bug hunting in a 2+ years >> > horizon if something goes wrong with [currently] working drivers that use >> > device_match_fwnode() against the cases when there are primary and secondary >> > fwnodes present. >> > >> > I won't put my bet that extending device_match_fwnode() won't break anything. >> > And I don't want to invest (waste?) my time to learn each of the existing cases. >> > >> > The proposed way is robust and safest. And for the record, I will be the first >> > person to push back device_match_fwnode() upgrade without a comprehensive testing >> > on real (affected) HW. >> >> Who's got the final word here? I responded to Danilo's email saying I can fold >> the new code into the existing function but you are against it. > > Of course I am not a maintainer, but as I said, I will be not okay without > proven tests on the real HW. It's non-trivial change as it may lead to > a problematic behaviour that one may not observe immediately (it might affect > 1 out of 100s platforms). So, it will be hidden till unknown point in time > in the future. > > I prefer safest way. And then we can convert case-by-case without hurry, which > is the usual cause of the subtle bugs. > I agree, given how many problem some GPIO patches cause on x86 platforms months after getting upstream. Bartosz ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:55 ` Danilo Krummrich 2026-02-19 19:27 ` Andy Shevchenko @ 2026-02-19 21:16 ` Bartosz Golaszewski 1 sibling, 0 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 21:16 UTC (permalink / raw) To: Danilo Krummrich Cc: Greg Kroah-Hartman, Bartosz Golaszewski, Rafael J. Wysocki, Linus Walleij, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Thu, 19 Feb 2026 17:55:20 +0100, Danilo Krummrich <dakr@kernel.org> said: > On Thu Feb 19, 2026 at 5:39 PM CET, Bartosz Golaszewski wrote: >> On Thu, Feb 19, 2026 at 5:36 PM Greg Kroah-Hartman >> <gregkh@linuxfoundation.org> wrote: >>> >>> On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >>> > Provide an extended variant of device_match_fwnode() that also tries to >>> > match the device's secondary fwnode. >>> > >>> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> >>> > --- >>> > drivers/base/core.c | 14 ++++++++++++++ >>> > include/linux/device/bus.h | 1 + >>> > 2 files changed, 15 insertions(+) >>> > >>> > diff --git a/drivers/base/core.c b/drivers/base/core.c >>> > index f599a1384eec90c104601422b04dc2b4c19d4382..bbf1337978fafc35eb94bda85e0bb7f6879879c0 100644 >>> > --- a/drivers/base/core.c >>> > +++ b/drivers/base/core.c >>> > @@ -5326,6 +5326,20 @@ int device_match_fwnode(struct device *dev, const void *fwnode) >>> > } >>> > EXPORT_SYMBOL_GPL(device_match_fwnode); >>> > >>> > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >>> >>> No kernel doc to explain what this function does? >>> >>> :( >>> >> >> It's not like any of the other variants from this file were documented >> but ok, I can add it in v2. Still, I'd like to hear if this even makes >> sense. > > I'd argue that the other ones are very obvious, as they just encapsulate a > single operation rather than any logic, whereas this one does have some logic. > > Also, is there a reason why we need both device_match_fwnode() *and* > device_match_fwnode_ext()? > No reason at all other than not wanting to have an impact wider than necessary. But in most case, this should be transparent for existing users to why not, let me go through them and see if anything can break. Bartosz ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:36 ` Greg Kroah-Hartman @ 2026-02-19 19:58 ` Andy Shevchenko 2026-02-19 21:21 ` Bartosz Golaszewski 1 sibling, 1 reply; 29+ messages in thread From: Andy Shevchenko @ 2026-02-19 19:58 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > Provide an extended variant of device_match_fwnode() that also tries to > match the device's secondary fwnode. ... > +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > +{ > + struct fwnode_handle *dev_node = dev_fwnode(dev); > + if (!fwnode) IS_ERR_OR_NULL() If supplied @fwnode is secondary, it might be an error pointer. > + return 0; > + if (dev_node == fwnode) > + return 1; > + > + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > +} I think we can refactor this. struct fwnode_handle *node; // I would name it like this, because in 3 cases in drivers/base/property.c // 2 with node and 1 with dev_node when the same API is called. if (IS_ERR(fwnode)) return 0; if (device_match_fwnode(dev, fwnode)) // NULL check is inside return 1; node = dev_fwnode(dev); return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > + if (!fwnode) > + return 0; > + if (dev_node == fwnode) > + return 1; > + > + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > +} ... > +int device_match_fwnode_ext(struct device *dev, const void *fwnode); Perhaps ext --> or_secondary ? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 19:58 ` Andy Shevchenko @ 2026-02-19 21:21 ` Bartosz Golaszewski 2026-02-20 7:36 ` Andy Shevchenko 0 siblings, 1 reply; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 21:21 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko <andriy.shevchenko@linux.intel.com> said: > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >> Provide an extended variant of device_match_fwnode() that also tries to >> match the device's secondary fwnode. > > ... > >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >> +{ >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > >> + if (!fwnode) > > IS_ERR_OR_NULL() > If supplied @fwnode is secondary, it might be an error pointer. > I mirrored existing device_match_fwnode(), should it be fixed too? >> + return 0; > >> + if (dev_node == fwnode) >> + return 1; >> + >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; >> +} > > I think we can refactor this. > > struct fwnode_handle *node; > > // I would name it like this, because in 3 cases in drivers/base/property.c > // 2 with node and 1 with dev_node when the same API is called. > Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > if (IS_ERR(fwnode)) > return 0; > > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > return 1; > Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve a reference to a secondary software node and try to lookup a GPIO through it, we'll end up with an IS_ERR() fwnode with existing code, right? > node = dev_fwnode(dev); > > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > > >> + if (!fwnode) >> + return 0; > >> + if (dev_node == fwnode) >> + return 1; >> + >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; >> +} > > > ... > >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > > Perhaps ext --> or_secondary ? > I thought about it but it would make it sound like it only matches the secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if we end up doing the linked list approach. > -- > With Best Regards, > Andy Shevchenko > > > Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-19 21:21 ` Bartosz Golaszewski @ 2026-02-20 7:36 ` Andy Shevchenko 2026-02-20 11:25 ` Bartosz Golaszewski 0 siblings, 1 reply; 29+ messages in thread From: Andy Shevchenko @ 2026-02-20 7:36 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: > On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> said: > > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > >> Provide an extended variant of device_match_fwnode() that also tries to > >> match the device's secondary fwnode. ... > >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > >> +{ > >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > > > >> + if (!fwnode) > > > > IS_ERR_OR_NULL() > > If supplied @fwnode is secondary, it might be an error pointer. > > I mirrored existing device_match_fwnode(), should it be fixed too? The answer is "I don't know". Strictly speaking this should be done everywhere in the generic code when we can't guarantee that fwnode that comes to the function is pure NULL or valid one. > >> + return 0; > > > >> + if (dev_node == fwnode) > >> + return 1; > >> + > >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > >> +} > > > > I think we can refactor this. > > > > struct fwnode_handle *node; > > > > // I would name it like this, because in 3 cases in drivers/base/property.c > > // 2 with node and 1 with dev_node when the same API is called. > > Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. But we need some consistency. drivers/base/property.c is inconsistent to begin with and here the code chose the least used one for unknown reasons to me. I'm fine with "node" that is inside the function. > > if (IS_ERR(fwnode)) > > return 0; > > > > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > > return 1; > > Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve > a reference to a secondary software node and try to lookup a GPIO through it, > we'll end up with an IS_ERR() fwnode with existing code, right? I'm not sure I understood the use case you are trying to describe here. The very first check guarantees that fwnode is either NULL or valid one. When it's a valid one, the comparison with error pointer will be false. What did I miss? > > node = dev_fwnode(dev); > > > > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > > > > > >> + if (!fwnode) > >> + return 0; > > > >> + if (dev_node == fwnode) > >> + return 1; > >> + > >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > >> +} ... > >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > > > > Perhaps ext --> or_secondary ? > > I thought about it but it would make it sound like it only matches the > secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if > we end up doing the linked list approach. Danilo proposed _full, but in my opinion it's not better than _ext unless you know very deep how fwnode structure is designed. Same with _all. It's confusing. fwnode_or_secondary (the key part is "or") sounds more precise. But if you come up with something else that makes less ambiguity I will be glad. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 7:36 ` Andy Shevchenko @ 2026-02-20 11:25 ` Bartosz Golaszewski 2026-02-20 12:08 ` Rafael J. Wysocki 0 siblings, 1 reply; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-20 11:25 UTC (permalink / raw) To: Andy Shevchenko Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski, Bartosz Golaszewski On Fri, 20 Feb 2026 08:36:58 +0100, Andy Shevchenko <andriy.shevchenko@linux.intel.com> said: > On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: >> On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko >> <andriy.shevchenko@linux.intel.com> said: >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: >> >> Provide an extended variant of device_match_fwnode() that also tries to >> >> match the device's secondary fwnode. > > ... > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) >> >> +{ >> >> + struct fwnode_handle *dev_node = dev_fwnode(dev); >> > >> >> + if (!fwnode) >> > >> > IS_ERR_OR_NULL() >> > If supplied @fwnode is secondary, it might be an error pointer. >> >> I mirrored existing device_match_fwnode(), should it be fixed too? > > The answer is "I don't know". Strictly speaking this should be done everywhere > in the generic code when we can't guarantee that fwnode that comes to the > function is pure NULL or valid one. > >> >> + return 0; >> > >> >> + if (dev_node == fwnode) >> >> + return 1; >> >> + >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; >> >> +} >> > >> > I think we can refactor this. >> > >> > struct fwnode_handle *node; >> > >> > // I would name it like this, because in 3 cases in drivers/base/property.c >> > // 2 with node and 1 with dev_node when the same API is called. >> >> Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > > But we need some consistency. drivers/base/property.c is inconsistent to begin > with and here the code chose the least used one for unknown reasons to me. > > I'm fine with "node" that is inside the function. > >> > if (IS_ERR(fwnode)) >> > return 0; >> > >> > if (device_match_fwnode(dev, fwnode)) // NULL check is inside >> > return 1; >> >> Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve >> a reference to a secondary software node and try to lookup a GPIO through it, >> we'll end up with an IS_ERR() fwnode with existing code, right? > > I'm not sure I understood the use case you are trying to describe here. > > The very first check guarantees that fwnode is either NULL or valid one. > When it's a valid one, the comparison with error pointer will be false. > What did I miss? > I mean: device_match_fwnode() has a NULL check but not an IS_ERR() check and can be passed a secondary fwnode as argument and that can be -ENODEV. It will probably not fail terribly but is still incorrect. I was speaking about the existing implementation, not addressing your comments. >> > node = dev_fwnode(dev); >> > >> > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside >> > >> > >> >> + if (!fwnode) >> >> + return 0; >> > >> >> + if (dev_node == fwnode) >> >> + return 1; >> >> + >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; >> >> +} > > ... > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); >> > >> > Perhaps ext --> or_secondary ? >> >> I thought about it but it would make it sound like it only matches the >> secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if >> we end up doing the linked list approach. > > Danilo proposed _full, but in my opinion it's not better than _ext unless you > know very deep how fwnode structure is designed. Same with _all. It's confusing. > > fwnode_or_secondary (the key part is "or") sounds more precise. But if you come > up with something else that makes less ambiguity I will be glad. > device_match_fwnode_or_secondary() sounds good to me but shouldn't we try to limit the propagation of the "secondary" token in namespaces if our goal is to get rid of the whole "secondary fwnode" concept? Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 11:25 ` Bartosz Golaszewski @ 2026-02-20 12:08 ` Rafael J. Wysocki 2026-02-20 14:35 ` Bartosz Golaszewski 2026-02-20 15:43 ` Danilo Krummrich 0 siblings, 2 replies; 29+ messages in thread From: Rafael J. Wysocki @ 2026-02-20 12:08 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Andy Shevchenko, Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri, Feb 20, 2026 at 12:25 PM Bartosz Golaszewski <brgl@kernel.org> wrote: > > On Fri, 20 Feb 2026 08:36:58 +0100, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> said: > > On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: > >> On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko > >> <andriy.shevchenko@linux.intel.com> said: > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > >> >> Provide an extended variant of device_match_fwnode() that also tries to > >> >> match the device's secondary fwnode. > > > > ... > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > >> >> +{ > >> >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > >> > > >> >> + if (!fwnode) > >> > > >> > IS_ERR_OR_NULL() > >> > If supplied @fwnode is secondary, it might be an error pointer. > >> > >> I mirrored existing device_match_fwnode(), should it be fixed too? > > > > The answer is "I don't know". Strictly speaking this should be done everywhere > > in the generic code when we can't guarantee that fwnode that comes to the > > function is pure NULL or valid one. > > > >> >> + return 0; > >> > > >> >> + if (dev_node == fwnode) > >> >> + return 1; > >> >> + > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > >> >> +} > >> > > >> > I think we can refactor this. > >> > > >> > struct fwnode_handle *node; > >> > > >> > // I would name it like this, because in 3 cases in drivers/base/property.c > >> > // 2 with node and 1 with dev_node when the same API is called. > >> > >> Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > > > > But we need some consistency. drivers/base/property.c is inconsistent to begin > > with and here the code chose the least used one for unknown reasons to me. > > > > I'm fine with "node" that is inside the function. > > > >> > if (IS_ERR(fwnode)) > >> > return 0; > >> > > >> > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > >> > return 1; > >> > >> Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve > >> a reference to a secondary software node and try to lookup a GPIO through it, > >> we'll end up with an IS_ERR() fwnode with existing code, right? > > > > I'm not sure I understood the use case you are trying to describe here. > > > > The very first check guarantees that fwnode is either NULL or valid one. > > When it's a valid one, the comparison with error pointer will be false. > > What did I miss? > > > > I mean: device_match_fwnode() has a NULL check but not an IS_ERR() check and > can be passed a secondary fwnode as argument and that can be -ENODEV. It will > probably not fail terribly but is still incorrect. > > I was speaking about the existing implementation, not addressing your comments. > > >> > node = dev_fwnode(dev); > >> > > >> > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > >> > > >> > > >> >> + if (!fwnode) > >> >> + return 0; > >> > > >> >> + if (dev_node == fwnode) > >> >> + return 1; > >> >> + > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > >> >> +} > > > > ... > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > >> > > >> > Perhaps ext --> or_secondary ? > >> > >> I thought about it but it would make it sound like it only matches the > >> secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if > >> we end up doing the linked list approach. > > > > Danilo proposed _full, but in my opinion it's not better than _ext unless you > > know very deep how fwnode structure is designed. Same with _all. It's confusing. > > > > fwnode_or_secondary (the key part is "or") sounds more precise. But if you come > > up with something else that makes less ambiguity I will be glad. > > > > device_match_fwnode_or_secondary() sounds good to me but shouldn't we try to > limit the propagation of the "secondary" token in namespaces if our goal is to > get rid of the whole "secondary fwnode" concept? I'm a bit late to this, but here's some background. Secondary fwnodes were not intended for device matching in the first place. The idea was to use them as a secondary supply of device properties that may be missing from the primary node (think of an ACPI device object accompanied by a software node supplying properties that cannot be derived from the former). They could be added by a driver after matching the primary node for the benefit of a generic framework. IMV, the example with children inheriting the fwnode from their parent where the user wants to add a different secondary node to each child doesn't really match the picture described above. At least it was not anticipated. The idea was to allow the parent's fwnode to be extended and then (possibly) inherited. That's why the secondary fwnode pointer is there under the primary one. So all of this goes beyond the original anticipated use of secondary fwnodes and it seems to be calling for a list of equivalent (not primary and secondary) fwnodes in struct device, but then of course there's the question about duplicate properties and whether or not the fwnode used for driver binding should be preferred (I don't see why not). Until all of this is resolved, I wouldn't even add a generic helper for matching secondary nodes. I'd just put this code directly into gpio_chip_match_by_fwnode() along with a big fat comment explaining why exactly secondary nodes need to be taken into account there. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 12:08 ` Rafael J. Wysocki @ 2026-02-20 14:35 ` Bartosz Golaszewski 2026-02-20 14:49 ` Andy Shevchenko 2026-02-20 14:55 ` Rafael J. Wysocki 2026-02-20 15:43 ` Danilo Krummrich 1 sibling, 2 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-20 14:35 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Andy Shevchenko, Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri, Feb 20, 2026 at 1:08 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Feb 20, 2026 at 12:25 PM Bartosz Golaszewski <brgl@kernel.org> wrote: > > > > On Fri, 20 Feb 2026 08:36:58 +0100, Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> said: > > > On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: > > >> On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko > > >> <andriy.shevchenko@linux.intel.com> said: > > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > >> >> Provide an extended variant of device_match_fwnode() that also tries to > > >> >> match the device's secondary fwnode. > > > > > > ... > > > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > >> >> +{ > > >> >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > > >> > > > >> >> + if (!fwnode) > > >> > > > >> > IS_ERR_OR_NULL() > > >> > If supplied @fwnode is secondary, it might be an error pointer. > > >> > > >> I mirrored existing device_match_fwnode(), should it be fixed too? > > > > > > The answer is "I don't know". Strictly speaking this should be done everywhere > > > in the generic code when we can't guarantee that fwnode that comes to the > > > function is pure NULL or valid one. > > > > > >> >> + return 0; > > >> > > > >> >> + if (dev_node == fwnode) > > >> >> + return 1; > > >> >> + > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > >> >> +} > > >> > > > >> > I think we can refactor this. > > >> > > > >> > struct fwnode_handle *node; > > >> > > > >> > // I would name it like this, because in 3 cases in drivers/base/property.c > > >> > // 2 with node and 1 with dev_node when the same API is called. > > >> > > >> Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > > > > > > But we need some consistency. drivers/base/property.c is inconsistent to begin > > > with and here the code chose the least used one for unknown reasons to me. > > > > > > I'm fine with "node" that is inside the function. > > > > > >> > if (IS_ERR(fwnode)) > > >> > return 0; > > >> > > > >> > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > > >> > return 1; > > >> > > >> Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve > > >> a reference to a secondary software node and try to lookup a GPIO through it, > > >> we'll end up with an IS_ERR() fwnode with existing code, right? > > > > > > I'm not sure I understood the use case you are trying to describe here. > > > > > > The very first check guarantees that fwnode is either NULL or valid one. > > > When it's a valid one, the comparison with error pointer will be false. > > > What did I miss? > > > > > > > I mean: device_match_fwnode() has a NULL check but not an IS_ERR() check and > > can be passed a secondary fwnode as argument and that can be -ENODEV. It will > > probably not fail terribly but is still incorrect. > > > > I was speaking about the existing implementation, not addressing your comments. > > > > >> > node = dev_fwnode(dev); > > >> > > > >> > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > > >> > > > >> > > > >> >> + if (!fwnode) > > >> >> + return 0; > > >> > > > >> >> + if (dev_node == fwnode) > > >> >> + return 1; > > >> >> + > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > >> >> +} > > > > > > ... > > > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > > >> > > > >> > Perhaps ext --> or_secondary ? > > >> > > >> I thought about it but it would make it sound like it only matches the > > >> secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if > > >> we end up doing the linked list approach. > > > > > > Danilo proposed _full, but in my opinion it's not better than _ext unless you > > > know very deep how fwnode structure is designed. Same with _all. It's confusing. > > > > > > fwnode_or_secondary (the key part is "or") sounds more precise. But if you come > > > up with something else that makes less ambiguity I will be glad. > > > > > > > device_match_fwnode_or_secondary() sounds good to me but shouldn't we try to > > limit the propagation of the "secondary" token in namespaces if our goal is to > > get rid of the whole "secondary fwnode" concept? > > I'm a bit late to this, but here's some background. > > Secondary fwnodes were not intended for device matching in the first > place. The idea was to use them as a secondary supply of device > properties that may be missing from the primary node (think of an ACPI > device object accompanied by a software node supplying properties that > cannot be derived from the former). They could be added by a driver > after matching the primary node for the benefit of a generic > framework. > > IMV, the example with children inheriting the fwnode from their parent > where the user wants to add a different secondary node to each child > doesn't really match the picture described above. At least it was not > anticipated. The idea was to allow the parent's fwnode to be extended > and then (possibly) inherited. > > That's why the secondary fwnode pointer is there under the primary one. > That's evolution in practice for you. :) It turned out software nodes are a good alternative for platform data in board files or MFD sub-devices and can serve as the primary firmware node of a device. That's alright - we have a common fwnode interface and it works fine. > So all of this goes beyond the original anticipated use of secondary > fwnodes and it seems to be calling for a list of equivalent (not > primary and secondary) fwnodes in struct device, but then of course > there's the question about duplicate properties and whether or not the > fwnode used for driver binding should be preferred (I don't see why > not). > I think that was Andy's initial proposal: treat the DT or ACPI node as the primary "main" node and any software node as an additional source of properties. In the absence of "real" nodes, I propose to treat the first software node as the primary. > Until all of this is resolved, I wouldn't even add a generic helper > for matching secondary nodes. I'd just put this code directly into > gpio_chip_match_by_fwnode() along with a big fat comment explaining > why exactly secondary nodes need to be taken into account there. > IMO that's a recipe for keeping it local forever but I don't mind it. I think I proposed to add the helper because Andy suggested doing it in driver core. Bart ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 14:35 ` Bartosz Golaszewski @ 2026-02-20 14:49 ` Andy Shevchenko 2026-02-20 14:55 ` Rafael J. Wysocki 1 sibling, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2026-02-20 14:49 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Rafael J. Wysocki, Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri, Feb 20, 2026 at 03:35:28PM +0100, Bartosz Golaszewski wrote: > On Fri, Feb 20, 2026 at 1:08 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Fri, Feb 20, 2026 at 12:25 PM Bartosz Golaszewski <brgl@kernel.org> wrote: > > > On Fri, 20 Feb 2026 08:36:58 +0100, Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> said: > > > > On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: > > > >> On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko > > > >> <andriy.shevchenko@linux.intel.com> said: > > > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > > >> >> Provide an extended variant of device_match_fwnode() that also tries to > > > >> >> match the device's secondary fwnode. ... > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > > >> >> +{ > > > >> >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > > > >> > > > > >> >> + if (!fwnode) > > > >> > > > > >> > IS_ERR_OR_NULL() > > > >> > If supplied @fwnode is secondary, it might be an error pointer. > > > >> > > > >> I mirrored existing device_match_fwnode(), should it be fixed too? > > > > > > > > The answer is "I don't know". Strictly speaking this should be done everywhere > > > > in the generic code when we can't guarantee that fwnode that comes to the > > > > function is pure NULL or valid one. > > > > > > > >> >> + return 0; > > > >> > > > > >> >> + if (dev_node == fwnode) > > > >> >> + return 1; > > > >> >> + > > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > > >> >> +} > > > >> > > > > >> > I think we can refactor this. > > > >> > > > > >> > struct fwnode_handle *node; > > > >> > > > > >> > // I would name it like this, because in 3 cases in drivers/base/property.c > > > >> > // 2 with node and 1 with dev_node when the same API is called. > > > >> > > > >> Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > > > > > > > > But we need some consistency. drivers/base/property.c is inconsistent to begin > > > > with and here the code chose the least used one for unknown reasons to me. > > > > > > > > I'm fine with "node" that is inside the function. > > > > > > > >> > if (IS_ERR(fwnode)) > > > >> > return 0; > > > >> > > > > >> > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > > > >> > return 1; > > > >> > > > >> Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve > > > >> a reference to a secondary software node and try to lookup a GPIO through it, > > > >> we'll end up with an IS_ERR() fwnode with existing code, right? > > > > > > > > I'm not sure I understood the use case you are trying to describe here. > > > > > > > > The very first check guarantees that fwnode is either NULL or valid one. > > > > When it's a valid one, the comparison with error pointer will be false. > > > > What did I miss? > > > > > > > > > > I mean: device_match_fwnode() has a NULL check but not an IS_ERR() check and > > > can be passed a secondary fwnode as argument and that can be -ENODEV. It will > > > probably not fail terribly but is still incorrect. > > > > > > I was speaking about the existing implementation, not addressing your comments. > > > > > > >> > node = dev_fwnode(dev); > > > >> > > > > >> > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > > > >> > > > > >> > > > > >> >> + if (!fwnode) > > > >> >> + return 0; > > > >> > > > > >> >> + if (dev_node == fwnode) > > > >> >> + return 1; > > > >> >> + > > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > > >> >> +} > > > > > > > > ... > > > > > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > > > >> > > > > >> > Perhaps ext --> or_secondary ? > > > >> > > > >> I thought about it but it would make it sound like it only matches the > > > >> secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if > > > >> we end up doing the linked list approach. > > > > > > > > Danilo proposed _full, but in my opinion it's not better than _ext unless you > > > > know very deep how fwnode structure is designed. Same with _all. It's confusing. > > > > > > > > fwnode_or_secondary (the key part is "or") sounds more precise. But if you come > > > > up with something else that makes less ambiguity I will be glad. > > > > > > > > > > device_match_fwnode_or_secondary() sounds good to me but shouldn't we try to > > > limit the propagation of the "secondary" token in namespaces if our goal is to > > > get rid of the whole "secondary fwnode" concept? > > > > I'm a bit late to this, but here's some background. > > > > Secondary fwnodes were not intended for device matching in the first > > place. The idea was to use them as a secondary supply of device > > properties that may be missing from the primary node (think of an ACPI > > device object accompanied by a software node supplying properties that > > cannot be derived from the former). They could be added by a driver > > after matching the primary node for the benefit of a generic > > framework. > > > > IMV, the example with children inheriting the fwnode from their parent > > where the user wants to add a different secondary node to each child > > doesn't really match the picture described above. At least it was not > > anticipated. The idea was to allow the parent's fwnode to be extended > > and then (possibly) inherited. > > > > That's why the secondary fwnode pointer is there under the primary one. > > > > That's evolution in practice for you. :) It turned out software nodes > are a good alternative for platform data in board files or MFD > sub-devices and can serve as the primary firmware node of a device. > That's alright - we have a common fwnode interface and it works fine. > > > So all of this goes beyond the original anticipated use of secondary > > fwnodes and it seems to be calling for a list of equivalent (not > > primary and secondary) fwnodes in struct device, but then of course > > there's the question about duplicate properties and whether or not the > > fwnode used for driver binding should be preferred (I don't see why > > not). > > I think that was Andy's initial proposal: treat the DT or ACPI node as > the primary "main" node and any software node as an additional source > of properties. In the absence of "real" nodes, I propose to treat the > first software node as the primary. > > > Until all of this is resolved, I wouldn't even add a generic helper > > for matching secondary nodes. I'd just put this code directly into > > gpio_chip_match_by_fwnode() along with a big fat comment explaining > > why exactly secondary nodes need to be taken into account there. > > > > IMO that's a recipe for keeping it local forever but I don't mind it. > I think I proposed to add the helper because Andy suggested doing it > in driver core. Hmm... Participating in the discussions for the implementation, yes, true. Don't remember suggesting that, but if you tell, I might have done t If it was the case, hat and forgot... Whatever, for now we have a compromise solution and since Rafael is the author and maintainer of device properties we may follow his suggestion. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 14:35 ` Bartosz Golaszewski 2026-02-20 14:49 ` Andy Shevchenko @ 2026-02-20 14:55 ` Rafael J. Wysocki 1 sibling, 0 replies; 29+ messages in thread From: Rafael J. Wysocki @ 2026-02-20 14:55 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Rafael J. Wysocki, Andy Shevchenko, Greg Kroah-Hartman, Danilo Krummrich, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri, Feb 20, 2026 at 3:35 PM Bartosz Golaszewski <brgl@kernel.org> wrote: > > On Fri, Feb 20, 2026 at 1:08 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Fri, Feb 20, 2026 at 12:25 PM Bartosz Golaszewski <brgl@kernel.org> wrote: > > > > > > On Fri, 20 Feb 2026 08:36:58 +0100, Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> said: > > > > On Thu, Feb 19, 2026 at 04:21:59PM -0500, Bartosz Golaszewski wrote: > > > >> On Thu, 19 Feb 2026 20:58:32 +0100, Andy Shevchenko > > > >> <andriy.shevchenko@linux.intel.com> said: > > > >> > On Thu, Feb 19, 2026 at 05:31:22PM +0100, Bartosz Golaszewski wrote: > > > >> >> Provide an extended variant of device_match_fwnode() that also tries to > > > >> >> match the device's secondary fwnode. > > > > > > > > ... > > > > > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode) > > > >> >> +{ > > > >> >> + struct fwnode_handle *dev_node = dev_fwnode(dev); > > > >> > > > > >> >> + if (!fwnode) > > > >> > > > > >> > IS_ERR_OR_NULL() > > > >> > If supplied @fwnode is secondary, it might be an error pointer. > > > >> > > > >> I mirrored existing device_match_fwnode(), should it be fixed too? > > > > > > > > The answer is "I don't know". Strictly speaking this should be done everywhere > > > > in the generic code when we can't guarantee that fwnode that comes to the > > > > function is pure NULL or valid one. > > > > > > > >> >> + return 0; > > > >> > > > > >> >> + if (dev_node == fwnode) > > > >> >> + return 1; > > > >> >> + > > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > > >> >> +} > > > >> > > > > >> > I think we can refactor this. > > > >> > > > > >> > struct fwnode_handle *node; > > > >> > > > > >> > // I would name it like this, because in 3 cases in drivers/base/property.c > > > >> > // 2 with node and 1 with dev_node when the same API is called. > > > >> > > > >> Haystack's node is "node" and the needle is "fwnode"? Seems confusing to me. > > > > > > > > But we need some consistency. drivers/base/property.c is inconsistent to begin > > > > with and here the code chose the least used one for unknown reasons to me. > > > > > > > > I'm fine with "node" that is inside the function. > > > > > > > >> > if (IS_ERR(fwnode)) > > > >> > return 0; > > > >> > > > > >> > if (device_match_fwnode(dev, fwnode)) // NULL check is inside > > > >> > return 1; > > > >> > > > >> Yeah, and it too can be supplied a secondary fwnode. Let's say we resolve > > > >> a reference to a secondary software node and try to lookup a GPIO through it, > > > >> we'll end up with an IS_ERR() fwnode with existing code, right? > > > > > > > > I'm not sure I understood the use case you are trying to describe here. > > > > > > > > The very first check guarantees that fwnode is either NULL or valid one. > > > > When it's a valid one, the comparison with error pointer will be false. > > > > What did I miss? > > > > > > > > > > I mean: device_match_fwnode() has a NULL check but not an IS_ERR() check and > > > can be passed a secondary fwnode as argument and that can be -ENODEV. It will > > > probably not fail terribly but is still incorrect. > > > > > > I was speaking about the existing implementation, not addressing your comments. > > > > > > >> > node = dev_fwnode(dev); > > > >> > > > > >> > return fwnode_is_primary(node) && node->secondary == fwnode; // NULL check is inside > > > >> > > > > >> > > > > >> >> + if (!fwnode) > > > >> >> + return 0; > > > >> > > > > >> >> + if (dev_node == fwnode) > > > >> >> + return 1; > > > >> >> + > > > >> >> + return fwnode_is_primary(dev_node) && dev_node->secondary == fwnode; > > > >> >> +} > > > > > > > > ... > > > > > > > >> >> +int device_match_fwnode_ext(struct device *dev, const void *fwnode); > > > >> > > > > >> > Perhaps ext --> or_secondary ? > > > >> > > > >> I thought about it but it would make it sound like it only matches the > > > >> secondary to me. Maybe device_match_all_fwnodes()? Would be future-proof if > > > >> we end up doing the linked list approach. > > > > > > > > Danilo proposed _full, but in my opinion it's not better than _ext unless you > > > > know very deep how fwnode structure is designed. Same with _all. It's confusing. > > > > > > > > fwnode_or_secondary (the key part is "or") sounds more precise. But if you come > > > > up with something else that makes less ambiguity I will be glad. > > > > > > > > > > device_match_fwnode_or_secondary() sounds good to me but shouldn't we try to > > > limit the propagation of the "secondary" token in namespaces if our goal is to > > > get rid of the whole "secondary fwnode" concept? > > > > I'm a bit late to this, but here's some background. > > > > Secondary fwnodes were not intended for device matching in the first > > place. The idea was to use them as a secondary supply of device > > properties that may be missing from the primary node (think of an ACPI > > device object accompanied by a software node supplying properties that > > cannot be derived from the former). They could be added by a driver > > after matching the primary node for the benefit of a generic > > framework. > > > > IMV, the example with children inheriting the fwnode from their parent > > where the user wants to add a different secondary node to each child > > doesn't really match the picture described above. At least it was not > > anticipated. The idea was to allow the parent's fwnode to be extended > > and then (possibly) inherited. > > > > That's why the secondary fwnode pointer is there under the primary one. > > > > That's evolution in practice for you. :) It turned out software nodes > are a good alternative for platform data in board files or MFD > sub-devices and can serve as the primary firmware node of a device. > That's alright - we have a common fwnode interface and it works fine. > > > So all of this goes beyond the original anticipated use of secondary > > fwnodes and it seems to be calling for a list of equivalent (not > > primary and secondary) fwnodes in struct device, but then of course > > there's the question about duplicate properties and whether or not the > > fwnode used for driver binding should be preferred (I don't see why > > not). > > > > I think that was Andy's initial proposal: treat the DT or ACPI node as > the primary "main" node and any software node as an additional source > of properties. In the absence of "real" nodes, I propose to treat the > first software node as the primary. IMV what really matters is which fwnode is used for picking up the driver because that's the one that holds the device identification matching the driver in the first place, so whatever properties come along with that identification should take precedence over properties supplied by the other fwnodes. Typically, that would be either a DT or an ACPI (device) node, but it need not be any of those in principle. It need not be the first one either. > > Until all of this is resolved, I wouldn't even add a generic helper > > for matching secondary nodes. I'd just put this code directly into > > gpio_chip_match_by_fwnode() along with a big fat comment explaining > > why exactly secondary nodes need to be taken into account there. > > > > IMO that's a recipe for keeping it local forever but I don't mind it. If there is a move toward replacing the fwnode pointer in struct device with a list of fwnodes, that code will need to be updated anyway and that would be the time to use a helper IMO. Otherwise, it may as well stay local. > I think I proposed to add the helper because Andy suggested doing it > in driver core. I see. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 12:08 ` Rafael J. Wysocki 2026-02-20 14:35 ` Bartosz Golaszewski @ 2026-02-20 15:43 ` Danilo Krummrich 2026-02-20 16:03 ` Rafael J. Wysocki 1 sibling, 1 reply; 29+ messages in thread From: Danilo Krummrich @ 2026-02-20 15:43 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Bartosz Golaszewski, Andy Shevchenko, Greg Kroah-Hartman, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri Feb 20, 2026 at 1:08 PM CET, Rafael J. Wysocki wrote: > Secondary fwnodes were not intended for device matching in the first > place. The idea was to use them as a secondary supply of device > properties that may be missing from the primary node (think of an ACPI > device object accompanied by a software node supplying properties that > cannot be derived from the former). They could be added by a driver > after matching the primary node for the benefit of a generic > framework. > > IMV, the example with children inheriting the fwnode from their parent > where the user wants to add a different secondary node to each child > doesn't really match the picture described above. At least it was not > anticipated. The idea was to allow the parent's fwnode to be extended > and then (possibly) inherited. > > That's why the secondary fwnode pointer is there under the primary one. > > So all of this goes beyond the original anticipated use of secondary > fwnodes and it seems to be calling for a list of equivalent (not > primary and secondary) fwnodes in struct device, but then of course > there's the question about duplicate properties and whether or not the > fwnode used for driver binding should be preferred (I don't see why > not). > > Until all of this is resolved, I wouldn't even add a generic helper > for matching secondary nodes. I'd just put this code directly into > gpio_chip_match_by_fwnode() along with a big fat comment explaining > why exactly secondary nodes need to be taken into account there. I'm aware that secondary software nodes were originally intended to supply missing properties only. However, I wonder whether the capability to match against them isn't actually a natural, unavoidable consequence of how software nodes can be used to bridge (broken) connections between providers and consumers. Or said differently, once PROPERTY_ENTRY_GPIO() - or more generically PROPERTY_ENTRY_REF() - was introduced, it exposed secondary nodes within the device graph, which subsystems have to consider. IIUC, this is also the reason why gpiod_fwnode_lookup() [1] (which is eventually called from gpiod_get()) has to consider secondary nodes. So, if I'm not mistaken with the above, it seems fair to provide such a helper. [1] https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/gpio/gpiolib.c#L4675 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] driver core: provide device_match_fwnode_ext() 2026-02-20 15:43 ` Danilo Krummrich @ 2026-02-20 16:03 ` Rafael J. Wysocki 0 siblings, 0 replies; 29+ messages in thread From: Rafael J. Wysocki @ 2026-02-20 16:03 UTC (permalink / raw) To: Danilo Krummrich Cc: Rafael J. Wysocki, Bartosz Golaszewski, Andy Shevchenko, Greg Kroah-Hartman, Linus Walleij, Dmitry Torokhov, driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski On Fri, Feb 20, 2026 at 4:43 PM Danilo Krummrich <dakr@kernel.org> wrote: > > On Fri Feb 20, 2026 at 1:08 PM CET, Rafael J. Wysocki wrote: > > Secondary fwnodes were not intended for device matching in the first > > place. The idea was to use them as a secondary supply of device > > properties that may be missing from the primary node (think of an ACPI > > device object accompanied by a software node supplying properties that > > cannot be derived from the former). They could be added by a driver > > after matching the primary node for the benefit of a generic > > framework. > > > > IMV, the example with children inheriting the fwnode from their parent > > where the user wants to add a different secondary node to each child > > doesn't really match the picture described above. At least it was not > > anticipated. The idea was to allow the parent's fwnode to be extended > > and then (possibly) inherited. > > > > That's why the secondary fwnode pointer is there under the primary one. > > > > So all of this goes beyond the original anticipated use of secondary > > fwnodes and it seems to be calling for a list of equivalent (not > > primary and secondary) fwnodes in struct device, but then of course > > there's the question about duplicate properties and whether or not the > > fwnode used for driver binding should be preferred (I don't see why > > not). > > > > Until all of this is resolved, I wouldn't even add a generic helper > > for matching secondary nodes. I'd just put this code directly into > > gpio_chip_match_by_fwnode() along with a big fat comment explaining > > why exactly secondary nodes need to be taken into account there. > > I'm aware that secondary software nodes were originally intended to supply > missing properties only. > > However, I wonder whether the capability to match against them isn't actually a > natural, unavoidable consequence of how software nodes can be used to bridge > (broken) connections between providers and consumers. > > Or said differently, once PROPERTY_ENTRY_GPIO() - or more generically > PROPERTY_ENTRY_REF() - was introduced, it exposed secondary nodes within the > device graph, which subsystems have to consider. > > IIUC, this is also the reason why gpiod_fwnode_lookup() [1] (which is eventually > called from gpiod_get()) has to consider secondary nodes. Well, in practice, is there any subsystem other than gpio that has to consider them? > So, if I'm not mistaken with the above, it seems fair to provide such a helper. In the absence of any, the helper doesn't seem necessary. > [1] https://elixir.bootlin.com/linux/v6.19-rc5/source/drivers/gpio/gpiolib.c#L4675 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/2] gpiolib: use device_match_fwnode_ext() 2026-02-19 16:31 [PATCH 0/2] driver core: provide and use device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski @ 2026-02-19 16:31 ` Bartosz Golaszewski 2026-02-19 17:32 ` [PATCH 0/2] driver core: provide and " Linus Walleij 2 siblings, 0 replies; 29+ messages in thread From: Bartosz Golaszewski @ 2026-02-19 16:31 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Linus Walleij, Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko Cc: driver-core, linux-kernel, linux-gpio, Bartosz Golaszewski Use the extended variant of device_match_fwnode() to also compare the secondary fwnode of the GPIO controller device during GPIO lookup. This is useful when a GPIO controller has a primary OF/ACPI node *and* a software node set up for devices created in board files. Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- drivers/gpio/gpiolib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index c52200eaaaff82b12f22dd1ee8459bdd8ec10d81..0430a04ea060c38b5823cb48dc7a439b73ba9b83 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1395,7 +1395,7 @@ 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); + return device_match_fwnode_ext(&gc->gpiodev->dev, fwnode); } /** -- 2.47.3 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 0/2] driver core: provide and use device_match_fwnode_ext() 2026-02-19 16:31 [PATCH 0/2] driver core: provide and use device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:31 ` [PATCH 2/2] gpiolib: use device_match_fwnode_ext() Bartosz Golaszewski @ 2026-02-19 17:32 ` Linus Walleij 2 siblings, 0 replies; 29+ messages in thread From: Linus Walleij @ 2026-02-19 17:32 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, Bartosz Golaszewski, Dmitry Torokhov, Andy Shevchenko, driver-core, linux-kernel, linux-gpio On Thu, Feb 19, 2026 at 5:31 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 > providing a new variant of device_match_fwnode() and using it in GPIO > core. > > Link: https://lore.kernel.org/all/aZUIFiOYt6GOlDQx@google.com/ > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> The concept (+/- code comments): Reviewed-by: Linus Walleij <linusw@kernel.org> Yours, Linus Walleij ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2026-02-20 16:04 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-02-19 16:31 [PATCH 0/2] driver core: provide and use device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:31 ` [PATCH 1/2] driver core: provide device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 16:36 ` Greg Kroah-Hartman 2026-02-19 16:39 ` Bartosz Golaszewski 2026-02-19 16:50 ` Greg Kroah-Hartman 2026-02-19 16:54 ` Dmitry Torokhov 2026-02-19 21:15 ` Bartosz Golaszewski 2026-02-20 0:47 ` Dmitry Torokhov 2026-02-20 7:27 ` Andy Shevchenko 2026-02-20 11:06 ` Bartosz Golaszewski 2026-02-19 16:55 ` Danilo Krummrich 2026-02-19 19:27 ` Andy Shevchenko 2026-02-19 21:18 ` Bartosz Golaszewski 2026-02-20 0:21 ` Danilo Krummrich 2026-02-20 7:42 ` Andy Shevchenko 2026-02-20 11:21 ` Bartosz Golaszewski 2026-02-19 21:16 ` Bartosz Golaszewski 2026-02-19 19:58 ` Andy Shevchenko 2026-02-19 21:21 ` Bartosz Golaszewski 2026-02-20 7:36 ` Andy Shevchenko 2026-02-20 11:25 ` Bartosz Golaszewski 2026-02-20 12:08 ` Rafael J. Wysocki 2026-02-20 14:35 ` Bartosz Golaszewski 2026-02-20 14:49 ` Andy Shevchenko 2026-02-20 14:55 ` Rafael J. Wysocki 2026-02-20 15:43 ` Danilo Krummrich 2026-02-20 16:03 ` Rafael J. Wysocki 2026-02-19 16:31 ` [PATCH 2/2] gpiolib: use device_match_fwnode_ext() Bartosz Golaszewski 2026-02-19 17:32 ` [PATCH 0/2] driver core: provide and " Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox