[PATCH v4 00/10] reset: rework reset-gpios handling

[PATCH v4 00/10] reset: rework reset-gpios handling

[Bartosz Golaszewski]

Software node maintainers: if this versions is good to go, can you leave
your Acks under patches 1-3 and allow Philipp to take it through the
reset tree, provided he creates an immutable branch you can pull from
for v6.19?

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>
---
Changes in v4:
- Fix an issue with uninitialized ret variable in reset core
- Use _Generic() to simplify the __SOFTWARE_NODE_REF() macro and remove
  one of the arguments
- Add a comment explaining the relationship between swnodes and fwnodes
  and why we're using the fwnode API in swnode code
- Allow longer lines
- Link to v3: https://lore.kernel.org/r/20251029-reset-gpios-swnodes-v3-0-638a4cb33201@linaro.org

Changes in v3:
- Really fix the typo in commit message in patch 7/9
- Update the commit message in patch 3/9 after implementation changes
- Don't remove checking the refnode for NULL and returning -ENOENT
- Move lockdep assertion higher up in the reset code
- Simplify patch 4/9: don't change the logic of inspecting the gpio
  device's software node
- Add new patch that still allows GPIO lookup from software nodes to
  find chips associated with any firmware nodes
- Drop the comma in reset-gpio auxiliary ID
- Drop the no longer used type argument from software node reference
  macros
- Link to v2: https://lore.kernel.org/r/20251022-reset-gpios-swnodes-v2-0-69088530291b@linaro.org

Changes in v2:
- Don't use a union for different pointer types in the software node
  reference struct
- Use fwnode_property_read_u32() instead of
  fwnode_property_read_u32_array() as we're only reading a single
  integer
- Rename reset_aux_device_release() to reset_gpio_aux_device_release()
- Initialize the device properties instead of memsetting them
- Fix typo in commit message
- As discussed on the list: I didn't change patch 7/9 because most of
  it goes away anyway in patch 9/9 and the cleanup issues will be fixed
  in the upcoming fwnode conversion
- Link to v1: https://lore.kernel.org/r/20251006-reset-gpios-swnodes-v1-0-6d3325b9af42@linaro.org

---
Bartosz Golaszewski (10):
      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: allow referencing GPIO chips by firmware nodes
      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         |  30 +++++++--
 drivers/gpio/gpiolib-swnode.c |  21 +++---
 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      |  43 ++++++++++--
 7 files changed, 173 insertions(+), 97 deletions(-)
---
base-commit: 6bc91893bd9c5d4c492cddd5b8b7a62ad1e1303c
change-id: 20250925-reset-gpios-swnodes-db553e67095b

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

[PATCH v4 01/10] 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.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/base/swnode.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index be1e9e61a7bf4d1301a3e109628517cfd9214704..016a6fd12864f2c81d4dfb021957f0c4efce4011 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -540,9 +540,7 @@ 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(refnode, nargs_prop, &nargs_prop_val);
 		if (error)
 			return error;
 

-- 
2.51.0

[PATCH v4 02/10] software node: increase the reference of the swnode by its fwnode

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Once we allow software nodes to reference other kinds of firmware nodes,
the node in args will no longer necessarily be a software node so bump
its reference count using its fwnode interface.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/base/swnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 016a6fd12864f2c81d4dfb021957f0c4efce4011..6b1ee75a908fbf272f29dbe65529ce69ce03a021 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -553,7 +553,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
 	if (!args)
 		return 0;
 
-	args->fwnode = software_node_get(refnode);
+	args->fwnode = fwnode_handle_get(refnode);
 	args->nargs = nargs;
 
 	for (i = 0; i < nargs; i++)

-- 
2.51.0

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

[Bartosz Golaszewski]

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

[PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Looking up a GPIO controller by label that is the name of the software
node is wonky at best - the GPIO controller driver is free to set
a different label than the name of its firmware node. We're already being
passed a firmware node handle attached to the GPIO device to
swnode_get_gpio_device() so use it instead for a more precise lookup.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-swnode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
 	    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
 		return ERR_PTR(-ENOENT);
 
-	gdev = gpio_device_find_by_label(gdev_node->name);
+	gdev = gpio_device_find_by_fwnode(fwnode);
 	return gdev ?: ERR_PTR(-EPROBE_DEFER);
 }
 

-- 
2.51.0

[PATCH v4 05/10] gpio: swnode: allow referencing GPIO chips by firmware nodes

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

When doing a software node lookup, we require both the fwnode that
references a GPIO chip as well as the node associated with that chip to
be software nodes. However, we now allow referencing generic firmware
nodes from software nodes in driver core so we should allow the same in
GPIO core. Make the software node name check optional and dependent on
whether the referenced firmware node is a software node. If it's not,
just continue with the lookup.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-swnode.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index e3806db1c0e077d76fcc71a50ca40bbf6872ca40..16af83fcc5aa886dd009dedc26b1ac23e5cbc4ea 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -30,16 +30,15 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
 	struct gpio_device *gdev;
 
 	gdev_node = to_software_node(fwnode);
