linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/8] reset: rework reset-gpios handling
@ 2025-11-06 14:32 Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 1/8] software node: read the reference args via the fwnode API Bartosz Golaszewski
                   ` (8 more replies)
  0 siblings, 9 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski

NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
swnode's name as the key for GPIO lookup") into my fixes branch and will
send it upstream by the end of this week. It will be part of v6.18-rc5
which tag will need to be the base for the future immutable branch
created by Philipp.

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 v6:
- Do ref[0].swnode -> ref->swnode in software_node_graph_get_remote_endpoint()
- Fix some nit picks from Andy
- Link to v5: https://lore.kernel.org/r/20251105-reset-gpios-swnodes-v5-0-1f67499a8287@linaro.org

Changes in v5:
- Use _Generic() even more and simplify the patch allowing to reference
  firmware nodes significantly
- Use _Generic() to avoid adding more macros to linux/property.h
- Don't rename macro arguments in linux/property.h
- Drop patch renaming the GPIO reference property
- Pick up the patch modifying the swnode GPIO lookup to using fwnodes
  into my fixes branch
- Simplify the patch allowing GPIO swnode references to reference
  firmware nodes
- Link to v4: https://lore.kernel.org/r/20251103-reset-gpios-swnodes-v4-0-6461800b6775@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 (8):
      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: allow referencing GPIO chips by firmware nodes
      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 |   3 +-
 drivers/reset/Kconfig         |   1 +
 drivers/reset/core.c          | 145 ++++++++++++++++++++++++------------------
 drivers/reset/reset-gpio.c    |  19 +++---
 include/linux/property.h      |  13 +++-
 6 files changed, 130 insertions(+), 81 deletions(-)
---
base-commit: cb69ab793066d9038345da89e54545aea34aaef0
change-id: 20250925-reset-gpios-swnodes-db553e67095b

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 1/8] software node: read the reference args via the fwnode API
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 2/8] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 2/8] software node: increase the reference of the swnode by its fwnode
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 1/8] software node: read the reference args via the fwnode API Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 3/8] software node: allow referencing firmware nodes Bartosz Golaszewski
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 3/8] software node: allow referencing firmware nodes
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 1/8] software node: read the reference args via the fwnode API Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 2/8] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 4/8] gpio: swnode: allow referencing GPIO chips by " Bartosz Golaszewski
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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.

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>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 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..16a8301c25d6390ba304c66411d1a261345184f3 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->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__ },				\
 }

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 4/8] gpio: swnode: allow referencing GPIO chips by firmware nodes
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 3/8] software node: allow referencing firmware nodes Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 5/8] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-swnode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-swnode.c b/drivers/gpio/gpiolib-swnode.c
index e3806db1c0e077d76fcc71a50ca40bbf6872ca40..b44f35d68459095a14b48056fca2c58e04b0a2ed 100644
--- a/drivers/gpio/gpiolib-swnode.c
+++ b/drivers/gpio/gpiolib-swnode.c
@@ -31,7 +31,7 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
 
 	gdev_node = to_software_node(fwnode);
 	if (!gdev_node || !gdev_node->name)
-		return ERR_PTR(-EINVAL);
+		goto fwnode_lookup;
 
 	/*
 	 * Check for a special node that identifies undefined GPIOs, this is
@@ -41,6 +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);
 
+fwnode_lookup:
 	gdev = gpio_device_find_by_fwnode(fwnode);
 	return gdev ?: ERR_PTR(-EPROBE_DEFER);
 }

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 5/8] reset: order includes alphabetically in reset/core.c
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 4/8] gpio: swnode: allow referencing GPIO chips by " Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 6/8] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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 8029e547e3db90ddb85c717dfd735bc8a314dd44..b3d3339dd6d30495b498b3f650d18fdd96a6bca7 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 6/8] reset: make the provider of reset-gpios the parent of the reset device
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (4 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 5/8] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 7/8] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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 b3d3339dd6d30495b498b3f650d18fdd96a6bca7..31dcd25b465d795dda275f751ed6ce28d4dc27ee 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 7/8] reset: gpio: convert the driver to using the auxiliary bus
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (5 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 6/8] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-06 14:32 ` [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
  2025-11-10  9:02 ` [PATCH v6 0/8] reset: rework reset-gpios handling Philipp Zabel
  8 siblings, 0 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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 31dcd25b465d795dda275f751ed6ce28d4dc27ee..cefeff10f6c82f5aef269a6d3a58d9d204ed6b7e 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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (6 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 7/8] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
@ 2025-11-06 14:32 ` Bartosz Golaszewski
  2025-11-18 16:44   ` Philipp Zabel
  2025-11-10  9:02 ` [PATCH v6 0/8] reset: rework reset-gpios handling Philipp Zabel
  8 siblings, 1 reply; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-06 14:32 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Philipp Zabel,
	Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, 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 | 126 +++++++++++++++++++++++++++++----------------------
 1 file changed, 73 insertions(+), 53 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index cefeff10f6c82f5aef269a6d3a58d9d204ed6b7e..8262879e3f0d9ce67683c6baa00d9eea9e3c3ca3 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,8 +871,10 @@ 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[2] = { };
+	unsigned int offset, of_flags, lflags;
 	struct reset_gpio_lookup *rgpio_dev;
-	struct auxiliary_device *adev;
+	struct device *parent;
 	int id, ret;
 
 	/*
@@ -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,10 @@ 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("reset-gpios", parent->fwnode, offset, lflags);
+
 	id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
 	if (id < 0)
 		return id;
@@ -920,11 +939,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 +946,25 @@ 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);
+	ret = PTR_ERR(rgpio_dev->swnode);
 	if (ret)
-		goto err_put;
+		goto err_put_of_node;
+
+	ret = reset_add_gpio_aux_device(parent, rgpio_dev->swnode, id,
+					&rgpio_dev->of_args);
+	if (ret)
+		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


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
                   ` (7 preceding siblings ...)
  2025-11-06 14:32 ` [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
@ 2025-11-10  9:02 ` Philipp Zabel
  2025-11-10 16:57   ` Bartosz Golaszewski
  8 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2025-11-10  9:02 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski

On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
> swnode's name as the key for GPIO lookup") into my fixes branch and will
> send it upstream by the end of this week. It will be part of v6.18-rc5
> which tag will need to be the base for the future immutable branch
> created by Philipp.
> 
> 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?

Now that -rc5 is out, could I get an Ack to create an immutable branch
with this series on top of v6.18-rc5 (and merge it into reset/next)?

regards
Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-10  9:02 ` [PATCH v6 0/8] reset: rework reset-gpios handling Philipp Zabel
@ 2025-11-10 16:57   ` Bartosz Golaszewski
  2025-11-13 10:30     ` Philipp Zabel
  0 siblings, 1 reply; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-10 16:57 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Mon, Nov 10, 2025 at 10:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
> > swnode's name as the key for GPIO lookup") into my fixes branch and will
> > send it upstream by the end of this week. It will be part of v6.18-rc5
> > which tag will need to be the base for the future immutable branch
> > created by Philipp.
> >
> > 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?
>
> Now that -rc5 is out, could I get an Ack to create an immutable branch
> with this series on top of v6.18-rc5 (and merge it into reset/next)?
>

Hi Philipp,

I assume the Reviewed-by tags by Andy and Sakari under patches 1-3
make them good enough to go in?

Bart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-10 16:57   ` Bartosz Golaszewski
@ 2025-11-13 10:30     ` Philipp Zabel
  2025-11-13 12:16       ` Bartosz Golaszewski
  0 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2025-11-13 10:30 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Mo, 2025-11-10 at 17:57 +0100, Bartosz Golaszewski wrote:
> On Mon, Nov 10, 2025 at 10:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > 
> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > > NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
> > > swnode's name as the key for GPIO lookup") into my fixes branch and will
> > > send it upstream by the end of this week. It will be part of v6.18-rc5
> > > which tag will need to be the base for the future immutable branch
> > > created by Philipp.
> > > 
> > > 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?
> > 
> > Now that -rc5 is out, could I get an Ack to create an immutable branch
> > with this series on top of v6.18-rc5 (and merge it into reset/next)?
> > 
> 
> Hi Philipp,
> 
> I assume the Reviewed-by tags by Andy and Sakari under patches 1-3
> make them good enough to go in?

I assumed I also need an Acked-by by Greg or Rafael.

regards
Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-13 10:30     ` Philipp Zabel
@ 2025-11-13 12:16       ` Bartosz Golaszewski
  2025-11-13 12:39         ` Bartosz Golaszewski
  0 siblings, 1 reply; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-13 12:16 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski, Bartosz Golaszewski

On Thu, 13 Nov 2025 11:30:39 +0100, Philipp Zabel <p.zabel@pengutronix.de> said:
> On Mo, 2025-11-10 at 17:57 +0100, Bartosz Golaszewski wrote:
>> On Mon, Nov 10, 2025 at 10:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> >
>> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
>> > > NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
>> > > swnode's name as the key for GPIO lookup") into my fixes branch and will
>> > > send it upstream by the end of this week. It will be part of v6.18-rc5
>> > > which tag will need to be the base for the future immutable branch
>> > > created by Philipp.
>> > >
>> > > 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?
>> >
>> > Now that -rc5 is out, could I get an Ack to create an immutable branch
>> > with this series on top of v6.18-rc5 (and merge it into reset/next)?
>> >
>>
>> Hi Philipp,
>>
>> I assume the Reviewed-by tags by Andy and Sakari under patches 1-3
>> make them good enough to go in?
>
> I assumed I also need an Acked-by by Greg or Rafael.
>

From MAINTAINERS:

SOFTWARE NODES AND DEVICE PROPERTIES
R:      Andy Shevchenko <andriy.shevchenko@linux.intel.com>
R:      Daniel Scally <djrscally@gmail.com>
R:      Heikki Krogerus <heikki.krogerus@linux.intel.com>
R:      Sakari Ailus <sakari.ailus@linux.intel.com>

Looks like neither Greg nor Rafael are mentioned.

Bart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-13 12:16       ` Bartosz Golaszewski
@ 2025-11-13 12:39         ` Bartosz Golaszewski
  2025-11-18 11:12           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-13 12:39 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Thu, Nov 13, 2025 at 1:16 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Thu, 13 Nov 2025 11:30:39 +0100, Philipp Zabel <p.zabel@pengutronix.de> said:
> > On Mo, 2025-11-10 at 17:57 +0100, Bartosz Golaszewski wrote:
> >> On Mon, Nov 10, 2025 at 10:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> >> >
> >> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> >> > > NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
> >> > > swnode's name as the key for GPIO lookup") into my fixes branch and will
> >> > > send it upstream by the end of this week. It will be part of v6.18-rc5
> >> > > which tag will need to be the base for the future immutable branch
> >> > > created by Philipp.
> >> > >
> >> > > 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?
> >> >
> >> > Now that -rc5 is out, could I get an Ack to create an immutable branch
> >> > with this series on top of v6.18-rc5 (and merge it into reset/next)?
> >> >
> >>
> >> Hi Philipp,
> >>
> >> I assume the Reviewed-by tags by Andy and Sakari under patches 1-3
> >> make them good enough to go in?
> >
> > I assumed I also need an Acked-by by Greg or Rafael.
> >
>
> From MAINTAINERS:
>
> SOFTWARE NODES AND DEVICE PROPERTIES
> R:      Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> R:      Daniel Scally <djrscally@gmail.com>
> R:      Heikki Krogerus <heikki.krogerus@linux.intel.com>
> R:      Sakari Ailus <sakari.ailus@linux.intel.com>
>
> Looks like neither Greg nor Rafael are mentioned.
>

Ah but also:

DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS
M:      Greg Kroah-Hartman <gregkh@linuxfoundation.org>
M:      "Rafael J. Wysocki" <rafael@kernel.org>
M:      Danilo Krummrich <dakr@kernel.org>

So depending how we look at it. Greg, Rafael, Danilo: can you leave an Ack here?

Bartosz

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/8] reset: rework reset-gpios handling
  2025-11-13 12:39         ` Bartosz Golaszewski
@ 2025-11-18 11:12           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 21+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-18 11:12 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Philipp Zabel, Linus Walleij, Andy Shevchenko, Daniel Scally,
	Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Thu, Nov 13, 2025 at 01:39:47PM +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 13, 2025 at 1:16 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > On Thu, 13 Nov 2025 11:30:39 +0100, Philipp Zabel <p.zabel@pengutronix.de> said:
> > > On Mo, 2025-11-10 at 17:57 +0100, Bartosz Golaszewski wrote:
> > >> On Mon, Nov 10, 2025 at 10:02 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > >> >
> > >> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > >> > > NOTE: I've picked up commit e5d527be7e69 ("gpio: swnode: don't use the
> > >> > > swnode's name as the key for GPIO lookup") into my fixes branch and will
> > >> > > send it upstream by the end of this week. It will be part of v6.18-rc5
> > >> > > which tag will need to be the base for the future immutable branch
> > >> > > created by Philipp.
> > >> > >
> > >> > > 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?
> > >> >
> > >> > Now that -rc5 is out, could I get an Ack to create an immutable branch
> > >> > with this series on top of v6.18-rc5 (and merge it into reset/next)?
> > >> >
> > >>
> > >> Hi Philipp,
> > >>
> > >> I assume the Reviewed-by tags by Andy and Sakari under patches 1-3
> > >> make them good enough to go in?
> > >
> > > I assumed I also need an Acked-by by Greg or Rafael.
> > >
> >
> > From MAINTAINERS:
> >
> > SOFTWARE NODES AND DEVICE PROPERTIES
> > R:      Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > R:      Daniel Scally <djrscally@gmail.com>
> > R:      Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > R:      Sakari Ailus <sakari.ailus@linux.intel.com>
> >
> > Looks like neither Greg nor Rafael are mentioned.
> >
> 
> Ah but also:
> 
> DRIVER CORE, KOBJECTS, DEBUGFS AND SYSFS
> M:      Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> M:      "Rafael J. Wysocki" <rafael@kernel.org>
> M:      Danilo Krummrich <dakr@kernel.org>
> 
> So depending how we look at it. Greg, Rafael, Danilo: can you leave an Ack here?

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-06 14:32 ` [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
@ 2025-11-18 16:44   ` Philipp Zabel
  2025-11-18 17:08     ` Bartosz Golaszewski
  0 siblings, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2025-11-18 16:44 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko,
	Daniel Scally, Heikki Krogerus, Sakari Ailus, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Krzysztof Kozlowski
  Cc: linux-gpio, linux-kernel, linux-acpi, Bartosz Golaszewski

On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> 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 | 126 +++++++++++++++++++++++++++++----------------------
>  1 file changed, 73 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index cefeff10f6c82f5aef269a6d3a58d9d204ed6b7e..8262879e3f0d9ce67683c6baa00d9eea9e3c3ca3 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,8 +871,10 @@ 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[2] = { };
> +	unsigned int offset, of_flags, lflags;
>  	struct reset_gpio_lookup *rgpio_dev;
> -	struct auxiliary_device *adev;
> +	struct device *parent;
>  	int id, ret;
>  
>  	/*
> @@ -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,10 @@ 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("reset-gpios", parent->fwnode, offset, lflags);
> +
>  	id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
>  	if (id < 0)
>  		return id;
> @@ -920,11 +939,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 +946,25 @@ 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);
> +	ret = PTR_ERR(rgpio_dev->swnode);

I'll apply this with the following patch squashed in:

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 3edf04ae8a95..8a7b112a9a77 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
        of_node_get(rgpio_dev->of_args.np);
 
        rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
-       ret = PTR_ERR(rgpio_dev->swnode);
+       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
        if (ret)
                goto err_put_of_node;
 

regards
Philipp

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-18 16:44   ` Philipp Zabel
@ 2025-11-18 17:08     ` Bartosz Golaszewski
  2025-11-18 18:09       ` Andy Shevchenko
  2025-11-19  8:19       ` Philipp Zabel
  0 siblings, 2 replies; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-18 17:08 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Tue, Nov 18, 2025 at 5:44 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>
> On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > 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>
> > ---
>
> I'll apply this with the following patch squashed in:
>
> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> index 3edf04ae8a95..8a7b112a9a77 100644
> --- a/drivers/reset/core.c
> +++ b/drivers/reset/core.c
> @@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
>         of_node_get(rgpio_dev->of_args.np);
>
>         rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
> -       ret = PTR_ERR(rgpio_dev->swnode);
> +       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
>         if (ret)
>                 goto err_put_of_node;

Huh? Why?

Bartosz

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-18 17:08     ` Bartosz Golaszewski
@ 2025-11-18 18:09       ` Andy Shevchenko
  2025-11-19  8:19       ` Philipp Zabel
  1 sibling, 0 replies; 21+ messages in thread
From: Andy Shevchenko @ 2025-11-18 18:09 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Philipp Zabel, Linus Walleij, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Tue, Nov 18, 2025 at 06:08:17PM +0100, Bartosz Golaszewski wrote:
> On Tue, Nov 18, 2025 at 5:44 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:

...

> > I'll apply this with the following patch squashed in:
> >
> > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> > index 3edf04ae8a95..8a7b112a9a77 100644
> > --- a/drivers/reset/core.c
> > +++ b/drivers/reset/core.c
> > @@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> >         of_node_get(rgpio_dev->of_args.np);
> >
> >         rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
> > -       ret = PTR_ERR(rgpio_dev->swnode);
> > +       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
> >         if (ret)
> >                 goto err_put_of_node;
> 
> Huh? Why?

Hmm... Maybe we need a kernel-doc for fwnode_create_software_node() to be more
explicit on what it might return (and no, NULL is not there, Bart is correct).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-18 17:08     ` Bartosz Golaszewski
  2025-11-18 18:09       ` Andy Shevchenko
@ 2025-11-19  8:19       ` Philipp Zabel
  2025-11-19  8:28         ` Bartosz Golaszewski
  1 sibling, 1 reply; 21+ messages in thread
From: Philipp Zabel @ 2025-11-19  8:19 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Di, 2025-11-18 at 18:08 +0100, Bartosz Golaszewski wrote:
> On Tue, Nov 18, 2025 at 5:44 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > 
> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > > 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>
> > > ---
> > 
> > I'll apply this with the following patch squashed in:

Strike that, I'll have to wait for the SPI issue to be resolved.

> > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> > index 3edf04ae8a95..8a7b112a9a77 100644
> > --- a/drivers/reset/core.c
> > +++ b/drivers/reset/core.c
> > @@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> >         of_node_get(rgpio_dev->of_args.np);
> > 
> >         rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
> > -       ret = PTR_ERR(rgpio_dev->swnode);
> > +       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
> >         if (ret)
> >                 goto err_put_of_node;
> 
> Huh? Why?

PTR_ERR(ptr) is just (long)ptr, so a valid swnode pointer makes ret
non-zero and takes us into the error path. PTR_ERR_OR_ZERO() includes
the IS_ERR() check and returns 0 for non-error pointers.

And there is a (false-positive) sparse warning:

 drivers/reset/core.c:978 __reset_add_reset_gpio_device() warn: passing zero to 'PTR_ERR'

I think it would be better to return to the explicit IS_ERR() check
from v5.

regards
Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-19  8:19       ` Philipp Zabel
@ 2025-11-19  8:28         ` Bartosz Golaszewski
  2025-11-19  8:41           ` Philipp Zabel
  0 siblings, 1 reply; 21+ messages in thread
From: Bartosz Golaszewski @ 2025-11-19  8:28 UTC (permalink / raw)
  To: Philipp Zabel
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski, Bartosz Golaszewski

On Wed, 19 Nov 2025 09:19:30 +0100, Philipp Zabel <p.zabel@pengutronix.de> said:
> On Di, 2025-11-18 at 18:08 +0100, Bartosz Golaszewski wrote:
>> On Tue, Nov 18, 2025 at 5:44 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
>> >
>> > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
>> > > 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>
>> > > ---
>> >
>> > I'll apply this with the following patch squashed in:
>
> Strike that, I'll have to wait for the SPI issue to be resolved.
>
>> > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
>> > index 3edf04ae8a95..8a7b112a9a77 100644
>> > --- a/drivers/reset/core.c
>> > +++ b/drivers/reset/core.c
>> > @@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
>> >         of_node_get(rgpio_dev->of_args.np);
>> >
>> >         rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
>> > -       ret = PTR_ERR(rgpio_dev->swnode);
>> > +       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
>> >         if (ret)
>> >                 goto err_put_of_node;
>>
>> Huh? Why?
>
> PTR_ERR(ptr) is just (long)ptr, so a valid swnode pointer makes ret
> non-zero and takes us into the error path. PTR_ERR_OR_ZERO() includes
> the IS_ERR() check and returns 0 for non-error pointers.
>
> And there is a (false-positive) sparse warning:
>
>  drivers/reset/core.c:978 __reset_add_reset_gpio_device() warn: passing zero to 'PTR_ERR'
>
> I think it would be better to return to the explicit IS_ERR() check
> from v5.
>

Yes, it was like this in my initial submission and seems like it's more
readable and doesn't cause this confusion. I will go back to it. Though
I'm not sure if the SPI issue will require a v5 - I'm looking into fixing it
in the affected driver.

Bart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup
  2025-11-19  8:28         ` Bartosz Golaszewski
@ 2025-11-19  8:41           ` Philipp Zabel
  0 siblings, 0 replies; 21+ messages in thread
From: Philipp Zabel @ 2025-11-19  8:41 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Andy Shevchenko, Daniel Scally, Heikki Krogerus,
	Sakari Ailus, Greg Kroah-Hartman, Rafael J. Wysocki,
	Danilo Krummrich, Krzysztof Kozlowski, linux-gpio, linux-kernel,
	linux-acpi, Bartosz Golaszewski

On Mi, 2025-11-19 at 00:28 -0800, Bartosz Golaszewski wrote:
> On Wed, 19 Nov 2025 09:19:30 +0100, Philipp Zabel <p.zabel@pengutronix.de> said:
> > On Di, 2025-11-18 at 18:08 +0100, Bartosz Golaszewski wrote:
> > > On Tue, Nov 18, 2025 at 5:44 PM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > > > 
> > > > On Do, 2025-11-06 at 15:32 +0100, Bartosz Golaszewski wrote:
> > > > > 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>
> > > > > ---
> > > > 
> > > > I'll apply this with the following patch squashed in:
> > 
> > Strike that, I'll have to wait for the SPI issue to be resolved.
> > 
> > > > diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> > > > index 3edf04ae8a95..8a7b112a9a77 100644
> > > > --- a/drivers/reset/core.c
> > > > +++ b/drivers/reset/core.c
> > > > @@ -945,7 +945,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
> > > >         of_node_get(rgpio_dev->of_args.np);
> > > > 
> > > >         rgpio_dev->swnode = fwnode_create_software_node(properties, NULL);
> > > > -       ret = PTR_ERR(rgpio_dev->swnode);
> > > > +       ret = PTR_ERR_OR_ZERO(rgpio_dev->swnode);
> > > >         if (ret)
> > > >                 goto err_put_of_node;
> > > 
> > > Huh? Why?
> > 
> > PTR_ERR(ptr) is just (long)ptr, so a valid swnode pointer makes ret
> > non-zero and takes us into the error path. PTR_ERR_OR_ZERO() includes
> > the IS_ERR() check and returns 0 for non-error pointers.
> > 
> > And there is a (false-positive) sparse warning:
> > 
> >  drivers/reset/core.c:978 __reset_add_reset_gpio_device() warn: passing zero to 'PTR_ERR'
> > 
> > I think it would be better to return to the explicit IS_ERR() check
> > from v5.
> > 
> 
> Yes, it was like this in my initial submission and seems like it's more
> readable and doesn't cause this confusion. I will go back to it. Though
> I'm not sure if the SPI issue will require a v5

v7 :)

If there are no other changes required, I can fix it up:

  https://git.pengutronix.de/cgit/pza/linux/commit/?id=8e72a48fc1df

>  - I'm looking into fixing it in the affected driver.

Great, can we make sure the fix is applied first, to avoid breaking git
bisect?

regards
Philipp

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-11-19  8:42 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-06 14:32 [PATCH v6 0/8] reset: rework reset-gpios handling Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 1/8] software node: read the reference args via the fwnode API Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 2/8] software node: increase the reference of the swnode by its fwnode Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 3/8] software node: allow referencing firmware nodes Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 4/8] gpio: swnode: allow referencing GPIO chips by " Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 5/8] reset: order includes alphabetically in reset/core.c Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 6/8] reset: make the provider of reset-gpios the parent of the reset device Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 7/8] reset: gpio: convert the driver to using the auxiliary bus Bartosz Golaszewski
2025-11-06 14:32 ` [PATCH v6 8/8] reset: gpio: use software nodes to setup the GPIO lookup Bartosz Golaszewski
2025-11-18 16:44   ` Philipp Zabel
2025-11-18 17:08     ` Bartosz Golaszewski
2025-11-18 18:09       ` Andy Shevchenko
2025-11-19  8:19       ` Philipp Zabel
2025-11-19  8:28         ` Bartosz Golaszewski
2025-11-19  8:41           ` Philipp Zabel
2025-11-10  9:02 ` [PATCH v6 0/8] reset: rework reset-gpios handling Philipp Zabel
2025-11-10 16:57   ` Bartosz Golaszewski
2025-11-13 10:30     ` Philipp Zabel
2025-11-13 12:16       ` Bartosz Golaszewski
2025-11-13 12:39         ` Bartosz Golaszewski
2025-11-18 11:12           ` Greg Kroah-Hartman

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