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 | 24 ++++++++++++++++++++++--
include/linux/property.h | 43 ++++++++++++++++++++++++++++++++++++-------
2 files changed, 58 insertions(+), 9 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 6b1ee75a908fbf272f29dbe65529ce69ce03a021..44710339255ffba1766f5984b2898a5fb4436557 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -535,7 +535,24 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
ref_array = prop->pointer;
ref = &ref_array[index];
- refnode = software_node_fwnode(ref->node);
+ /*
+ * 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.
+ */
+
+ if (ref->swnode)
+ refnode = software_node_fwnode(ref->swnode);
+ else if (ref->fwnode)
+ refnode = ref->fwnode;
+ else
+ return -EINVAL;
+
if (!refnode)
return -ENOENT;
@@ -633,7 +650,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..2d838117b7912b5aaff75318f9e7ad256039f2e7 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -355,23 +355,40 @@ 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, ...) \
(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), \
.nargs = COUNT_ARGS(__VA_ARGS__), \
.args = { __VA_ARGS__ }, \
}
+#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
+ __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
+
+#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
+ __SOFTWARE_NODE_REF(_ref, __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 +480,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.51.0On Mon, Nov 03, 2025 at 10:35:23AM +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.
...
> + /*
> + * 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
A nit-pick (since anyway it requires a new version): move 'the' to the next
line to make them more equal in the length.
> + * relevant properties and bump the reference count.
> + */
...
> -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> +#define __SOFTWARE_NODE_REF(_ref, ...) \
No, NAK. The renaming of the parameters is not related to this change _at all_.
Why do you change established style here? Did I miss your answer to my question
in the previous rounds?
> (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), \
> .nargs = COUNT_ARGS(__VA_ARGS__), \
> .args = { __VA_ARGS__ }, \
> }
...
> +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
> + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> +
> +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
> + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> +
> +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
> +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \
> + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)
Now, useless.
...
> -#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__), }, \
> }
Do we need this now? I assume that _Generic() takes case of this.
...
> +#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__)
Seems like useless churn.
--
With Best Regards,
Andy Shevchenko
On Mon, Nov 3, 2025 at 10:49 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Nov 03, 2025 at 10:35:23AM +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.
>
> ...
>
> > + /*
> > + * 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
>
> A nit-pick (since anyway it requires a new version): move 'the' to the next
> line to make them more equal in the length.
>
> > + * relevant properties and bump the reference count.
> > + */
>
> ...
>
> > -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> > +#define __SOFTWARE_NODE_REF(_ref, ...) \
>
> No, NAK. The renaming of the parameters is not related to this change _at all_.
> Why do you change established style here? Did I miss your answer to my question
> in the previous rounds?
>
Ah, my brain just filtered out the trailing '_'.
> > (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), \
> > .nargs = COUNT_ARGS(__VA_ARGS__), \
> > .args = { __VA_ARGS__ }, \
> > }
>
> ...
>
> > +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
> > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > +
> > +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
> > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > +
> > +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
> > +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \
> > + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)
>
> Now, useless.
>
No, why? With these changes, SOFTWARE_NODE_REFERENCE()'s name is a bit
misleading or incomplete, so I'm proposing to start replacing it with
SOFTWARE_NODE_REF_SWNODE() which is compatible with the former but has
a better name.
> ...
>
>
> > -#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__), }, \
> > }
>
> Do we need this now? I assume that _Generic() takes case of this.
>
Ah, right, it should be done here as well.
> ...
>
> > +#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__)
>
> Seems like useless churn.
>
This is the same argument as with SOFTWARE_NODE_REF_SWNODE(). It's not
clear from the name what PROPERTY_ENTRY_REF() is really referencing.
Bart
On Mon, Nov 03, 2025 at 11:36:36AM +0100, Bartosz Golaszewski wrote:
> On Mon, Nov 3, 2025 at 10:49 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Nov 03, 2025 at 10:35:23AM +0100, Bartosz Golaszewski wrote:
...
> > > +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
> > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > > +
> > > +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
> > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > > +
> > > +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
> > > +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \
> > > + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)
> >
> > Now, useless.
>
> No, why? With these changes, SOFTWARE_NODE_REFERENCE()'s name is a bit
> misleading or incomplete, so I'm proposing to start replacing it with
> SOFTWARE_NODE_REF_SWNODE() which is compatible with the former but has
> a better name.
It's an unneeded churn. I don't see a confusion here. One may interpret
That it is a reference in a software node to another node.
...
> > > -#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__), }, \
> > > }
> >
> > Do we need this now? I assume that _Generic() takes case of this.
> Ah, right, it should be done here as well.
Just it should work as is without changes, did I miss anything?
...
> > > +#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__)
> >
> > Seems like useless churn.
>
> This is the same argument as with SOFTWARE_NODE_REF_SWNODE(). It's not
> clear from the name what PROPERTY_ENTRY_REF() is really referencing.
Same answer as above.
...
TL;DR: Let's leave renaming / splitting to another series. It doesn't sound
like a required thingy. Only what I see is unneeded churn.
--
With Best Regards,
Andy Shevchenko
Hi Bartosz, Andy, On Mon, Nov 03, 2025 at 11:36:36AM +0100, Bartosz Golaszewski wrote: > On Mon, Nov 3, 2025 at 10:49 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \ > > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__) > > > + > > > +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \ > > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__) > > > + > > > +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */ > > > +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \ > > > + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__) > > > > Now, useless. > > > > No, why? With these changes, SOFTWARE_NODE_REFERENCE()'s name is a bit > misleading or incomplete, so I'm proposing to start replacing it with > SOFTWARE_NODE_REF_SWNODE() which is compatible with the former but has > a better name. Given we're already using _Generic() to determine the argument type, could we simply use e.g. SOFTWARE_NODE_REF() in both cases? -- Kind regards, Sakari Ailus
On Mon, Nov 3, 2025 at 11:52 AM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Bartosz, Andy, > > On Mon, Nov 03, 2025 at 11:36:36AM +0100, Bartosz Golaszewski wrote: > > On Mon, Nov 3, 2025 at 10:49 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \ > > > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__) > > > > + > > > > +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \ > > > > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__) > > > > + > > > > +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */ > > > > +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \ > > > > + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__) > > > > > > Now, useless. > > > > > > > No, why? With these changes, SOFTWARE_NODE_REFERENCE()'s name is a bit > > misleading or incomplete, so I'm proposing to start replacing it with > > SOFTWARE_NODE_REF_SWNODE() which is compatible with the former but has > > a better name. > > Given we're already using _Generic() to determine the argument type, could > we simply use e.g. SOFTWARE_NODE_REF() in both cases? > It may be possible, yes. I'll look into it. Bart
© 2016 - 2026 Red Hat, Inc.