-	if (!gdev_node || !gdev_node->name)
-		return ERR_PTR(-EINVAL);
-
-	/*
-	 * Check for a special node that identifies undefined GPIOs, this is
-	 * primarily used as a key for internal chip selects in SPI bindings.
-	 */
-	if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
-	    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
-		return ERR_PTR(-ENOENT);
+	if (gdev_node && gdev_node->name) {
+		/*
+		 * Check for a special node that identifies undefined GPIOs, this is
+		 * primarily used as a key for internal chip selects in SPI bindings.
+		 */
+		if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
+		    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
+			return ERR_PTR(-ENOENT);
+	}
 
 	gdev = gpio_device_find_by_fwnode(fwnode);
 	return gdev ?: ERR_PTR(-EPROBE_DEFER);

-- 
2.51.0

[PATCH v4 06/10] gpio: swnode: update the property definitions

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Use the recommended macros for creating references to software and
firmware nodes attached to GPIO providers.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 include/linux/gpio/property.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/gpio/property.h b/include/linux/gpio/property.h
index 0d220930800276a21b5ba96a68371ce66fc4ae3e..6b1c2ed9c57594bf3ead5edc82439f9fb7f514fd 100644
--- a/include/linux/gpio/property.h
+++ b/include/linux/gpio/property.h
@@ -7,7 +7,10 @@
 struct software_node;
 
 #define PROPERTY_ENTRY_GPIO(_name_, _chip_node_, _idx_, _flags_) \
-	PROPERTY_ENTRY_REF(_name_, _chip_node_, _idx_, _flags_)
+	PROPERTY_ENTRY_REF_SWNODE(_name_, _chip_node_, _idx_, _flags_)
+
+#define PROPERTY_ENTRY_GPIO_FWNODE(_name_, _chip_node_, _idx_, _flags_) \
+	PROPERTY_ENTRY_REF_FWNODE(_name_, _chip_node_, _idx_, _flags_)
 
 extern const struct software_node swnode_gpio_undefined;
 

-- 
2.51.0

[PATCH v4 07/10] reset: order includes alphabetically in reset/core.c

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

For better readability and easier maintenance order the includes
alphabetically.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/reset/core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 4fbaa67a6f79a4af62855c88f6b74f92c3d97159..a368b14144e7bc29ae23becab2eb7a96a4adbe44 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -4,19 +4,20 @@
  *
  * Copyright 2013 Philipp Zabel, Pengutronix
  */
+
+#include <linux/acpi.h>
 #include <linux/atomic.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/export.h>
-#include <linux/kernel.h>
-#include <linux/kref.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/machine.h>
 #include <linux/idr.h>
+#include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/acpi.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/reset-controller.h>

-- 
2.51.0

[PATCH v4 08/10] reset: make the provider of reset-gpios the parent of the reset device

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Auxiliary devices really do need a parent so ahead of converting the
reset-gpios driver to registering on the auxiliary bus, make the GPIO
device that provides the reset GPIO the parent of the reset-gpio device.
To that end move the lookup of the GPIO device by fwnode to the
beginning of __reset_add_reset_gpio_device() which has the added benefit
of bailing out earlier, before allocating resources for the virtual
device, if the chip is not up yet.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/reset/core.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index a368b14144e7bc29ae23becab2eb7a96a4adbe44..af42f4d12bbbfcba225219eac6d6c7edbe2405cc 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -822,11 +822,11 @@ static void __reset_control_put_internal(struct reset_control *rstc)
 	kref_put(&rstc->refcnt, __reset_control_release);
 }
 
