Re: [PATCH 1/9] software node: read the reference args via the fwnode API

Re: [PATCH 1/9] software node: read the reference args via the fwnode API

[Andy Shevchenko]

On Mon, Oct 06, 2025 at 03:00:16PM +0200, Bartosz Golaszewski wrote:
> 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.

This needs a comment in the code.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 0/9] reset: rework reset-gpios handling

[Bartosz Golaszewski]

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.

[1] https://lore.kernel.org/all/20250924-gpio-shared-v1-0-775e7efeb1a3@linaro.org/

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Bartosz Golaszewski (9):
      software node: read the reference args via the fwnode API
      software node: increase the reference of the swnode by its fwnode
      software node: allow referencing firmware nodes
      gpio: swnode: don't use the swnode's name as the key for GPIO lookup
      gpio: swnode: update the property definitions
      reset: order includes alphabetically in reset/core.c
      reset: make the provider of reset-gpios the parent of the reset device
      reset: gpio: convert the driver to using the auxiliary bus
      reset: gpio: use software nodes to setup the GPIO lookup

 drivers/base/swnode.c         |  28 +++++---
 drivers/gpio/gpiolib-swnode.c |  16 ++---
 drivers/reset/Kconfig         |   1 +
 drivers/reset/core.c          | 151 ++++++++++++++++++++++++------------------
 drivers/reset/reset-gpio.c    |  19 +++---
 include/linux/gpio/property.h |   5 +-
 include/linux/property.h      |  51 ++++++++++++--
 7 files changed, 174 insertions(+), 97 deletions(-)
---
base-commit: 097d5ce7a680da489516958e943910fa962e574a
change-id: 20250925-reset-gpios-swnodes-db553e67095b

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[PATCH 1/9] software node: read the reference args via the fwnode API

[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.

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..cc48cff54d9c3d4d257095b6cb4a7869bf657373 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_array(refnode, nargs_prop,
+						       &nargs_prop_val, 1);
 		if (error)
 			return error;
 

-- 
2.48.1

Re: [PATCH 1/9] software node: read the reference args via the fwnode API

[Andy Shevchenko]

On Mon, Oct 06, 2025 at 03:00:16PM +0200, Bartosz Golaszewski wrote:
> 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.

This needs a comment in the code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/9] software node: read the reference args via the fwnode API

[Bartosz Golaszewski]

On Sat, Oct 18, 2025 at 7:35 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Oct 06, 2025 at 03:00:16PM +0200, Bartosz Golaszewski wrote:
> > 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.
>
> This needs a comment in the code.
>

Honestly after a second glance, I disagree. Literally a few lines before we do:

refnode = software_node_fwnode(ref->node);

We know very well what refnode is here and why we should use fwnode
API. If anything, the previous use of direct property routines was
unusual. A comment would be redundant as the code is self-describing,
what do you even want me to write there?

Bartosz

Re: [PATCH 1/9] software node: read the reference args via the fwnode API

[Sakari Ailus]

Hi Bartosz,

On Wed, Oct 22, 2025 at 09:51:44AM +0200, Bartosz Golaszewski wrote:
> On Sat, Oct 18, 2025 at 7:35 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Mon, Oct 06, 2025 at 03:00:16PM +0200, Bartosz Golaszewski wrote:
> > > 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.
> >
> > This needs a comment in the code.
> >
> 
> Honestly after a second glance, I disagree. Literally a few lines before we do:
> 
> refnode = software_node_fwnode(ref->node);
> 
> We know very well what refnode is here and why we should use fwnode
> API. If anything, the previous use of direct property routines was
> unusual. A comment would be redundant as the code is self-describing,
> what do you even want me to write there?

Given that the only way the three implementations of fwnode have interacted
in the past has been via the secondary pointer (for software nodes) and
that this will continue to be an exception, I'd also add a comment. E.g.

	/* ref->node may be non-software node fwnode */

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 1/9] software node: read the reference args via the fwnode API

[Bartosz Golaszewski]

On Wed, Oct 22, 2025 at 10:24 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Bartosz,
>
> On Wed, Oct 22, 2025 at 09:51:44AM +0200, Bartosz Golaszewski wrote:
> > On Sat, Oct 18, 2025 at 7:35 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Mon, Oct 06, 2025 at 03:00:16PM +0200, Bartosz Golaszewski wrote:
> > > > 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.
> > >
> > > This needs a comment in the code.
> > >
> >
> > Honestly after a second glance, I disagree. Literally a few lines before we do:
> >
> > refnode = software_node_fwnode(ref->node);
> >
> > We know very well what refnode is here and why we should use fwnode
> > API. If anything, the previous use of direct property routines was
> > unusual. A comment would be redundant as the code is self-describing,
> > what do you even want me to write there?
>
> Given that the only way the three implementations of fwnode have interacted
> in the past has been via the secondary pointer (for software nodes) and
> that this will continue to be an exception, I'd also add a comment. E.g.
>
>         /* ref->node may be non-software node fwnode */
>

But this becomes very clear after patch 3/9 just from looking at the
code. Even after I removed the union, we still check for ref->swnode
and ref->fwnode and proceeded accordingly.

Let me send a v2 and please look at the resulting code after patch
3/9. Tell me if you still think it needs a comment.

Bart