[PATCH v3 03/10] software node: allow referencing firmware nodes

Bartosz Golaszewski posted 10 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Bartosz Golaszewski 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Andy Shevchenko 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Bartosz Golaszewski 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Andy Shevchenko 3 months, 1 week ago
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


Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Bartosz Golaszewski 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Andy Shevchenko 3 months, 1 week ago
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


Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Philipp Zabel 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Bartosz Golaszewski 3 months, 1 week ago
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
Re: [PATCH v3 03/10] software node: allow referencing firmware nodes
Posted by Philipp Zabel 3 months, 1 week ago
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