-static int __reset_add_reset_gpio_lookup(int id, struct device_node *np,
+static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
+					 struct device_node *np,
 					 unsigned int gpio,
 					 unsigned int of_flags)
 {
-	const struct fwnode_handle *fwnode = of_fwnode_handle(np);
 	unsigned int lookup_flags;
 	const char *label_tmp;
 
@@ -841,10 +841,6 @@ static int __reset_add_reset_gpio_lookup(int id, struct device_node *np,
 		return -EINVAL;
 	}
 
-	struct gpio_device *gdev __free(gpio_device_put) = gpio_device_find_by_fwnode(fwnode);
-	if (!gdev)
-		return -EPROBE_DEFER;
-
 	label_tmp = gpio_device_get_label(gdev);
 	if (!label_tmp)
 		return -EINVAL;
@@ -899,6 +895,11 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 */
 	lockdep_assert_not_held(&reset_list_mutex);
 
+	struct gpio_device *gdev __free(gpio_device_put) =
+		gpio_device_find_by_fwnode(of_fwnode_handle(args->np));
+	if (!gdev)
+		return -EPROBE_DEFER;
+
 	guard(mutex)(&reset_gpio_lookup_mutex);
 
 	list_for_each_entry(rgpio_dev, &reset_gpio_lookup_list, list) {
@@ -919,7 +920,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 		goto err_ida_free;
 	}
 
-	ret = __reset_add_reset_gpio_lookup(id, args->np, args->args[0],
+	ret = __reset_add_reset_gpio_lookup(gdev, id, args->np, args->args[0],
 					    args->args[1]);
 	if (ret < 0)
 		goto err_kfree;
@@ -931,7 +932,8 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 * Hold reference as long as rgpio_dev memory is valid.
 	 */
 	of_node_get(rgpio_dev->of_args.np);
-	pdev = platform_device_register_data(NULL, "reset-gpio", id,
+	pdev = platform_device_register_data(gpio_device_to_device(gdev),
+					     "reset-gpio", id,
 					     &rgpio_dev->of_args,
 					     sizeof(rgpio_dev->of_args));
 	ret = PTR_ERR_OR_ZERO(pdev);

-- 
2.51.0

[PATCH v4 09/10] reset: gpio: convert the driver to using the auxiliary bus

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

As the reset-gpio devices are purely virtual and never instantiated from
real firmware nodes, let's convert the driver to using the - more
fitting - auxiliary bus.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/reset/Kconfig      |  1 +
 drivers/reset/core.c       | 14 ++++++--------
 drivers/reset/reset-gpio.c | 19 ++++++++++---------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 67ca87c9a86ecdbd41cbd3397d2a0c9921227eef..26c8efce0394b238691e87b04087b3d705bfadb0 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -99,6 +99,7 @@ config RESET_EYEQ
 config RESET_GPIO
 	tristate "GPIO reset controller"
 	depends on GPIOLIB
+	select AUXILIARY_BUS
 	help
 	  This enables a generic reset controller for resets attached via
 	  GPIOs.  Typically for OF platforms this driver expects "reset-gpios"
diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index af42f4d12bbbfcba225219eac6d6c7edbe2405cc..fcf1c24086e565015b0956fdd40334274a1edb00 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -7,6 +7,7 @@
 
 #include <linux/acpi.h>
 #include <linux/atomic.h>
+#include <linux/auxiliary_bus.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -18,7 +19,6 @@
 #include <linux/kref.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/reset-controller.h>
 #include <linux/slab.h>
@@ -855,7 +855,7 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
 	if (!lookup)
 		return -ENOMEM;
 
-	lookup->dev_id = kasprintf(GFP_KERNEL, "reset-gpio.%d", id);
+	lookup->dev_id = kasprintf(GFP_KERNEL, "reset.gpio.%d", id);
 	if (!lookup->dev_id)
 		return -ENOMEM;
 
@@ -876,7 +876,7 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
 static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 {
 	struct reset_gpio_lookup *rgpio_dev;
-	struct platform_device *pdev;
+	struct auxiliary_device *adev;
 	int id, ret;
 
 	/*
@@ -932,11 +932,9 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 * Hold reference as long as rgpio_dev memory is valid.
 	 */
 	of_node_get(rgpio_dev->of_args.np);
-	pdev = platform_device_register_data(gpio_device_to_device(gdev),
-					     "reset-gpio", id,
-					     &rgpio_dev->of_args,
-					     sizeof(rgpio_dev->of_args));
-	ret = PTR_ERR_OR_ZERO(pdev);
+	adev = auxiliary_device_create(gpio_device_to_device(gdev), "reset",
+				       "gpio", &rgpio_dev->of_args, id);
+	ret = PTR_ERR_OR_ZERO(adev);
 	if (ret)
 		goto err_put;
 
diff --git a/drivers/reset/reset-gpio.c b/drivers/reset/reset-gpio.c
index 2290b25b6703536f2245f15cab870bd7092d3453..e5512b3b596b5290af20e5fdd99a38f81e670d2b 100644
--- a/drivers/reset/reset-gpio.c
+++ b/drivers/reset/reset-gpio.c
@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <linux/auxiliary_bus.h>
 #include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/platform_device.h>
 #include <linux/reset-controller.h>
 
 struct reset_gpio_priv {
@@ -61,9 +61,10 @@ static void reset_gpio_of_node_put(void *data)
 	of_node_put(data);
 }
 
-static int reset_gpio_probe(struct platform_device *pdev)
+static int reset_gpio_probe(struct auxiliary_device *adev,
+			    const struct auxiliary_device_id *id)
 {
-	struct device *dev = &pdev->dev;
+	struct device *dev = &adev->dev;
 	struct of_phandle_args *platdata = dev_get_platdata(dev);
 	struct reset_gpio_priv *priv;
 	int ret;
@@ -75,7 +76,7 @@ static int reset_gpio_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, &priv->rc);
+	auxiliary_set_drvdata(adev, &priv->rc);
 
 	priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(priv->reset))
@@ -99,20 +100,20 @@ static int reset_gpio_probe(struct platform_device *pdev)
 	return devm_reset_controller_register(dev, &priv->rc);
 }
 
-static const struct platform_device_id reset_gpio_ids[] = {
-	{ .name = "reset-gpio", },
+static const struct auxiliary_device_id reset_gpio_ids[] = {
+	{ .name = "reset.gpio" },
 	{}
 };
-MODULE_DEVICE_TABLE(platform, reset_gpio_ids);
+MODULE_DEVICE_TABLE(auxiliary, reset_gpio_ids);
 
-static struct platform_driver reset_gpio_driver = {
+static struct auxiliary_driver reset_gpio_driver = {
 	.probe		= reset_gpio_probe,
 	.id_table	= reset_gpio_ids,
 	.driver	= {
 		.name = "reset-gpio",
 	},
 };
-module_platform_driver(reset_gpio_driver);
+module_auxiliary_driver(reset_gpio_driver);
 
 MODULE_AUTHOR("Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>");
 MODULE_DESCRIPTION("Generic GPIO reset driver");

-- 
2.51.0

[PATCH v4 10/10] reset: gpio: use software nodes to setup the GPIO lookup

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

GPIO machine lookup is a nice mechanism for associating GPIOs with
consumers if we don't know what kind of device the GPIO provider is or
when it will become available. However in the case of the reset-gpio, we
are already holding a reference to the device and so can reference its
firmware node. Let's setup a software node that references the relevant
GPIO and attach it to the auxiliary device we're creating.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/reset/core.c | 132 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 78 insertions(+), 54 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index fcf1c24086e565015b0956fdd40334274a1edb00..770d82ed7978f1006882908d92526fc6d8be3299 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -14,6 +14,7 @@
 #include <linux/export.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/machine.h>
+#include <linux/gpio/property.h>
 #include <linux/idr.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
@@ -77,10 +78,12 @@ struct reset_control_array {
 /**
  * struct reset_gpio_lookup - lookup key for ad-hoc created reset-gpio devices
  * @of_args: phandle to the reset controller with all the args like GPIO number
+ * @swnode: Software node containing the reference to the GPIO provider
  * @list: list entry for the reset_gpio_lookup_list
  */
 struct reset_gpio_lookup {
 	struct of_phandle_args of_args;
+	struct fwnode_handle *swnode;
 	struct list_head list;
 };
 
@@ -822,52 +825,45 @@ static void __reset_control_put_internal(struct reset_control *rstc)
 	kref_put(&rstc->refcnt, __reset_control_release);
 }
 
-static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
-					 struct device_node *np,
-					 unsigned int gpio,
-					 unsigned int of_flags)
+static void reset_gpio_aux_device_release(struct device *dev)
 {
-	unsigned int lookup_flags;
-	const char *label_tmp;
+	struct auxiliary_device *adev = to_auxiliary_dev(dev);
 
-	/*
-	 * Later we map GPIO flags between OF and Linux, however not all
-	 * constants from include/dt-bindings/gpio/gpio.h and
-	 * include/linux/gpio/machine.h match each other.
-	 */
-	if (of_flags > GPIO_ACTIVE_LOW) {
-		pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
-		       of_flags, gpio);
-		return -EINVAL;
+	kfree(adev);
+}
+
+static int reset_add_gpio_aux_device(struct device *parent,
+				     struct fwnode_handle *swnode,
+				     int id, void *pdata)
+{
+	struct auxiliary_device *adev;
+	int ret;
+
+	adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+	if (!adev)
+		return -ENOMEM;
+
+	adev->id = id;
+	adev->name = "gpio";
+	adev->dev.parent = parent;
+	adev->dev.platform_data = pdata;
+	adev->dev.release = reset_gpio_aux_device_release;
+	device_set_node(&adev->dev, swnode);
+
+	ret = auxiliary_device_init(adev);
+	if (ret) {
+		kfree(adev);
+		return ret;
 	}
 
-	label_tmp = gpio_device_get_label(gdev);
-	if (!label_tmp)
-		return -EINVAL;
+	ret = __auxiliary_device_add(adev, "reset");
+	if (ret) {
+		auxiliary_device_uninit(adev);
+		kfree(adev);
+		return ret;
+	}
 
-	char *label __free(kfree) = kstrdup(label_tmp, GFP_KERNEL);
-	if (!label)
-		return -ENOMEM;
-
-	/* Size: one lookup entry plus sentinel */
-	struct gpiod_lookup_table *lookup __free(kfree) = kzalloc(struct_size(lookup, table, 2),
-								  GFP_KERNEL);
-	if (!lookup)
-		return -ENOMEM;
-
-	lookup->dev_id = kasprintf(GFP_KERNEL, "reset.gpio.%d", id);
-	if (!lookup->dev_id)
-		return -ENOMEM;
-
-	lookup_flags = GPIO_PERSISTENT;
-	lookup_flags |= of_flags & GPIO_ACTIVE_LOW;
-	lookup->table[0] = GPIO_LOOKUP(no_free_ptr(label), gpio, "reset",
-				       lookup_flags);
-
-	/* Not freed on success, because it is persisent subsystem data. */
-	gpiod_add_lookup_table(no_free_ptr(lookup));
-
-	return 0;
+	return ret;
 }
 
 /*
@@ -875,9 +871,11 @@ static int __reset_add_reset_gpio_lookup(struct gpio_device *gdev, int id,
  */
 static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 {
+	struct property_entry properties[] = { {}, {} };
 	struct reset_gpio_lookup *rgpio_dev;
-	struct auxiliary_device *adev;
-	int id, ret;
+	unsigned int offset, of_flags;
+	struct device *parent;
+	int id, ret, lflags;
 
 	/*
 	 * Currently only #gpio-cells=2 is supported with the meaning of:
@@ -895,6 +893,23 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 */
 	lockdep_assert_not_held(&reset_list_mutex);
 
+	offset = args->args[0];
+	of_flags = args->args[1];
+
+	/*
+	 * Later we map GPIO flags between OF and Linux, however not all
+	 * constants from include/dt-bindings/gpio/gpio.h and
+	 * include/linux/gpio/machine.h match each other.
+	 *
+	 * FIXME: Find a better way of translating OF flags to GPIO lookup
+	 * flags.
+	 */
+	if (of_flags > GPIO_ACTIVE_LOW) {
+		pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
+		       of_flags, offset);
+		return -EINVAL;
+	}
+
 	struct gpio_device *gdev __free(gpio_device_put) =
 		gpio_device_find_by_fwnode(of_fwnode_handle(args->np));
 	if (!gdev)
@@ -909,6 +924,13 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 		}
 	}
 
+	lflags = GPIO_PERSISTENT | (of_flags & GPIO_ACTIVE_LOW);
+	parent = gpio_device_to_device(gdev);
+
+	properties[0] = PROPERTY_ENTRY_GPIO_FWNODE("reset-gpios",
+						   parent->fwnode,
+						   offset, lflags);
+
 	id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
 	if (id < 0)
 		return id;
@@ -920,11 +942,6 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 		goto err_ida_free;
 	}
 
