* [PATCH v3 01/10] software node: read the reference args via the fwnode API
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-30 9:33 ` Andy Shevchenko
2025-10-29 12:28 ` [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
` (9 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Once we allow software nodes to reference all kinds of firmware nodes,
the refnode here will no longer necessarily be a software node so read
its proprties going through its fwnode implementation.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/base/swnode.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index be1e9e61a7bf4d1301a3e109628517cfd9214704..2994efaf1d5d74c82df70e7df8bddf61ba0bfd41 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -540,9 +540,8 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
return -ENOENT;
if (nargs_prop) {
- error = property_entry_read_int_array(ref->node->properties,
- nargs_prop, sizeof(u32),
- &nargs_prop_val, 1);
+ error = fwnode_property_read_u32(refnode, nargs_prop,
+ &nargs_prop_val);
if (error)
return error;
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 01/10] software node: read the reference args via the fwnode API
2025-10-29 12:28 ` [PATCH v3 01/10] software node: read the reference args via the fwnode API Bartosz Golaszewski
@ 2025-10-30 9:33 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-30 9:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Wed, Oct 29, 2025 at 01:28:35PM +0100, Bartosz Golaszewski wrote:
>
> Once we allow software nodes to reference all kinds of firmware nodes,
> the refnode here will no longer necessarily be a software node so read
> its proprties going through its fwnode implementation.
As pointed out in previous reviews this would have benefit of a short comment.
explaining the indirect call to read an array.
...
> if (nargs_prop) {
> - error = property_entry_read_int_array(ref->node->properties,
> - nargs_prop, sizeof(u32),
> - &nargs_prop_val, 1);
> + error = fwnode_property_read_u32(refnode, nargs_prop,
> + &nargs_prop_val);
It can be one line now. We are not fanatics of 80 limitation. :-)
> if (error)
> return error;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 01/10] software node: read the reference args via the fwnode API Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-30 9:34 ` Andy Shevchenko
2025-10-29 12:28 ` [PATCH v3 03/10] software node: allow referencing firmware nodes Bartosz Golaszewski
` (8 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Once we allow software nodes to reference other kinds of firmware nodes,
the node in args will no longer necessarily be a software node so bump
its reference count using its fwnode interface.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/base/swnode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 2994efaf1d5d74c82df70e7df8bddf61ba0bfd41..b7c3926b67be72671ba4e4c442b3acca80688cf7 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -554,7 +554,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
if (!args)
return 0;
- args->fwnode = software_node_get(refnode);
+ args->fwnode = fwnode_handle_get(refnode);
args->nargs = nargs;
for (i = 0; i < nargs; i++)
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-29 12:28 ` [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
@ 2025-10-30 9:34 ` Andy Shevchenko
2025-10-30 10:33 ` Bartosz Golaszewski
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-30 9:34 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Wed, Oct 29, 2025 at 01:28:36PM +0100, Bartosz Golaszewski wrote:
>
> Once we allow software nodes to reference other kinds of firmware nodes,
> the node in args will no longer necessarily be a software node so bump
> its reference count using its fwnode interface.
Same, a short comment (or an update of a kernel-doc if present, I don't
remember).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-30 9:34 ` Andy Shevchenko
@ 2025-10-30 10:33 ` Bartosz Golaszewski
2025-10-31 8:30 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-30 10:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski, Bartosz Golaszewski
On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> said:
> On Wed, Oct 29, 2025 at 01:28:36PM +0100, Bartosz Golaszewski wrote:
>>
>> Once we allow software nodes to reference other kinds of firmware nodes,
>> the node in args will no longer necessarily be a software node so bump
>> its reference count using its fwnode interface.
>
> Same, a short comment (or an update of a kernel-doc if present, I don't
> remember).
>
Andy: the resulting code after patch 3/10 looks like this:
struct fwnode_handle *refnode;
(...)
if (ref->swnode)
refnode = software_node_fwnode(ref->swnode);
else if (ref->fwnode)
refnode = ref->fwnode;
else
return -EINVAL;
if (!refnode)
return -ENOENT;
if (nargs_prop) {
error = fwnode_property_read_u32(refnode, nargs_prop,
&nargs_prop_val);
if (error)
return error;
nargs = nargs_prop_val;
}
(...)
args->fwnode = fwnode_handle_get(refnode);
I'm typically all for comments but this code really is self-commenting.
There's nothing ambiguous about the above. We know the refnode is an fwnode,
we assign it and we pass it to the fwnode_ routines. What exactly would you
add here that would make it clearer?
Bartosz
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-30 10:33 ` Bartosz Golaszewski
@ 2025-10-31 8:30 ` Andy Shevchenko
2025-10-31 9:03 ` Bartosz Golaszewski
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-31 8:30 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Thu, Oct 30, 2025 at 03:33:02AM -0700, Bartosz Golaszewski wrote:
> On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> said:
> > On Wed, Oct 29, 2025 at 01:28:36PM +0100, Bartosz Golaszewski wrote:
> >>
> >> Once we allow software nodes to reference other kinds of firmware nodes,
> >> the node in args will no longer necessarily be a software node so bump
> >> its reference count using its fwnode interface.
> >
> > Same, a short comment (or an update of a kernel-doc if present, I don't
> > remember).
> >
>
> Andy: the resulting code after patch 3/10 looks like this:
>
> struct fwnode_handle *refnode;
>
> (...)
Let's say something like below to be put here
/*
* The reference in software node may refer to a node of a different type.
* Depending on the type we choose either to use software node directly, or
* delegate that to fwnode API.
*/
> if (ref->swnode)
> refnode = software_node_fwnode(ref->swnode);
> else if (ref->fwnode)
> refnode = ref->fwnode;
> else
> return -EINVAL;
>
> if (!refnode)
> return -ENOENT;
>
> if (nargs_prop) {
> error = fwnode_property_read_u32(refnode, nargs_prop,
> &nargs_prop_val);
> if (error)
> return error;
>
> nargs = nargs_prop_val;
> }
>
> (...)
>
> args->fwnode = fwnode_handle_get(refnode);
>
> I'm typically all for comments but this code really is self-commenting.
> There's nothing ambiguous about the above. We know the refnode is an fwnode,
> we assign it and we pass it to the fwnode_ routines. What exactly would you
> add here that would make it clearer?
See above.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-31 8:30 ` Andy Shevchenko
@ 2025-10-31 9:03 ` Bartosz Golaszewski
2025-10-31 9:44 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-31 9:03 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Fri, Oct 31, 2025 at 9:30 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 30, 2025 at 03:33:02AM -0700, Bartosz Golaszewski wrote:
> > On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> said:
> > > On Wed, Oct 29, 2025 at 01:28:36PM +0100, Bartosz Golaszewski wrote:
> > >>
> > >> Once we allow software nodes to reference other kinds of firmware nodes,
> > >> the node in args will no longer necessarily be a software node so bump
> > >> its reference count using its fwnode interface.
> > >
> > > Same, a short comment (or an update of a kernel-doc if present, I don't
> > > remember).
> > >
> >
> > Andy: the resulting code after patch 3/10 looks like this:
> >
> > struct fwnode_handle *refnode;
> >
> > (...)
>
> Let's say something like below to be put here
>
> /*
> * The reference in software node may refer to a node of a different type.
> * Depending on the type we choose either to use software node directly, or
> * delegate that to fwnode API.
> */
>
But this is incorrect: we're not really doing that. We either use the
firmware node reference directly OR cast the software node to its
firmware node representation. We ALWAYS use the firmware node API
below.
This really *is* evident from the code but if it'll make you happy and
make you sign off on this, I'll add a corrected version.
IMO It's completely redundant.
Bartosz
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-31 9:03 ` Bartosz Golaszewski
@ 2025-10-31 9:44 ` Andy Shevchenko
2025-10-31 10:27 ` Bartosz Golaszewski
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-31 9:44 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Fri, Oct 31, 2025 at 10:03:47AM +0100, Bartosz Golaszewski wrote:
> On Fri, Oct 31, 2025 at 9:30 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Oct 30, 2025 at 03:33:02AM -0700, Bartosz Golaszewski wrote:
> > > On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> said:
...
> > > Andy: the resulting code after patch 3/10 looks like this:
> > >
> > > struct fwnode_handle *refnode;
> > >
> > > (...)
> >
> > Let's say something like below to be put here
> >
> > /*
> > * The reference in software node may refer to a node of a different type.
> > * Depending on the type we choose either to use software node directly, or
> > * delegate that to fwnode API.
> > */
>
> But this is incorrect: we're not really doing that. We either use the
> firmware node reference directly OR cast the software node to its
> firmware node representation. We ALWAYS use the firmware node API
> below.
>
> This really *is* evident from the code but if it'll make you happy and
> make you sign off on this, I'll add a corrected version.
The comment should answer to the Q: "Why the heck are we calling fwnode APIs here?"
> IMO It's completely redundant.
This is unusual case for swnode API (see other functions, they call directly
the low-level implementation instead of going to a round via fwnode). That's
why I insist on a comment of this piece. It may be obvious for you, but the
unprepared read would be surprised by this inconsistency.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-31 9:44 ` Andy Shevchenko
@ 2025-10-31 10:27 ` Bartosz Golaszewski
2025-10-31 12:31 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-31 10:27 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, Linus Walleij, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski, linux-gpio, linux-kernel, linux-acpi,
Bartosz Golaszewski
On Fri, 31 Oct 2025 10:44:46 +0100, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> said:
> On Fri, Oct 31, 2025 at 10:03:47AM +0100, Bartosz Golaszewski wrote:
>> On Fri, Oct 31, 2025 at 9:30 AM Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>> > On Thu, Oct 30, 2025 at 03:33:02AM -0700, Bartosz Golaszewski wrote:
>> > > On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
>> > > <andriy.shevchenko@linux.intel.com> said:
>
> ...
>
>> > > Andy: the resulting code after patch 3/10 looks like this:
>> > >
>> > > struct fwnode_handle *refnode;
>> > >
>> > > (...)
>> >
>> > Let's say something like below to be put here
>> >
>> > /*
>> > * The reference in software node may refer to a node of a different type.
>> > * Depending on the type we choose either to use software node directly, or
>> > * delegate that to fwnode API.
>> > */
>>
>> But this is incorrect: we're not really doing that. We either use the
>> firmware node reference directly OR cast the software node to its
>> firmware node representation. We ALWAYS use the firmware node API
>> below.
>>
>> This really *is* evident from the code but if it'll make you happy and
>> make you sign off on this, I'll add a corrected version.
>
> The comment should answer to the Q: "Why the heck are we calling fwnode APIs here?"
>
>> IMO It's completely redundant.
>
> This is unusual case for swnode API (see other functions, they call directly
> the low-level implementation instead of going to a round via fwnode). That's
> why I insist on a comment of this piece. It may be obvious for you, but the
> unprepared read would be surprised by this inconsistency.
>
I propose to have the following:
+ /*
+ * A software node can reference other software nodes or firmware
+ * nodes (which are the abstraction layer sitting on top of them).
+ * This is done to ensure we can create references to static software
+ * nodes before they're registered with the firmware node framework.
+ * At the time the reference is being resolved, we expect the swnodes
+ * in question to already have been registered and to be backed by
+ * a firmware node. This is why we use the fwnode API below to read the
+ * relevant properties and bump the reference count.
+ */
This at least adds relevant information on *why* we're using the fwnode API.
Bart
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode
2025-10-31 10:27 ` Bartosz Golaszewski
@ 2025-10-31 12:31 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-31 12:31 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Fri, Oct 31, 2025 at 05:27:10AM -0500, Bartosz Golaszewski wrote:
> On Fri, 31 Oct 2025 10:44:46 +0100, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> said:
> > On Fri, Oct 31, 2025 at 10:03:47AM +0100, Bartosz Golaszewski wrote:
> >> On Fri, Oct 31, 2025 at 9:30 AM Andy Shevchenko
> >> <andriy.shevchenko@linux.intel.com> wrote:
> >> > On Thu, Oct 30, 2025 at 03:33:02AM -0700, Bartosz Golaszewski wrote:
> >> > > On Thu, 30 Oct 2025 10:34:46 +0100, Andy Shevchenko
> >> > > <andriy.shevchenko@linux.intel.com> said:
...
> >> > > Andy: the resulting code after patch 3/10 looks like this:
> >> > >
> >> > > struct fwnode_handle *refnode;
> >> > >
> >> > > (...)
> >> >
> >> > Let's say something like below to be put here
> >> >
> >> > /*
> >> > * The reference in software node may refer to a node of a different type.
> >> > * Depending on the type we choose either to use software node directly, or
> >> > * delegate that to fwnode API.
> >> > */
> >>
> >> But this is incorrect: we're not really doing that. We either use the
> >> firmware node reference directly OR cast the software node to its
> >> firmware node representation. We ALWAYS use the firmware node API
> >> below.
> >>
> >> This really *is* evident from the code but if it'll make you happy and
> >> make you sign off on this, I'll add a corrected version.
> >
> > The comment should answer to the Q: "Why the heck are we calling fwnode APIs here?"
> >
> >> IMO It's completely redundant.
> >
> > This is unusual case for swnode API (see other functions, they call directly
> > the low-level implementation instead of going to a round via fwnode). That's
> > why I insist on a comment of this piece. It may be obvious for you, but the
> > unprepared read would be surprised by this inconsistency.
> >
>
> I propose to have the following:
>
> + /*
> + * A software node can reference other software nodes or firmware
> + * nodes (which are the abstraction layer sitting on top of them).
> + * This is done to ensure we can create references to static software
> + * nodes before they're registered with the firmware node framework.
> + * At the time the reference is being resolved, we expect the swnodes
> + * in question to already have been registered and to be backed by
> + * a firmware node. This is why we use the fwnode API below to read the
> + * relevant properties and bump the reference count.
> + */
>
> This at least adds relevant information on *why* we're using the fwnode API.
Yes, works for me, thanks!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 01/10] software node: read the reference args via the fwnode API Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 02/10] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:51 ` Philipp Zabel
2025-10-30 9:41 ` Andy Shevchenko
2025-10-29 12:28 ` [PATCH v3 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup Bartosz Golaszewski
` (7 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
At the moment software nodes can only reference other software nodes.
This is a limitation for devices created, for instance, on the auxiliary
bus with a dynamic software node attached which cannot reference devices
the firmware node of which is "real" (as an OF node or otherwise).
Make it possible for a software node to reference all firmware nodes in
addition to static software nodes. To that end: add a second pointer to
struct software_node_ref_args of type struct fwnode_handle. The core
swnode code will first check the swnode pointer and if it's NULL, it
will assume the fwnode pointer should be set. Rework the helper macros
and deprecate the existing ones whose names don't indicate the reference
type.
Software node graphs remain the same, as in: the remote endpoints still
have to be software nodes.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/base/swnode.c | 13 +++++++++++--
include/linux/property.h | 38 +++++++++++++++++++++++++++++++-------
2 files changed, 42 insertions(+), 9 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index b7c3926b67be72671ba4e4c442b3acca80688cf7..8601d1612be31febb6abbbe1fb35228499480c56 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -535,7 +535,13 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
ref_array = prop->pointer;
ref = &ref_array[index];
- refnode = software_node_fwnode(ref->node);
+ if (ref->swnode)
+ refnode = software_node_fwnode(ref->swnode);
+ else if (ref->fwnode)
+ refnode = ref->fwnode;
+ else
+ return -EINVAL;
+
if (!refnode)
return -ENOENT;
@@ -634,7 +640,10 @@ software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
ref = prop->pointer;
- return software_node_get(software_node_fwnode(ref[0].node));
+ if (!ref->swnode)
+ return NULL;
+
+ return software_node_get(software_node_fwnode(ref[0].swnode));
}
static struct fwnode_handle *
diff --git a/include/linux/property.h b/include/linux/property.h
index 50b26589dd70d1756f3b8644255c24a011e2617c..66640b3a4cba21e65e562694691f18ecb2aeae18 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -355,23 +355,35 @@ struct software_node;
/**
* struct software_node_ref_args - Reference property with additional arguments
- * @node: Reference to a software node
+ * @swnode: Reference to a software node
+ * @fwnode: Alternative reference to a firmware node handle
* @nargs: Number of elements in @args array
* @args: Integer arguments
*/
struct software_node_ref_args {
- const struct software_node *node;
+ const struct software_node *swnode;
+ struct fwnode_handle *fwnode;
unsigned int nargs;
u64 args[NR_FWNODE_REFERENCE_ARGS];
};
-#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
+#define __SOFTWARE_NODE_REF(_ref, _node, ...) \
(const struct software_node_ref_args) { \
- .node = _ref_, \
+ ._node = _ref, \
.nargs = COUNT_ARGS(__VA_ARGS__), \
.args = { __VA_ARGS__ }, \
}
+#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
+ __SOFTWARE_NODE_REF(_ref, swnode, __VA_ARGS__)
+
+#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
+ __SOFTWARE_NODE_REF(_ref, fwnode, __VA_ARGS__)
+
+/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
+#define SOFTWARE_NODE_REFERENCE(_ref, ...) \
+ SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)
+
/**
* struct property_entry - "Built-in" device property representation.
* @name: Name of the property.
@@ -463,14 +475,26 @@ struct property_entry {
#define PROPERTY_ENTRY_STRING(_name_, _val_) \
__PROPERTY_ENTRY_ELEMENT(_name_, str, STRING, _val_)
-#define PROPERTY_ENTRY_REF(_name_, _ref_, ...) \
+#define __PROPERTY_ENTRY_REF(_type, _name, _ref, ...) \
(struct property_entry) { \
- .name = _name_, \
+ .name = _name, \
.length = sizeof(struct software_node_ref_args), \
.type = DEV_PROP_REF, \
- { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \
+ { .pointer = &_type(_ref, ##__VA_ARGS__), }, \
}
+#define PROPERTY_ENTRY_REF_SWNODE(_name, _ref, ...) \
+ __PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_SWNODE, \
+ _name, _ref, __VA_ARGS__)
+
+#define PROPERTY_ENTRY_REF_FWNODE(_name, _ref, ...) \
+ __PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_FWNODE, \
+ _name, _ref, __VA_ARGS__)
+
+/* DEPRECATED, use PROPERTY_ENTRY_REF_SWNODE() instead. */
+#define PROPERTY_ENTRY_REF(_name, _ref, ...) \
+ PROPERTY_ENTRY_REF_SWNODE(_name, _ref, __VA_ARGS__)
+
#define PROPERTY_ENTRY_BOOL(_name_) \
(struct property_entry) { \
.name = _name_, \
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-29 12:28 ` [PATCH v3 03/10] software node: allow referencing firmware nodes Bartosz Golaszewski
@ 2025-10-29 12:51 ` Philipp Zabel
2025-10-29 12:55 ` Bartosz Golaszewski
2025-10-30 9:41 ` Andy Shevchenko
1 sibling, 1 reply; 29+ messages in thread
From: Philipp Zabel @ 2025-10-29 12:51 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
On Mi, 2025-10-29 at 13:28 +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> At the moment software nodes can only reference other software nodes.
> This is a limitation for devices created, for instance, on the auxiliary
> bus with a dynamic software node attached which cannot reference devices
> the firmware node of which is "real" (as an OF node or otherwise).
>
> Make it possible for a software node to reference all firmware nodes in
> addition to static software nodes. To that end: add a second pointer to
> struct software_node_ref_args of type struct fwnode_handle. The core
> swnode code will first check the swnode pointer and if it's NULL, it
> will assume the fwnode pointer should be set. Rework the helper macros
> and deprecate the existing ones whose names don't indicate the reference
> type.
>
> Software node graphs remain the same, as in: the remote endpoints still
> have to be software nodes.
>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/base/swnode.c | 13 +++++++++++--
> include/linux/property.h | 38 +++++++++++++++++++++++++++++++-------
> 2 files changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index b7c3926b67be72671ba4e4c442b3acca80688cf7..8601d1612be31febb6abbbe1fb35228499480c56 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -535,7 +535,13 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
> ref_array = prop->pointer;
> ref = &ref_array[index];
>
> - refnode = software_node_fwnode(ref->node);
> + if (ref->swnode)
> + refnode = software_node_fwnode(ref->swnode);
software_node_fwnode(ref->swnode) never returns NULL if given a non-
NULL parameter.
> + else if (ref->fwnode)
> + refnode = ref->fwnode;
> + else
> + return -EINVAL;
> +
> if (!refnode)
> return -ENOENT;
So this check is not needed anymore.
regards
Philipp
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-29 12:51 ` Philipp Zabel
@ 2025-10-29 12:55 ` Bartosz Golaszewski
2025-10-29 13:16 ` Philipp Zabel
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:55 UTC (permalink / raw)
To: Philipp Zabel
Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Wed, Oct 29, 2025 at 1:51 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> > @@ -535,7 +535,13 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
> > ref_array = prop->pointer;
> > ref = &ref_array[index];
> >
> > - refnode = software_node_fwnode(ref->node);
> > + if (ref->swnode)
> > + refnode = software_node_fwnode(ref->swnode);
>
> software_node_fwnode(ref->swnode) never returns NULL if given a non-
> NULL parameter.
>
That's not true, it *will* return NULL if the software node in
question has not yet been registered with the fwnode framework.
Bart
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-29 12:55 ` Bartosz Golaszewski
@ 2025-10-29 13:16 ` Philipp Zabel
0 siblings, 0 replies; 29+ messages in thread
From: Philipp Zabel @ 2025-10-29 13:16 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Mi, 2025-10-29 at 13:55 +0100, Bartosz Golaszewski wrote:
> On Wed, Oct 29, 2025 at 1:51 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> >
> > > @@ -535,7 +535,13 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
> > > ref_array = prop->pointer;
> > > ref = &ref_array[index];
> > >
> > > - refnode = software_node_fwnode(ref->node);
> > > + if (ref->swnode)
> > > + refnode = software_node_fwnode(ref->swnode);
> >
> > software_node_fwnode(ref->swnode) never returns NULL if given a non-
> > NULL parameter.
> >
>
> That's not true, it *will* return NULL if the software node in
> question has not yet been registered with the fwnode framework.
I completely missed the list lookup in software_node_to_swnode().
Thank you for the quick correction and additional context.
regards
Philipp
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-29 12:28 ` [PATCH v3 03/10] software node: allow referencing firmware nodes Bartosz Golaszewski
2025-10-29 12:51 ` Philipp Zabel
@ 2025-10-30 9:41 ` Andy Shevchenko
2025-10-30 11:17 ` Bartosz Golaszewski
1 sibling, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-30 9:41 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Wed, Oct 29, 2025 at 01:28:37PM +0100, Bartosz Golaszewski wrote:
>
> At the moment software nodes can only reference other software nodes.
> This is a limitation for devices created, for instance, on the auxiliary
> bus with a dynamic software node attached which cannot reference devices
> the firmware node of which is "real" (as an OF node or otherwise).
>
> Make it possible for a software node to reference all firmware nodes in
> addition to static software nodes. To that end: add a second pointer to
> struct software_node_ref_args of type struct fwnode_handle. The core
> swnode code will first check the swnode pointer and if it's NULL, it
> will assume the fwnode pointer should be set. Rework the helper macros
> and deprecate the existing ones whose names don't indicate the reference
> type.
>
> Software node graphs remain the same, as in: the remote endpoints still
> have to be software nodes.
...
> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> +#define __SOFTWARE_NODE_REF(_ref, _node, ...) \
> (const struct software_node_ref_args) { \
> - .node = _ref_, \
> + ._node = _ref, \
> .nargs = COUNT_ARGS(__VA_ARGS__), \
> .args = { __VA_ARGS__ }, \
> }
Okay, looking at this again I think we don't need a new parameter.
We may check the type of _ref_
(actually why are the macro parameters got renamed here and elsewhere?)
and assign the correct one accordingly. I think this is what _Generic()
is good for.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-30 9:41 ` Andy Shevchenko
@ 2025-10-30 11:17 ` Bartosz Golaszewski
2025-10-31 8:23 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-30 11:17 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, Linus Walleij, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski, linux-gpio, linux-kernel, linux-acpi,
Bartosz Golaszewski
On Thu, 30 Oct 2025 10:41:39 +0100, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> said:
> On Wed, Oct 29, 2025 at 01:28:37PM +0100, Bartosz Golaszewski wrote:
>>
>> At the moment software nodes can only reference other software nodes.
>> This is a limitation for devices created, for instance, on the auxiliary
>> bus with a dynamic software node attached which cannot reference devices
>> the firmware node of which is "real" (as an OF node or otherwise).
>>
>> Make it possible for a software node to reference all firmware nodes in
>> addition to static software nodes. To that end: add a second pointer to
>> struct software_node_ref_args of type struct fwnode_handle. The core
>> swnode code will first check the swnode pointer and if it's NULL, it
>> will assume the fwnode pointer should be set. Rework the helper macros
>> and deprecate the existing ones whose names don't indicate the reference
>> type.
>>
>> Software node graphs remain the same, as in: the remote endpoints still
>> have to be software nodes.
>
> ...
>
>> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
>> +#define __SOFTWARE_NODE_REF(_ref, _node, ...) \
>> (const struct software_node_ref_args) { \
>> - .node = _ref_, \
>> + ._node = _ref, \
>> .nargs = COUNT_ARGS(__VA_ARGS__), \
>> .args = { __VA_ARGS__ }, \
>> }
>
> Okay, looking at this again I think we don't need a new parameter.
> We may check the type of _ref_
> (actually why are the macro parameters got renamed here and elsewhere?)
> and assign the correct one accordingly. I think this is what _Generic()
> is good for.
>
Oh, that's neat, I would love to use _Generic() here but I honest to god have
no idea how to make it work. I tried something like:
#define __SOFTWARE_NODE_REF(_ref, ...) \
_Generic(_ref, \
const struct software_node *: \
(const struct software_node_ref_args) { \
.swnode = _ref, \
.nargs = COUNT_ARGS(__VA_ARGS__), \
.args = { __VA_ARGS__ }, \
}, \
struct fwnode_handle *: \
(const struct software_node_ref_args) { \
.fwnode = _ref, \
.nargs = COUNT_ARGS(__VA_ARGS__), \
.args = { __VA_ARGS__ }, \
} \
)
But this fails like this:
In file included from ./include/linux/acpi.h:16,
from drivers/reset/core.c:8:
drivers/reset/core.c: In function ‘__reset_add_reset_gpio_device’:
drivers/reset/core.c:958:52: error: initialization of ‘const struct
software_node *’ from incompatible pointer type ‘struct fwnode_handle
*’ [-Wincompatible-pointer-types]
958 | parent->fwnode,
| ^~~~~~
./include/linux/property.h:374:35: note: in definition of macro
‘__SOFTWARE_NODE_REF’
374 | .swnode = _ref, \
So the right branch is not selected. How exactly would you use it here?
Bart
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-30 11:17 ` Bartosz Golaszewski
@ 2025-10-31 8:23 ` Andy Shevchenko
2025-10-31 9:00 ` Bartosz Golaszewski
0 siblings, 1 reply; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-31 8:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Thu, Oct 30, 2025 at 04:17:48AM -0700, Bartosz Golaszewski wrote:
> On Thu, 30 Oct 2025 10:41:39 +0100, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> said:
> > On Wed, Oct 29, 2025 at 01:28:37PM +0100, Bartosz Golaszewski wrote:
> >>
> >> At the moment software nodes can only reference other software nodes.
> >> This is a limitation for devices created, for instance, on the auxiliary
> >> bus with a dynamic software node attached which cannot reference devices
> >> the firmware node of which is "real" (as an OF node or otherwise).
> >>
> >> Make it possible for a software node to reference all firmware nodes in
> >> addition to static software nodes. To that end: add a second pointer to
> >> struct software_node_ref_args of type struct fwnode_handle. The core
> >> swnode code will first check the swnode pointer and if it's NULL, it
> >> will assume the fwnode pointer should be set. Rework the helper macros
> >> and deprecate the existing ones whose names don't indicate the reference
> >> type.
> >>
> >> Software node graphs remain the same, as in: the remote endpoints still
> >> have to be software nodes.
> >
> > ...
> >
> >> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> >> +#define __SOFTWARE_NODE_REF(_ref, _node, ...) \
> >> (const struct software_node_ref_args) { \
> >> - .node = _ref_, \
> >> + ._node = _ref, \
> >> .nargs = COUNT_ARGS(__VA_ARGS__), \
> >> .args = { __VA_ARGS__ }, \
> >> }
> >
> > Okay, looking at this again I think we don't need a new parameter.
> > We may check the type of _ref_
> > (actually why are the macro parameters got renamed here and elsewhere?)
> > and assign the correct one accordingly. I think this is what _Generic()
> > is good for.
> >
>
> Oh, that's neat, I would love to use _Generic() here but I honest to god have
> no idea how to make it work. I tried something like:
>
> #define __SOFTWARE_NODE_REF(_ref, ...) \
> _Generic(_ref, \
> const struct software_node *: \
> (const struct software_node_ref_args) { \
> .swnode = _ref, \
> .nargs = COUNT_ARGS(__VA_ARGS__), \
> .args = { __VA_ARGS__ }, \
> }, \
> struct fwnode_handle *: \
> (const struct software_node_ref_args) { \
> .fwnode = _ref, \
> .nargs = COUNT_ARGS(__VA_ARGS__), \
> .args = { __VA_ARGS__ }, \
> } \
> )
>
>
> But this fails like this:
>
> In file included from ./include/linux/acpi.h:16,
> from drivers/reset/core.c:8:
> drivers/reset/core.c: In function ‘__reset_add_reset_gpio_device’:
> drivers/reset/core.c:958:52: error: initialization of ‘const struct
> software_node *’ from incompatible pointer type ‘struct fwnode_handle
> *’ [-Wincompatible-pointer-types]
> 958 | parent->fwnode,
> | ^~~~~~
> ./include/linux/property.h:374:35: note: in definition of macro
> ‘__SOFTWARE_NODE_REF’
> 374 | .swnode = _ref, \
>
> So the right branch is not selected. How exactly would you use it here?
I believe this is an easy task.
But first of all, your series doesn't compile AFAICS:
drivers/reset/core.c:981:6: error: variable 'ret' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
981 | if (IS_ERR(rgpio_dev->swnode))
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/reset/core.c:1001:9: note: uninitialized use occurs here
1001 | return ret;
| ^~~
drivers/reset/core.c:981:2: note: remove the 'if' if its condition is always false
981 | if (IS_ERR(rgpio_dev->swnode))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
982 | goto err_put_of_node;
| ~~~~~~~~~~~~~~~~~~~~
drivers/reset/core.c:905:13: note: initialize the variable 'ret' to silence this warning
905 | int id, ret, lflags;
| ^
| = 0
1 error generated.
So, but to the topic
I have applied this and get the only error as per above
(const struct software_node_ref_args) { \
- ._node = _ref, \
+ .swnode = _Generic(_ref, const struct software_node *: _ref, default: NULL), \
+ .fwnode = _Generic(_ref, struct fwnode_handle *: _ref, default: NULL), \
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-31 8:23 ` Andy Shevchenko
@ 2025-10-31 9:00 ` Bartosz Golaszewski
2025-10-31 9:46 ` Andy Shevchenko
0 siblings, 1 reply; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-31 9:00 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Fri, Oct 31, 2025 at 9:24 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Oct 30, 2025 at 04:17:48AM -0700, Bartosz Golaszewski wrote:
> > On Thu, 30 Oct 2025 10:41:39 +0100, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> said:
> > > On Wed, Oct 29, 2025 at 01:28:37PM +0100, Bartosz Golaszewski wrote:
> > >>
> > >> At the moment software nodes can only reference other software nodes.
> > >> This is a limitation for devices created, for instance, on the auxiliary
> > >> bus with a dynamic software node attached which cannot reference devices
> > >> the firmware node of which is "real" (as an OF node or otherwise).
> > >>
> > >> Make it possible for a software node to reference all firmware nodes in
> > >> addition to static software nodes. To that end: add a second pointer to
> > >> struct software_node_ref_args of type struct fwnode_handle. The core
> > >> swnode code will first check the swnode pointer and if it's NULL, it
> > >> will assume the fwnode pointer should be set. Rework the helper macros
> > >> and deprecate the existing ones whose names don't indicate the reference
> > >> type.
> > >>
> > >> Software node graphs remain the same, as in: the remote endpoints still
> > >> have to be software nodes.
> > >
> > > ...
> > >
> > >> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> > >> +#define __SOFTWARE_NODE_REF(_ref, _node, ...) \
> > >> (const struct software_node_ref_args) { \
> > >> - .node = _ref_, \
> > >> + ._node = _ref, \
> > >> .nargs = COUNT_ARGS(__VA_ARGS__), \
> > >> .args = { __VA_ARGS__ }, \
> > >> }
> > >
> > > Okay, looking at this again I think we don't need a new parameter.
> > > We may check the type of _ref_
> > > (actually why are the macro parameters got renamed here and elsewhere?)
> > > and assign the correct one accordingly. I think this is what _Generic()
> > > is good for.
> > >
> >
> > Oh, that's neat, I would love to use _Generic() here but I honest to god have
> > no idea how to make it work. I tried something like:
> >
> > #define __SOFTWARE_NODE_REF(_ref, ...) \
> > _Generic(_ref, \
> > const struct software_node *: \
> > (const struct software_node_ref_args) { \
> > .swnode = _ref, \
> > .nargs = COUNT_ARGS(__VA_ARGS__), \
> > .args = { __VA_ARGS__ }, \
> > }, \
> > struct fwnode_handle *: \
> > (const struct software_node_ref_args) { \
> > .fwnode = _ref, \
> > .nargs = COUNT_ARGS(__VA_ARGS__), \
> > .args = { __VA_ARGS__ }, \
> > } \
> > )
> >
> >
> > But this fails like this:
> >
> > In file included from ./include/linux/acpi.h:16,
> > from drivers/reset/core.c:8:
> > drivers/reset/core.c: In function ‘__reset_add_reset_gpio_device’:
> > drivers/reset/core.c:958:52: error: initialization of ‘const struct
> > software_node *’ from incompatible pointer type ‘struct fwnode_handle
> > *’ [-Wincompatible-pointer-types]
> > 958 | parent->fwnode,
> > | ^~~~~~
> > ./include/linux/property.h:374:35: note: in definition of macro
> > ‘__SOFTWARE_NODE_REF’
> > 374 | .swnode = _ref, \
> >
> > So the right branch is not selected. How exactly would you use it here?
>
> I believe this is an easy task.
>
> But first of all, your series doesn't compile AFAICS:
>
> drivers/reset/core.c:981:6: error: variable 'ret' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> 981 | if (IS_ERR(rgpio_dev->swnode))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/reset/core.c:1001:9: note: uninitialized use occurs here
> 1001 | return ret;
> | ^~~
> drivers/reset/core.c:981:2: note: remove the 'if' if its condition is always false
> 981 | if (IS_ERR(rgpio_dev->swnode))
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 982 | goto err_put_of_node;
> | ~~~~~~~~~~~~~~~~~~~~
> drivers/reset/core.c:905:13: note: initialize the variable 'ret' to silence this warning
> 905 | int id, ret, lflags;
> | ^
> | = 0
> 1 error generated.
>
You're not wrong but for the record: it builds fine for me with
aarch64-linux-gnu-gcc 14.2 for some reason so I didn't notice it. I'll
fix it.
> So, but to the topic
>
> I have applied this and get the only error as per above
>
> (const struct software_node_ref_args) { \
> - ._node = _ref, \
> + .swnode = _Generic(_ref, const struct software_node *: _ref, default: NULL), \
> + .fwnode = _Generic(_ref, struct fwnode_handle *: _ref, default: NULL), \
>
That works, thanks for the idea.
Bartosz
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
2025-10-31 9:00 ` Bartosz Golaszewski
@ 2025-10-31 9:46 ` Andy Shevchenko
0 siblings, 0 replies; 29+ messages in thread
From: Andy Shevchenko @ 2025-10-31 9:46 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Philipp Zabel, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Fri, Oct 31, 2025 at 10:00:37AM +0100, Bartosz Golaszewski wrote:
> On Fri, Oct 31, 2025 at 9:24 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Oct 30, 2025 at 04:17:48AM -0700, Bartosz Golaszewski wrote:
...
> > But first of all, your series doesn't compile AFAICS:
> >
> > drivers/reset/core.c:981:6: error: variable 'ret' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> > 981 | if (IS_ERR(rgpio_dev->swnode))
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/reset/core.c:1001:9: note: uninitialized use occurs here
> > 1001 | return ret;
> > | ^~~
> > drivers/reset/core.c:981:2: note: remove the 'if' if its condition is always false
> > 981 | if (IS_ERR(rgpio_dev->swnode))
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 982 | goto err_put_of_node;
> > | ~~~~~~~~~~~~~~~~~~~~
> > drivers/reset/core.c:905:13: note: initialize the variable 'ret' to silence this warning
> > 905 | int id, ret, lflags;
> > | ^
> > | = 0
> > 1 error generated.
>
> You're not wrong but for the record: it builds fine for me with
> aarch64-linux-gnu-gcc 14.2 for some reason so I didn't notice it. I'll
> fix it.
GCC is not _the_ compiler nowadays. And building with `make W=1` should be a good
practice for subsystem maintainers :-)
...
> > So, but to the topic
> >
> > I have applied this and get the only error as per above
> >
> > (const struct software_node_ref_args) { \
> > - ._node = _ref, \
> > + .swnode = _Generic(_ref, const struct software_node *: _ref, default: NULL), \
> > + .fwnode = _Generic(_ref, struct fwnode_handle *: _ref, default: NULL), \
> >
>
> That works, thanks for the idea.
You're welcome!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v3 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (2 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 03/10] software node: allow referencing firmware nodes Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 05/10] gpio: swnode: allow referencing GPIO chips by firmware nodes Bartosz Golaszewski
` (6 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Looking up a GPIO controller by label that is the name of the software
node is wonky at best - the GPIO controller driver is free to set
a different label than the name of its firmware node. We're already being
passed a firmware node handle attached to the GPIO device to
swnode_get_gpio_device() so use it instead for a more precise lookup.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib-swnode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
!strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
return ERR_PTR(-ENOENT);
- gdev = gpio_device_find_by_label(gdev_node->name);
+ gdev = gpio_device_find_by_fwnode(fwnode);
return gdev ?: ERR_PTR(-EPROBE_DEFER);
}
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 05/10] gpio: swnode: allow referencing GPIO chips by firmware nodes
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (3 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 06/10] gpio: swnode: update the property definitions Bartosz Golaszewski
` (5 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
When doing a software node lookup, we require both the fwnode that
references a GPIO chip as well as the node associated with that chip to
be software nodes. However, we now allow referencing generic firmware
nodes from software nodes in driver core so we should allow the same in
GPIO core. Make the software node name check optional and dependent on
whether the referenced firmware node is a software node. If it's not,
just continue with the lookup.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib-swnode.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index e3806db1c0e077d76fcc71a50ca40bbf6872ca40..16af83fcc5aa886dd009dedc26b1ac23e5cbc4ea 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -30,16 +30,15 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
struct gpio_device *gdev;
gdev_node = to_software_node(fwnode);
- if (!gdev_node || !gdev_node->name)
- return ERR_PTR(-EINVAL);
-
- /*
- * Check for a special node that identifies undefined GPIOs, this is
- * primarily used as a key for internal chip selects in SPI bindings.
- */
- if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
- !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
- return ERR_PTR(-ENOENT);
+ if (gdev_node && gdev_node->name) {
+ /*
+ * Check for a special node that identifies undefined GPIOs, this is
+ * primarily used as a key for internal chip selects in SPI bindings.
+ */
+ if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
+ !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
+ return ERR_PTR(-ENOENT);
+ }
gdev = gpio_device_find_by_fwnode(fwnode);
return gdev ?: ERR_PTR(-EPROBE_DEFER);
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 06/10] gpio: swnode: update the property definitions
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (4 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 05/10] gpio: swnode: allow referencing GPIO chips by firmware nodes Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 07/10] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
` (4 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the recommended macros for creating references to software and
firmware nodes attached to GPIO providers.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
include/linux/gpio/property.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/gpio/property.h b/include/linux/gpio/property.h
index 0d220930800276a21b5ba96a68371ce66fc4ae3e..6b1c2ed9c57594bf3ead5edc82439f9fb7f514fd 100644
--- a/include/linux/gpio/property.h
+++ b/include/linux/gpio/property.h
@@ -7,7 +7,10 @@
struct software_node;
#define PROPERTY_ENTRY_GPIO(_name_, _chip_node_, _idx_, _flags_) \
- PROPERTY_ENTRY_REF(_name_, _chip_node_, _idx_, _flags_)
+ PROPERTY_ENTRY_REF_SWNODE(_name_, _chip_node_, _idx_, _flags_)
+
+#define PROPERTY_ENTRY_GPIO_FWNODE(_name_, _chip_node_, _idx_, _flags_) \
+ PROPERTY_ENTRY_REF_FWNODE(_name_, _chip_node_, _idx_, _flags_)
extern const struct software_node swnode_gpio_undefined;
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 07/10] reset: order includes alphabetically in reset/core.c
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (5 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 06/10] gpio: swnode: update the property definitions Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 08/10] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
` (3 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
For better readability and easier maintenance order the includes
alphabetically.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/reset/core.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 22f67fc77ae531c6efba3ce92cc73a2d57397762..5a696e2dbcc224a633e2b321da53b7bc699cb5f3 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -4,19 +4,20 @@
*
* Copyright 2013 Philipp Zabel, Pengutronix
*/
+
+#include <linux/acpi.h>
#include <linux/atomic.h>
#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/export.h>
-#include <linux/kernel.h>
-#include <linux/kref.h>
#include <linux/gpio/driver.h>
#include <linux/gpio/machine.h>
#include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/acpi.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/reset-controller.h>
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 08/10] reset: make the provider of reset-gpios the parent of the reset device
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (6 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 07/10] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 09/10] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
` (2 subsequent siblings)
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Auxiliary devices really do need a parent so ahead of converting the
reset-gpios driver to registering on the auxiliary bus, make the GPIO
device that provides the reset GPIO the parent of the reset-gpio device.
To that end move the lookup of the GPIO device by fwnode to the
beginning of __reset_add_reset_gpio_device() which has the added benefit
of bailing out earlier, before allocating resources for the virtual
device, if the chip is not up yet.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/reset/core.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 5a696e2dbcc224a633e2b321da53b7bc699cb5f3..13236ab69f10ec80e19b982be2bee5e4b0f99388 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -849,11 +849,11 @@ static void __reset_control_put_internal(struct reset_control *rstc)
kref_put(&rstc->refcnt, __reset_control_release);
}
-static int __reset_add_reset_gpio_lookup(int id, struct device_node *np,
+static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
+ struct device_node *np,
unsigned int gpio,
unsigned int of_flags)
{
- const struct fwnode_handle *fwnode = of_fwnode_handle(np);
unsigned int lookup_flags;
const char *label_tmp;
@@ -868,10 +868,6 @@ static int __reset_add_reset_gpio_lookup(int id, struct device_node *np,
return -EINVAL;
}
- struct gpio_device *gdev __free(gpio_device_put) = gpio_device_find_by_fwnode(fwnode);
- if (!gdev)
- return -EPROBE_DEFER;
-
label_tmp = gpio_device_get_label(gdev);
if (!label_tmp)
return -EINVAL;
@@ -926,6 +922,11 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
*/
lockdep_assert_not_held(&reset_list_mutex);
+ struct gpio_device *gdev __free(gpio_device_put) =
+ gpio_device_find_by_fwnode(of_fwnode_handle(args->np));
+ if (!gdev)
+ return -EPROBE_DEFER;
+
guard(mutex)(&reset_gpio_lookup_mutex);
list_for_each_entry(rgpio_dev, &reset_gpio_lookup_list, list) {
@@ -946,7 +947,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
goto err_ida_free;
}
- ret = __reset_add_reset_gpio_lookup(id, args->np, args->args[0],
+ ret = __reset_add_reset_gpio_lookup(gdev, id, args->np, args->args[0],
args->args[1]);
if (ret < 0)
goto err_kfree;
@@ -958,7 +959,8 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
* Hold reference as long as rgpio_dev memory is valid.
*/
of_node_get(rgpio_dev->of_args.np);
- pdev = platform_device_register_data(NULL, "reset-gpio", id,
+ pdev = platform_device_register_data(gpio_device_to_device(gdev),
+ "reset-gpio", id,
&rgpio_dev->of_args,
sizeof(rgpio_dev->of_args));
ret = PTR_ERR_OR_ZERO(pdev);
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 09/10] reset: gpio: convert the driver to using the auxiliary bus
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (7 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 08/10] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 12:28 ` [PATCH v3 10/10] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
2025-10-29 13:16 ` [PATCH v3 00/10] reset: rework reset-gpios handling Philipp Zabel
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
As the reset-gpio devices are purely virtual and never instantiated from
real firmware nodes, let's convert the driver to using the - more
fitting - auxiliary bus.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/reset/Kconfig | 1 +
drivers/reset/core.c | 14 ++++++--------
drivers/reset/reset-gpio.c | 19 ++++++++++---------
3 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 3102f5d7a93690f262722733e475b1215f61051c..24c9048cc7a31d3a6c9fb9af0726a8387bb3154a 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -89,6 +89,7 @@ config RESET_EYEQ
config RESET_GPIO
tristate "GPIO reset controller"
depends on GPIOLIB
+ select AUXILIARY_BUS
help
This enables a generic reset controller for resets attached via
GPIOs. Typically for OF platforms this driver expects "reset-gpios"
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 13236ab69f10ec80e19b982be2bee5e4b0f99388..e129c4c803eaa7e7e7122d96e9eff187f8dd826f 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -7,6 +7,7 @@
#include <linux/acpi.h>
#include <linux/atomic.h>
+#include <linux/auxiliary_bus.h>
#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/err.h>
@@ -18,7 +19,6 @@
#include <linux/kref.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/platform_device.h>
#include <linux/reset.h>
#include <linux/reset-controller.h>
#include <linux/slab.h>
@@ -882,7 +882,7 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
if (!lookup)
return -ENOMEM;
- lookup->dev_id = kasprintf(GFP_KERNEL, "reset-gpio.%d", id);
+ lookup->dev_id = kasprintf(GFP_KERNEL, "reset.gpio.%d", id);
if (!lookup->dev_id)
return -ENOMEM;
@@ -903,7 +903,7 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
{
struct reset_gpio_lookup *rgpio_dev;
- struct platform_device *pdev;
+ struct auxiliary_device *adev;
int id, ret;
/*
@@ -959,11 +959,9 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
* Hold reference as long as rgpio_dev memory is valid.
*/
of_node_get(rgpio_dev->of_args.np);
- pdev = platform_device_register_data(gpio_device_to_device(gdev),
- "reset-gpio", id,
- &rgpio_dev->of_args,
- sizeof(rgpio_dev->of_args));
- ret = PTR_ERR_OR_ZERO(pdev);
+ adev = auxiliary_device_create(gpio_device_to_device(gdev), "reset",
+ "gpio", &rgpio_dev->of_args, id);
+ ret = PTR_ERR_OR_ZERO(adev);
if (ret)
goto err_put;
diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
index 2290b25b6703536f2245f15cab870bd7092d3453..e5512b3b596b5290af20e5fdd99a38f81e670d2b 100644
--- a/drivers/reset/reset-gpio.c
+++ b/drivers/reset/reset-gpio.c
@@ -1,10 +1,10 @@
// SPDX-License-Identifier: GPL-2.0
+#include <linux/auxiliary_bus.h>
#include <linux/gpio/consumer.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/platform_device.h>
#include <linux/reset-controller.h>
struct reset_gpio_priv {
@@ -61,9 +61,10 @@ static void reset_gpio_of_node_put(void *data)
of_node_put(data);
}
-static int reset_gpio_probe(struct platform_device *pdev)
+static int reset_gpio_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
{
- struct device *dev = &pdev->dev;
+ struct device *dev = &adev->dev;
struct of_phandle_args *platdata = dev_get_platdata(dev);
struct reset_gpio_priv *priv;
int ret;
@@ -75,7 +76,7 @@ static int reset_gpio_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
- platform_set_drvdata(pdev, &priv->rc);
+ auxiliary_set_drvdata(adev, &priv->rc);
priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(priv->reset))
@@ -99,20 +100,20 @@ static int reset_gpio_probe(struct platform_device *pdev)
return devm_reset_controller_register(dev, &priv->rc);
}
-static const struct platform_device_id reset_gpio_ids[] = {
- { .name = "reset-gpio", },
+static const struct auxiliary_device_id reset_gpio_ids[] = {
+ { .name = "reset.gpio" },
{}
};
-MODULE_DEVICE_TABLE(platform, reset_gpio_ids);
+MODULE_DEVICE_TABLE(auxiliary, reset_gpio_ids);
-static struct platform_driver reset_gpio_driver = {
+static struct auxiliary_driver reset_gpio_driver = {
.probe = reset_gpio_probe,
.id_table = reset_gpio_ids,
.driver = {
.name = "reset-gpio",
},
};
-module_platform_driver(reset_gpio_driver);
+module_auxiliary_driver(reset_gpio_driver);
MODULE_AUTHOR("Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>");
MODULE_DESCRIPTION("Generic GPIO reset driver");
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH v3 10/10] reset: gpio: use software nodes to setup the GPIO lookup
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (8 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 09/10] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
@ 2025-10-29 12:28 ` Bartosz Golaszewski
2025-10-29 13:16 ` [PATCH v3 00/10] reset: rework reset-gpios handling Philipp Zabel
10 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 12:28 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
GPIO machine lookup is a nice mechanism for associating GPIOs with
consumers if we don't know what kind of device the GPIO provider is or
when it will become available. However in the case of the reset-gpio, we
are already holding a reference to the device and so can reference its
firmware node. Let's setup a software node that references the relevant
GPIO and attach it to the auxiliary device we're creating.
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/reset/core.c | 130 ++++++++++++++++++++++++++++++---------------------
1 file changed, 76 insertions(+), 54 deletions(-)
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index e129c4c803eaa7e7e7122d96e9eff187f8dd826f..4617bbac58314b1f37b937e7b0ffff745a81fcde 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -14,6 +14,7 @@
#include <linux/export.h>
#include <linux/gpio/driver.h>
#include <linux/gpio/machine.h>
+#include <linux/gpio/property.h>
#include <linux/idr.h>
#include <linux/kernel.h>
#include <linux/kref.h>
@@ -77,10 +78,12 @@ struct reset_control_array {
/**
* struct reset_gpio_lookup - lookup key for ad-hoc created reset-gpio devices
* @of_args: phandle to the reset controller with all the args like GPIO number
+ * @swnode: Software node containing the reference to the GPIO provider
* @list: list entry for the reset_gpio_lookup_list
*/
struct reset_gpio_lookup {
struct of_phandle_args of_args;
+ struct fwnode_handle *swnode;
struct list_head list;
};
@@ -849,52 +852,45 @@ static void __reset_control_put_internal(struct reset_control *rstc)
kref_put(&rstc->refcnt, __reset_control_release);
}
-static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
- struct device_node *np,
- unsigned int gpio,
- unsigned int of_flags)
+static void reset_gpio_aux_device_release(struct device *dev)
{
- unsigned int lookup_flags;
- const char *label_tmp;
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
- /*
- * Later we map GPIO flags between OF and Linux, however not all
- * constants from include/dt-bindings/gpio/gpio.h and
- * include/linux/gpio/machine.h match each other.
- */
- if (of_flags > GPIO_ACTIVE_LOW) {
- pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
- of_flags, gpio);
- return -EINVAL;
+ kfree(adev);
+}
+
+static int reset_add_gpio_aux_device(struct device *parent,
+ struct fwnode_handle *swnode,
+ int id, void *pdata)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return -ENOMEM;
+
+ adev->id = id;
+ adev->name = "gpio";
+ adev->dev.parent = parent;
+ adev->dev.platform_data = pdata;
+ adev->dev.release = reset_gpio_aux_device_release;
+ device_set_node(&adev->dev, swnode);
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ kfree(adev);
+ return ret;
}
- label_tmp = gpio_device_get_label(gdev);
- if (!label_tmp)
- return -EINVAL;
+ ret = __auxiliary_device_add(adev, "reset");
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ kfree(adev);
+ return ret;
+ }
- char *label __free(kfree) = kstrdup(label_tmp, GFP_KERNEL);
- if (!label)
- return -ENOMEM;
-
- /* Size: one lookup entry plus sentinel */
- struct gpiod_lookup_table *lookup __free(kfree) = kzalloc(struct_size(lookup, table, 2),
- GFP_KERNEL);
- if (!lookup)
- return -ENOMEM;
-
- lookup->dev_id = kasprintf(GFP_KERNEL, "reset.gpio.%d", id);
- if (!lookup->dev_id)
- return -ENOMEM;
-
- lookup_flags = GPIO_PERSISTENT;
- lookup_flags |= of_flags & GPIO_ACTIVE_LOW;
- lookup->table[0] = GPIO_LOOKUP(no_free_ptr(label), gpio, "reset",
- lookup_flags);
-
- /* Not freed on success, because it is persisent subsystem data. */
- gpiod_add_lookup_table(no_free_ptr(lookup));
-
- return 0;
+ return ret;
}
/*
@@ -902,9 +898,11 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
*/
static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
{
+ struct property_entry properties[] = { {}, {} };
struct reset_gpio_lookup *rgpio_dev;
- struct auxiliary_device *adev;
- int id, ret;
+ unsigned int offset, of_flags;
+ struct device *parent;
+ int id, ret, lflags;
/*
* Currently only #gpio-cells=2 is supported with the meaning of:
@@ -922,6 +920,23 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
*/
lockdep_assert_not_held(&reset_list_mutex);
+ offset = args->args[0];
+ of_flags = args->args[1];
+
+ /*
+ * Later we map GPIO flags between OF and Linux, however not all
+ * constants from include/dt-bindings/gpio/gpio.h and
+ * include/linux/gpio/machine.h match each other.
+ *
+ * FIXME: Find a better way of translating OF flags to GPIO lookup
+ * flags.
+ */
+ if (of_flags > GPIO_ACTIVE_LOW) {
+ pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
+ of_flags, offset);
+ return -EINVAL;
+ }
+
struct gpio_device *gdev __free(gpio_device_put) =
gpio_device_find_by_fwnode(of_fwnode_handle(args->np));
if (!gdev)
@@ -936,6 +951,13 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
}
}
+ lflags = GPIO_PERSISTENT | (of_flags & GPIO_ACTIVE_LOW);
+ parent = gpio_device_to_device(gdev);
+
+ properties[0] = PROPERTY_ENTRY_GPIO_FWNODE("reset-gpios",
+ parent->fwnode,
+ offset, lflags);
+
id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
if (id < 0)
return id;
@@ -947,11 +969,6 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
goto err_ida_free;
}
- ret = __reset_add_reset_gpio_lookup(gdev, id, args->np, args->args[0],
- args->args[1]);
- if (ret < 0)
- goto err_kfree;
-
rgpio_dev->of_args = *args;
/*
* We keep the device_node reference, but of_args.np is put at the end
@@ -959,19 +976,24 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
* Hold reference as long as rgpio_dev memory is valid.
*/
of_node_get(rgpio_dev->of_args.np);
- adev = auxiliary_device_create(gpio_device_to_device(gdev), "reset",
- "gpio", &rgpio_dev->of_args, id);
- ret = PTR_ERR_OR_ZERO(adev);
+
+ rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
+ if (IS_ERR(rgpio_dev->swnode))
+ goto err_put_of_node;
+
+ ret = reset_add_gpio_aux_device(parent, rgpio_dev->swnode, id,
+ &rgpio_dev->of_args);
if (ret)
- goto err_put;
+ goto err_del_swnode;
list_add(&rgpio_dev->list, &reset_gpio_lookup_list);
return 0;
-err_put:
+err_del_swnode:
+ fwnode_remove_software_node(rgpio_dev->swnode);
+err_put_of_node:
of_node_put(rgpio_dev->of_args.np);
-err_kfree:
kfree(rgpio_dev);
err_ida_free:
ida_free(&reset_gpio_ida, id);
--
2.48.1
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v3 00/10] reset: rework reset-gpios handling
2025-10-29 12:28 [PATCH v3 00/10] reset: rework reset-gpios handling Bartosz Golaszewski
` (9 preceding siblings ...)
2025-10-29 12:28 ` [PATCH v3 10/10] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
@ 2025-10-29 13:16 ` Philipp Zabel
2025-10-29 13:19 ` Bartosz Golaszewski
10 siblings, 1 reply; 29+ messages in thread
From: Philipp Zabel @ 2025-10-29 13:16 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
Rafael J. Wysocki, Danilo Krummrich, Krzysztof Kozlowski
Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski
On Mi, 2025-10-29 at 13:28 +0100, Bartosz Golaszewski wrote:
> Machine GPIO lookup is a nice, if a bit clunky, mechanism when we have
> absolutely no idea what the GPIO provider is or when it will be created.
> However in the case of reset-gpios, we not only know if the chip is
> there - we also already hold a reference to its firmware node.
>
> In this case using fwnode lookup makes more sense. However, since the
> reset provider is created dynamically, it doesn't have a corresponding
> firmware node (in this case: an OF-node). That leaves us with software
> nodes which currently cannot reference other implementations of the
> fwnode API, only other struct software_node objects. This is a needless
> limitation as it's imaginable that a dynamic auxiliary device (with a
> software node attached) would want to reference a real device with an OF
> node.
>
> This series does three things: extends the software node implementation,
> allowing its properties to reference not only static software nodes but
> also existing firmware nodes, updates the GPIO property interface to use
> the reworked swnode macros and finally makes the reset-gpio code the
> first user by converting the GPIO lookup from machine to swnode.
>
> Another user of the software node changes in the future could become the
> shared GPIO modules that's in the works in parallel[1].
>
> Merging strategy: the series is logically split into three parts: driver
> core, GPIO and reset respectively. However there are build-time
> dependencies between all three parts so I suggest the reset tree as the
> right one to take it upstream with an immutable branch provided to
> driver core and GPIO.
Should that branch include the reset changes, or only up to patch 6?
regards
Philipp
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v3 00/10] reset: rework reset-gpios handling
2025-10-29 13:16 ` [PATCH v3 00/10] reset: rework reset-gpios handling Philipp Zabel
@ 2025-10-29 13:19 ` Bartosz Golaszewski
0 siblings, 0 replies; 29+ messages in thread
From: Bartosz Golaszewski @ 2025-10-29 13:19 UTC (permalink / raw)
To: Philipp Zabel
Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
linux-acpi, Bartosz Golaszewski
On Wed, Oct 29, 2025 at 2:16 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Mi, 2025-10-29 at 13:28 +0100, Bartosz Golaszewski wrote:
> > Machine GPIO lookup is a nice, if a bit clunky, mechanism when we have
> > absolutely no idea what the GPIO provider is or when it will be created.
> > However in the case of reset-gpios, we not only know if the chip is
> > there - we also already hold a reference to its firmware node.
> >
> > In this case using fwnode lookup makes more sense. However, since the
> > reset provider is created dynamically, it doesn't have a corresponding
> > firmware node (in this case: an OF-node). That leaves us with software
> > nodes which currently cannot reference other implementations of the
> > fwnode API, only other struct software_node objects. This is a needless
> > limitation as it's imaginable that a dynamic auxiliary device (with a
> > software node attached) would want to reference a real device with an OF
> > node.
> >
> > This series does three things: extends the software node implementation,
> > allowing its properties to reference not only static software nodes but
> > also existing firmware nodes, updates the GPIO property interface to use
> > the reworked swnode macros and finally makes the reset-gpio code the
> > first user by converting the GPIO lookup from machine to swnode.
> >
> > Another user of the software node changes in the future could become the
> > shared GPIO modules that's in the works in parallel[1].
> >
> > Merging strategy: the series is logically split into three parts: driver
> > core, GPIO and reset respectively. However there are build-time
> > dependencies between all three parts so I suggest the reset tree as the
> > right one to take it upstream with an immutable branch provided to
> > driver core and GPIO.
>
> Should that branch include the reset changes, or only up to patch 6?
>
I was thinking about it containing the entire series, somewhat similar
to what Lee Jones does with MFD changes.
Bartosz
^ permalink raw reply [flat|nested] 29+ messages in thread