linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Daniel Scally <djrscally@gmail.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Danilo Krummrich <dakr@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: Re: [PATCH v5 3/8] software node: allow referencing firmware nodes
Date: Wed, 5 Nov 2025 22:54:31 +0200	[thread overview]
Message-ID: <aQu5hxGGrdPC7VOB@kekkonen.localdomain> (raw)
In-Reply-To: <20251105-reset-gpios-swnodes-v5-3-1f67499a8287@linaro.org>

Hi Bartosz,

Thanks for the update.

On Wed, Nov 05, 2025 at 09:47:34AM +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.
> 
> 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>

Reviewed-by: Sakari Ailus <sakari.ailus@linux.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

  parent reply	other threads:[~2025-11-06  7:28 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-05  8:47 [PATCH v5 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 1/8] software node: read the reference args via the fwnode API Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 2/8] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 3/8] software node: allow referencing firmware nodes Bartosz Golaszewski
2025-11-05 11:38   ` Andy Shevchenko
2025-11-05 20:54   ` Sakari Ailus [this message]
2025-11-05  8:47 ` [PATCH v5 4/8] gpio: swnode: allow referencing GPIO chips by " Bartosz Golaszewski
2025-11-05 13:58   ` Andy Shevchenko
2025-11-05  8:47 ` [PATCH v5 5/8] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 6/8] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 7/8] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
2025-11-05  8:47 ` [PATCH v5 8/8] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
2025-11-05 14:10   ` Andy Shevchenko
2025-11-06 14:18     ` Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aQu5hxGGrdPC7VOB@kekkonen.localdomain \
    --to=sakari.ailus@linux.intel.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=brgl@bgdev.pl \
    --cc=dakr@kernel.org \
    --cc=djrscally@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=krzk@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).