-	ret = __reset_add_reset_gpio_lookup(gdev, id, args->np, args->args[0],
-					    args->args[1]);
-	if (ret < 0)
-		goto err_kfree;
-
 	rgpio_dev->of_args = *args;
 	/*
 	 * We keep the device_node reference, but of_args.np is put at the end
@@ -932,19 +949,26 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 * Hold reference as long as rgpio_dev memory is valid.
 	 */
 	of_node_get(rgpio_dev->of_args.np);
-	adev = auxiliary_device_create(gpio_device_to_device(gdev), "reset",
-				       "gpio", &rgpio_dev->of_args, id);
-	ret = PTR_ERR_OR_ZERO(adev);
+
+	rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
+	if (IS_ERR(rgpio_dev->swnode)) {
+		ret = PTR_ERR(rgpio_dev->swnode);
+		goto err_put_of_node;
+	}
+
+	ret = reset_add_gpio_aux_device(parent, rgpio_dev->swnode, id,
+					&rgpio_dev->of_args);
 	if (ret)
-		goto err_put;
+		goto err_del_swnode;
 
 	list_add(&rgpio_dev->list, &reset_gpio_lookup_list);
 
 	return 0;
 
-err_put:
+err_del_swnode:
+	fwnode_remove_software_node(rgpio_dev->swnode);
+err_put_of_node:
 	of_node_put(rgpio_dev->of_args.np);
