[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQu5hxGGrdPC7VOB@kekkonen.localdomain>
Date: Wed, 5 Nov 2025 22:54:31 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Krzysztof Kozlowski <krzk@...nel.org>, linux-gpio@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v5 3/8] software node: allow referencing firmware nodes
Hi Bartosz,
Thanks for the update.
On Wed, Nov 05, 2025 at 09:47:34AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.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.
>
> Software node graphs remain the same, as in: the remote endpoints still
> have to be software nodes.
>
> Acked-by: Linus Walleij <linus.walleij@...aro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Reviewed-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
But see below...
> ---
> drivers/base/swnode.c | 24 ++++++++++++++++++++++--
> include/linux/property.h | 13 ++++++++++---
> 2 files changed, 32 insertions(+), 5 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));
This could be:
return software_node_get(software_node_fwnode(ref->swnode));
> }
>
> static struct fwnode_handle *
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 50b26589dd70d1756f3b8644255c24a011e2617c..272bfbdea7bf4ab1143159fa49fc29dcdde0ef9d 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -355,19 +355,26 @@ 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_, ...) \
> (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__ }, \
> }
>
--
Kind regards,
Sakari Ailus
Powered by blists - more mailing lists