[v2] driver core: provide and use device_match_fwnode_ext()

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

Posted by Bartosz Golaszewski 1 month, 1 week ago

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

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

Posted by Andy Shevchenko 1 month, 1 week ago

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

Posted by Bartosz Golaszewski 1 month, 1 week ago

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

Posted by Andy Shevchenko 1 month, 1 week ago

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 1/2] driver core: make fwnode_is_primary() public

Posted by Rafael J. Wysocki 1 month, 1 week ago

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 1/2] driver core: make fwnode_is_primary() public

Posted by Andy Shevchenko 1 month, 1 week ago

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 1/2] driver core: make fwnode_is_primary() public

Posted by Rafael J. Wysocki 1 month, 1 week ago

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

Posted by Dmitry Torokhov 1 month, 1 week ago

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

Posted by Rafael J. Wysocki 1 month, 1 week ago

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