-err_kfree:
 	kfree(rgpio_dev);
 err_ida_free:
 	ida_free(&reset_gpio_ida, id);

-- 
2.51.0

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

[Andy Shevchenko]

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?

>  (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

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Andy Shevchenko]

On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> 
> Looking up a GPIO controller by label that is the name of the software
> node is wonky at best - the GPIO controller driver is free to set
> a different label than the name of its firmware node. We're already being
> passed a firmware node handle attached to the GPIO device to
> swnode_get_gpio_device() so use it instead for a more precise lookup.

Sounds to me like a ready-to-go patch and even maybe with a Fixes tags, but
it's up to you. So, why not apply it so we have less churn in the next version
of the series?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 05/10] gpio: swnode: allow referencing GPIO chips by firmware nodes

[Andy Shevchenko]

On Mon, Nov 03, 2025 at 10:35:25AM +0100, Bartosz Golaszewski wrote:
> 
> When doing a software node lookup, we require both the fwnode that
> references a GPIO chip as well as the node associated with that chip to
> be software nodes. However, we now allow referencing generic firmware
> nodes from software nodes in driver core so we should allow the same in
> GPIO core. Make the software node name check optional and dependent on
> whether the referenced firmware node is a software node. If it's not,
> just continue with the lookup.

...

>  	gdev_node = to_software_node(fwnode);
> -	if (!gdev_node || !gdev_node->name)
> -		return ERR_PTR(-EINVAL);

The whole patch can be done in two lines (1 changed, 1 added):

		goto out_fwnode_lookup;

> -	/*
> -	 * Check for a special node that identifies undefined GPIOs, this is
> -	 * primarily used as a key for internal chip selects in SPI bindings.
> -	 */
> -	if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
> -	    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> -		return ERR_PTR(-ENOENT);
> +	if (gdev_node && gdev_node->name) {
> +		/*
> +		 * Check for a special node that identifies undefined GPIOs, this is
> +		 * primarily used as a key for internal chip selects in SPI bindings.
> +		 */
> +		if (IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED) &&
> +		    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> +			return ERR_PTR(-ENOENT);
> +	}

out_fwnode_lookup:

>  	gdev = gpio_device_find_by_fwnode(fwnode);
>  	return gdev ?: ERR_PTR(-EPROBE_DEFER);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 06/10] gpio: swnode: update the property definitions

[Andy Shevchenko]

On Mon, Nov 03, 2025 at 10:35:26AM +0100, Bartosz Golaszewski wrote:
> 
> Use the recommended macros for creating references to software and
> firmware nodes attached to GPIO providers.

Do we need now this patch at all?
See comments to the patch 3.

-- 
With Best Regards,
Andy Shevchenko

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

[Bartosz Golaszewski]

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

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

[Sakari Ailus]

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

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

On Mon, Nov 3, 2025 at 10:51 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> >
> > Looking up a GPIO controller by label that is the name of the software
> > node is wonky at best - the GPIO controller driver is free to set
> > a different label than the name of its firmware node. We're already being
> > passed a firmware node handle attached to the GPIO device to
> > swnode_get_gpio_device() so use it instead for a more precise lookup.
>
> Sounds to me like a ready-to-go patch and even maybe with a Fixes tags, but
> it's up to you. So, why not apply it so we have less churn in the next version
> of the series?
>

Yeah, makes sense.

Fixes: e7f9ff5dc90c ("gpiolib: add support for software nodes")

Re: [PATCH v4 01/10] software node: read the reference args via the fwnode API

[Sakari Ailus]

On Mon, Nov 03, 2025 at 10:35:21AM +0100, 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.
> 
> 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>

-- 
Sakari Ailus

Re: [PATCH v4 02/10] software node: increase the reference of the swnode by its fwnode

[Sakari Ailus]

On Mon, Nov 03, 2025 at 10:35:22AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Once we allow software nodes to reference other kinds of firmware nodes,
> the node in args will no longer necessarily be a software node so bump
> its reference count using its fwnode interface.
> 
> 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>

-- 
Sakari Ailus

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

[Bartosz Golaszewski]

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

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

[Andy Shevchenko]

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

Re: (subset) [PATCH v4 00/10] reset: rework reset-gpios handling

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 03 Nov 2025 10:35:20 +0100, Bartosz Golaszewski wrote:
> Software node maintainers: if this versions is good to go, can you leave
> your Acks under patches 1-3 and allow Philipp to take it through the
> reset tree, provided he creates an immutable branch you can pull from
> for v6.19?
> 
> 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.
> 
> [...]

Applied, thanks!

[04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup
        https://git.kernel.org/brgl/linux/c/e5d527be7e6984882306b49c067f1fec18920735

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

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Charles Keepax]

On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Looking up a GPIO controller by label that is the name of the software
> node is wonky at best - the GPIO controller driver is free to set
> a different label than the name of its firmware node. We're already being
> passed a firmware node handle attached to the GPIO device to
> swnode_get_gpio_device() so use it instead for a more precise lookup.
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/gpio/gpiolib-swnode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
> index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
> --- a/drivers/gpio/gpiolib-swnode.c
> +++ b/drivers/gpio/gpiolib-swnode.c
> @@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
>  	    !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
>  		return ERR_PTR(-ENOENT);
>  
> -	gdev = gpio_device_find_by_label(gdev_node->name);
> +	gdev = gpio_device_find_by_fwnode(fwnode);
>  	return gdev ?: ERR_PTR(-EPROBE_DEFER);
>  }

One small problem is this does break drivers/spi/spi-cs42l43.c.
That driver has to register some swnodes to specify some GPIO
chip selects due to some squiffy ACPI from Windows land. Currently
it relies on the sw node being called cs42l43-pinctrl to match
the driver.

I guess that is not quite the right way to handle that but its
not clear to me how to link the software node properties to the
pinctrl otherwise, anyone have any pointers there?

Note: There are a reasonable amount of shipping laptops this will
break.

Thanks,
Charles

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

On Tue, Nov 18, 2025 at 5:34 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Looking up a GPIO controller by label that is the name of the software
> > node is wonky at best - the GPIO controller driver is free to set
> > a different label than the name of its firmware node. We're already being
> > passed a firmware node handle attached to the GPIO device to
> > swnode_get_gpio_device() so use it instead for a more precise lookup.
> >
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  drivers/gpio/gpiolib-swnode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
> > index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
> > --- a/drivers/gpio/gpiolib-swnode.c
> > +++ b/drivers/gpio/gpiolib-swnode.c
> > @@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
> >           !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> >               return ERR_PTR(-ENOENT);
> >
> > -     gdev = gpio_device_find_by_label(gdev_node->name);
> > +     gdev = gpio_device_find_by_fwnode(fwnode);
> >       return gdev ?: ERR_PTR(-EPROBE_DEFER);
> >  }
>
> One small problem is this does break drivers/spi/spi-cs42l43.c.

I'd say it's a big problem. :)

> That driver has to register some swnodes to specify some GPIO
> chip selects due to some squiffy ACPI from Windows land. Currently
> it relies on the sw node being called cs42l43-pinctrl to match
> the driver.
>

What is the problem exactly? The "cs42l43-pinctrl" swnode is
associated with a GPIO device I suppose? Does it not find it? I'd need
some more information in order to figure out a way to fix it.

> I guess that is not quite the right way to handle that but its
> not clear to me how to link the software node properties to the
> pinctrl otherwise, anyone have any pointers there?
>

Depends what you mean. Creating software nodes is fine, depending on
some arbitrary string not so much. As I said: I need more information
but I'm willing to help you fix it ASAP.

Bart

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Charles Keepax]

On Tue, Nov 18, 2025 at 07:01:24PM +0100, Bartosz Golaszewski wrote:
> On Tue, Nov 18, 2025 at 5:34 PM Charles Keepax
> <ckeepax@opensource.cirrus.com> wrote:
> >
> > On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Looking up a GPIO controller by label that is the name of the software
> > > node is wonky at best - the GPIO controller driver is free to set
> > > a different label than the name of its firmware node. We're already being
> > > passed a firmware node handle attached to the GPIO device to
> > > swnode_get_gpio_device() so use it instead for a more precise lookup.
> > >
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >  drivers/gpio/gpiolib-swnode.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
> > > index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
> > > --- a/drivers/gpio/gpiolib-swnode.c
> > > +++ b/drivers/gpio/gpiolib-swnode.c
> > > @@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
> > >           !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> > >               return ERR_PTR(-ENOENT);
> > >
> > > -     gdev = gpio_device_find_by_label(gdev_node->name);
> > > +     gdev = gpio_device_find_by_fwnode(fwnode);
> > >       return gdev ?: ERR_PTR(-EPROBE_DEFER);
> > >  }
> >
> > One small problem is this does break drivers/spi/spi-cs42l43.c.
> 
> I'd say it's a big problem. :)
> 
> > That driver has to register some swnodes to specify some GPIO
> > chip selects due to some squiffy ACPI from Windows land. Currently
> > it relies on the sw node being called cs42l43-pinctrl to match
> > the driver.
> >
> 
> What is the problem exactly? The "cs42l43-pinctrl" swnode is
> associated with a GPIO device I suppose? Does it not find it? I'd need
> some more information in order to figure out a way to fix it.

Yeah doesn't find the GPIO device. Apologies the background is
fairly lenghty here but to give a high level summary. The cs42l43
is an audio CODEC but it has a SPI controller on it, in some
configurations there are a couple of speaker amps connected to
this SPI controller. For Window reasons this SPI controller isn't
properly represented in ACPI, so we have to depend on a couple
magic properties and then use software nodes to register the
speaker amps. However, as part of this we need to register a
cs-gpios property for the chip selects used by the amps.

This chip select gpios property is where the problem happens, we
need to refer to the gpio/pinctrl driver of the cs42l43, but
software nodes only seem to allow referring to other software
nodes. Previously this worked as we gave the node the same name
as the real driver, which meant it found the real driver.
However, after switching to look up by fwnode, it is looking for
a gpio controller attached to the software node.

static const struct software_node cs42l43_gpiochip_swnode = {
	/* Previously matched the driver name for the pinctrl driver */
	.name	=  "cs42l43-pinctrl",
};

static const struct software_node_ref_args cs42l43_cs_refs[] = {
	/* This needs to point to the genuine pinctrl driver? */
	SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
	/* This is a mark that indicates the native chip select is used*/
	SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
};

The bit that is unclear to me is how we get that software node
property to point to the real pinctrl driver rather than the
dummy software node. I think maybe we need to add a SW node as a
secondary node on the pinctrl driver itself and link to that?

Also happy from my side to spend some time working on this as I
am not convinced what the driver is doing now is totally legit.

Thanks,
Charles

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Andy Shevchenko]

On Tue, Nov 18, 2025 at 06:15:49PM +0000, Charles Keepax wrote:
> On Tue, Nov 18, 2025 at 07:01:24PM +0100, Bartosz Golaszewski wrote:
> > On Tue, Nov 18, 2025 at 5:34 PM Charles Keepax
> > <ckeepax@opensource.cirrus.com> wrote:
> > > On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:

...

> > > One small problem is this does break drivers/spi/spi-cs42l43.c.
> > 
> > I'd say it's a big problem. :)
> > 
> > > That driver has to register some swnodes to specify some GPIO
> > > chip selects due to some squiffy ACPI from Windows land. Currently
> > > it relies on the sw node being called cs42l43-pinctrl to match
> > > the driver.
> > 
> > What is the problem exactly? The "cs42l43-pinctrl" swnode is
> > associated with a GPIO device I suppose? Does it not find it? I'd need
> > some more information in order to figure out a way to fix it.
> 
> Yeah doesn't find the GPIO device. Apologies the background is
> fairly lenghty here but to give a high level summary. The cs42l43
> is an audio CODEC but it has a SPI controller on it, in some
> configurations there are a couple of speaker amps connected to
> this SPI controller. For Window reasons this SPI controller isn't
> properly represented in ACPI, so we have to depend on a couple
> magic properties and then use software nodes to register the
> speaker amps. However, as part of this we need to register a
> cs-gpios property for the chip selects used by the amps.
> 
> This chip select gpios property is where the problem happens, we
> need to refer to the gpio/pinctrl driver of the cs42l43, but
> software nodes only seem to allow referring to other software
> nodes.

Interestingly that Bart's series from where this patch came has more patches
targeting exactly this scenario, i.e. to allow to refer any type of fwnode from
swnode. Maybe we need those too and something on top?

> Previously this worked as we gave the node the same name
> as the real driver, which meant it found the real driver.
> However, after switching to look up by fwnode, it is looking for
> a gpio controller attached to the software node.
> 
> static const struct software_node cs42l43_gpiochip_swnode = {
> 	/* Previously matched the driver name for the pinctrl driver */
> 	.name	=  "cs42l43-pinctrl",
> };
> 
> static const struct software_node_ref_args cs42l43_cs_refs[] = {
> 	/* This needs to point to the genuine pinctrl driver? */
> 	SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
> 	/* This is a mark that indicates the native chip select is used*/
> 	SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
> };
> 
> The bit that is unclear to me is how we get that software node
> property to point to the real pinctrl driver rather than the
> dummy software node. I think maybe we need to add a SW node as a
> secondary node on the pinctrl driver itself and link to that?
> 
> Also happy from my side to spend some time working on this as I
> am not convinced what the driver is doing now is totally legit.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

On Tue, Nov 18, 2025 at 7:16 PM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> On Tue, Nov 18, 2025 at 07:01:24PM +0100, Bartosz Golaszewski wrote:
> > On Tue, Nov 18, 2025 at 5:34 PM Charles Keepax
> > <ckeepax@opensource.cirrus.com> wrote:
> > >
> > > On Mon, Nov 03, 2025 at 10:35:24AM +0100, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > >
> > > > Looking up a GPIO controller by label that is the name of the software
> > > > node is wonky at best - the GPIO controller driver is free to set
> > > > a different label than the name of its firmware node. We're already being
> > > > passed a firmware node handle attached to the GPIO device to
> > > > swnode_get_gpio_device() so use it instead for a more precise lookup.
> > > >
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > > ---
> > > >  drivers/gpio/gpiolib-swnode.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
> > > > index f21dbc28cf2c8c2d06d034b7c89d302cc52bb9b5..e3806db1c0e077d76fcc71a50ca40bbf6872ca40 100644
> > > > --- a/drivers/gpio/gpiolib-swnode.c
> > > > +++ b/drivers/gpio/gpiolib-swnode.c
> > > > @@ -41,7 +41,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
> > > >           !strcmp(gdev_node->name, GPIOLIB_SWNODE_UNDEFINED_NAME))
> > > >               return ERR_PTR(-ENOENT);
> > > >
> > > > -     gdev = gpio_device_find_by_label(gdev_node->name);
> > > > +     gdev = gpio_device_find_by_fwnode(fwnode);
> > > >       return gdev ?: ERR_PTR(-EPROBE_DEFER);
> > > >  }
> > >
> > > One small problem is this does break drivers/spi/spi-cs42l43.c.
> >
> > I'd say it's a big problem. :)
> >
> > > That driver has to register some swnodes to specify some GPIO
> > > chip selects due to some squiffy ACPI from Windows land. Currently
> > > it relies on the sw node being called cs42l43-pinctrl to match
> > > the driver.
> > >
> >
> > What is the problem exactly? The "cs42l43-pinctrl" swnode is
> > associated with a GPIO device I suppose? Does it not find it? I'd need
> > some more information in order to figure out a way to fix it.
>
> Yeah doesn't find the GPIO device. Apologies the background is
> fairly lenghty here but to give a high level summary. The cs42l43
> is an audio CODEC but it has a SPI controller on it, in some
> configurations there are a couple of speaker amps connected to
> this SPI controller. For Window reasons this SPI controller isn't
> properly represented in ACPI, so we have to depend on a couple
> magic properties and then use software nodes to register the
> speaker amps. However, as part of this we need to register a
> cs-gpios property for the chip selects used by the amps.
>
> This chip select gpios property is where the problem happens, we
> need to refer to the gpio/pinctrl driver of the cs42l43, but
> software nodes only seem to allow referring to other software
> nodes. Previously this worked as we gave the node the same name
> as the real driver, which meant it found the real driver.
> However, after switching to look up by fwnode, it is looking for
> a gpio controller attached to the software node.
>

As mentioned by Andy: this sounds precisely like what this series
addresses with the reset-gpio driver. We now CAN reference fwnodes
from software nodes.


> static const struct software_node cs42l43_gpiochip_swnode = {
>         /* Previously matched the driver name for the pinctrl driver */
>         .name   =  "cs42l43-pinctrl",
> };
>
> static const struct software_node_ref_args cs42l43_cs_refs[] = {
>         /* This needs to point to the genuine pinctrl driver? */
>         SOFTWARE_NODE_REFERENCE(&cs42l43_gpiochip_swnode, 0, GPIO_ACTIVE_LOW),
>         /* This is a mark that indicates the native chip select is used*/
>         SOFTWARE_NODE_REFERENCE(&swnode_gpio_undefined),
> };
>
> The bit that is unclear to me is how we get that software node
> property to point to the real pinctrl driver rather than the
> dummy software node. I think maybe we need to add a SW node as a
> secondary node on the pinctrl driver itself and link to that?
>
> Also happy from my side to spend some time working on this as I
> am not convinced what the driver is doing now is totally legit.

Well, it is sketchy. We register the cs42l43_gpiochip_swnode and
reference it but never assign it to the GPIO device. If you assigned
it as a secondary fwnode to the relevant GPIO device, it would have
been found during the lookup.

Right now, with how the lookup-by-label works in gpiolib-swnode.c, you
get the referenced software node and read its name but there's no real
link between the swnode and the GPIO device. It's just a big hack.

I have an idea for fixing it, let me cook up a patch. It'll still be a
bit hacky but will at least create a true link.

Bart

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> I have an idea for fixing it, let me cook up a patch. It'll still be a
> bit hacky but will at least create a true link.
>

Scratch that, I didn't notice before but we register both devices from
MFD core. We can just set up software nodes there.

Bart

Re: [PATCH v4 04/10] gpio: swnode: don't use the swnode's name as the key for GPIO lookup

[Bartosz Golaszewski]

On Wed, Nov 19, 2025 at 9:41 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Wed, Nov 19, 2025 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > I have an idea for fixing it, let me cook up a patch. It'll still be a
> > bit hacky but will at least create a true link.
> >
>
> Scratch that, I didn't notice before but we register both devices from
> MFD core. We can just set up software nodes there.
>

Here you go: https://lore.kernel.org/all/20251119-cs42l43-gpio-swnodes-v1-1-25996afebd97@linaro.org/

Please give it a try. This is independent from this series and should
probably be backported to stable.

Bartosz