Re: [RFC PATCH 1/1] reset: add support the GPIO provider with #gpio-cells=3

Re: [RFC PATCH 1/1] reset: add support the GPIO provider with #gpio-cells=3

[Krzysztof Kozlowski]

On 04/11/2026 04:56, Marc Taylor wrote:
> Sorry, my mail client mangled the inline diff in my previous reply
> (indentation stripped, and it turned "

Then please switch to standard email clients. See also kernel docs for
helping to setup normal apps properly handling email.


> https://www.google.com/url?q=http://spec.np&source=gmail&ust=1793850929317000&sa=E"
> into a link). The
> attached reset-gpio-3cell-v7.2.15.patch is the correct version; it is
> re-attached here so this message stands on its own.

I have no clue what are you referring to to, but I am not opening any
attachments. I doubt other maintainers will.

Best regards,
Krzysztof

Re: [RFC PATCH 1/1] reset: add support the GPIO provider with #gpio-cells=3

[Marc Taylor]

[-- Attachment #1: Type: text/plain, Size: 4667 bytes --]

Hi Maksim,

(Re: <20260423230338.442497-2-bigunclemax@gmail.com>. I'm not
subscribed, so this may not thread; sorry. Adding linux-sunxi.)

I hit the same bug on an Allwinner H700 board (Anbernic RG35XX Plus,
sun50i-h700-anbernic-rg35xx-plus.dts: RTL8821CS SDIO Wi-Fi behind
mmc-pwrseq-simple with reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>) on
v7.2.15, and the code is unchanged in v7.4-rc1. With CONFIG_RESET_GPIO
enabled (arm64 defconfig has it =m):

pwrseq_simple pwrseq: error -ENOENT: reset control not ready
pwrseq_simple pwrseq: probe with driver pwrseq_simple failed with error -2

mmc@4021000 never probes, so there is no wlan0. With RESET_GPIO
disabled, pwrseq_simple falls back to plain GPIOs and Wi-Fi works.

Two notes on the RFC:

1. It no longer applies after the reset-gpio rework that landed in
v7.1/v7.2 (fwnode_reference_args, __reset_add_reset_gpio_device()
taking a fwnode, fwnode_reset_n_cells).

2. I think the 3-cell software node reference resolves to the wrong
line. gpiolib-swnode reads the reference with nargs = 2 and uses
args[0] as the chip offset and args[1] as the flags. With
<bank pin flags> stored in the swnode, PG18 would come out as
offset 6 (PA6) with flags 18. If the CB1 works with it, I suspect
the real reset line is simply left deasserted. (Code reading only;
I didn't run the RFC itself, since it doesn't apply here.)

Attached (and below) is a port to v7.2.15 that asks the provider's
of_xlate() for the chip offset, so the swnode stays <offset flags>, and
skips the reset-cells check for the GPIO fallback (reset-gpio says its
cell count "is not really used"). On the RG35XX Plus with
CONFIG_RESET_GPIO=y:

sunxi-mmc 4021000.mmc: allocated mmc-pwrseq
mmc2: new high speed SDIO card at address 0001
rtw88_8821cs mmc2:0001:1: Firmware version 24.11.0, H2C version 12

/sys/bus/auxiliary/devices/reset.gpio.0 exists, debugfs shows gpio-210
(PG18 = 6 * 32 + 18) requested as "reset", out hi ACTIVE LOW, and Wi-Fi
associates and gets a DHCP lease.

This isn't a formal submission. I'm happy to test a v2 of yours, or to
turn this into a proper patch if that's preferred. Philipp's earlier
suggestion (pwrseq_simple falling back to devm_gpiod_get_array() on
-ENOENT) would also fix this board.

For the approach below, on Anbernic RG35XX Plus (H700):
Tested-by: Marc Taylor <marctaylor700@gmail.com>

--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -1019,12 +1019,13 @@
int ret, prop = 0;

/*
- * Currently only #gpio-cells=2 is supported with the meaning of:
- * args[0]: GPIO number
- * args[1]: GPIO flags
- * TODO: Handle other cases.
+ * Supported specifiers:
+ * #gpio-cells = 2: <offset flags>
+ * #gpio-cells = 3: provider-specific (e.g. Allwinner <bank pin flags>),
+ * translated to a chip offset by the chip's of_xlate
+ * The flags are always the last cell.
*/
- if (args->nargs != 2)
+ if (args->nargs != 2 && args->nargs != 3)
return -ENOENT;

/*
@@ -1034,8 +1035,7 @@
*/
lockdep_assert_not_held(&reset_list_mutex);

- offset = args->args[0];
- flags = args->args[1];
+ flags = args->args[args->nargs - 1];

/*
* Later we map GPIO flags between OF and Linux, however not all
@@ -1046,8 +1046,7 @@
* flags.
*/
if (flags > GPIO_ACTIVE_LOW) {
- pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
- flags, offset);
+ pr_err("reset-gpio code does not support GPIO flags %u\n", flags);
return -EINVAL;
}

@@ -1055,6 +1054,33 @@
gpio_device_find_by_fwnode(args->fwnode);
if (!gdev)
return -EPROBE_DEFER;
+
+ if (args->nargs == 2) {
+ offset = args->args[0];
+ } else {
+ /*
+ * The software node built below is resolved by gpiolib-swnode,
+ * which only understands <offset flags>. Let the provider turn
+ * its own multi-cell specifier into a chip offset.
+ */
+ struct gpio_chip *gc = gpio_device_get_chip(gdev);
+ struct of_phandle_args spec = { };
+ u32 of_flags;
+ int i;
+
+ if (!gc || !gc->of_xlate || !is_of_node(args->fwnode))
+ return -ENOENT;
+
+ https://www.google.com/url?q=http://spec.np&source=gmail&ust=1793850302384000&sa=E
= to_of_node(args->fwnode);
+ spec.args_count = args->nargs;
+ for (i = 0; i < args->nargs; i++)
+ spec.args[i] = args->args[i];
+
+ ret = gc->of_xlate(gc, &spec, &of_flags);
+ if (ret < 0)
+ return ret;
+ offset = ret;
+ }

guard(mutex)(&reset_gpio_lookup_mutex);

@@ -1193,7 +1219,11 @@
goto out_put;
}

- if (WARN_ON(args.nargs != rcdev->fwnode_reset_n_cells)) {
+ /*
+ * For the reset-gpio fallback args is the GPIO specifier, whose cell
+ * count is the GPIO provider's, not the reset controller's.
+ */
+ if (!gpio_fallback && WARN_ON(args.nargs != rcdev->fwnode_reset_n_cells)) {
rstc = ERR_PTR(-EINVAL);
goto out_put;
}

Thanks,
Marc Taylor

[-- Attachment #2: reset-gpio-3cell-v7.2.15.patch --]
[-- Type: text/x-patch, Size: 2368 bytes --]

--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -1019,12 +1019,13 @@
 	int ret, prop = 0;
 
 	/*
-	 * Currently only #gpio-cells=2 is supported with the meaning of:
-	 * args[0]: GPIO number
-	 * args[1]: GPIO flags
-	 * TODO: Handle other cases.
+	 * Supported specifiers:
+	 *   #gpio-cells = 2: <offset flags>
+	 *   #gpio-cells = 3: provider-specific (e.g. Allwinner <bank pin flags>),
+	 *                    translated to a chip offset by the chip's of_xlate
+	 * The flags are always the last cell.
 	 */
-	if (args->nargs != 2)
+	if (args->nargs != 2 && args->nargs != 3)
 		return -ENOENT;
 
 	/*
@@ -1034,8 +1035,7 @@
 	 */
 	lockdep_assert_not_held(&reset_list_mutex);
 
-	offset = args->args[0];
-	flags = args->args[1];
+	flags = args->args[args->nargs - 1];
 
 	/*
 	 * Later we map GPIO flags between OF and Linux, however not all
@@ -1046,8 +1046,7 @@
 	 * flags.
 	 */
 	if (flags > GPIO_ACTIVE_LOW) {
-		pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
-		       flags, offset);
+		pr_err("reset-gpio code does not support GPIO flags %u\n", flags);
 		return -EINVAL;
 	}
 
@@ -1055,6 +1054,33 @@
 			gpio_device_find_by_fwnode(args->fwnode);
 	if (!gdev)
 		return -EPROBE_DEFER;
+
+	if (args->nargs == 2) {
+		offset = args->args[0];
+	} else {
+		/*
+		 * The software node built below is resolved by gpiolib-swnode,
+		 * which only understands <offset flags>. Let the provider turn
+		 * its own multi-cell specifier into a chip offset.
+		 */
+		struct gpio_chip *gc = gpio_device_get_chip(gdev);
+		struct of_phandle_args spec = { };
+		u32 of_flags;
+		int i;
+
+		if (!gc || !gc->of_xlate || !is_of_node(args->fwnode))
+			return -ENOENT;
+
+		spec.np = to_of_node(args->fwnode);
+		spec.args_count = args->nargs;
+		for (i = 0; i < args->nargs; i++)
+			spec.args[i] = args->args[i];
+
+		ret = gc->of_xlate(gc, &spec, &of_flags);
+		if (ret < 0)
+			return ret;
+		offset = ret;
+	}
 
 	guard(mutex)(&reset_gpio_lookup_mutex);
 
@@ -1193,7 +1219,11 @@
 		goto out_put;
 	}
 
-	if (WARN_ON(args.nargs != rcdev->fwnode_reset_n_cells)) {
+	/*
+	 * For the reset-gpio fallback args is the GPIO specifier, whose cell
+	 * count is the GPIO provider's, not the reset controller's.
+	 */
+	if (!gpio_fallback && WARN_ON(args.nargs != rcdev->fwnode_reset_n_cells)) {
 		rstc = ERR_PTR(-EINVAL);
 		goto out_put;
 	}

[RFC PATCH 0/1] reset: gpio: Add support for GPIO providers with #gpio-cells=3

[bigunclemax]

From: Maksim Kiselev <bigunclemax@gmail.com>

Hello everyone,

A little background.
I have a BigTreeTech CB1 board based on the Allwinner H616.
I decided to try upstream Linux v7.0 and ran into the following issue:

[    0.453297] pwrseq_simple wifi-pwrseq: error -ENOENT: reset control not ready
[    0.460472] pwrseq_simple wifi-pwrseq: probe with driver pwrseq_simple failed with error -2

This error prevents the WiFi chip from coming up.

I started investigating and traced it down to the function
__reset_add_reset_gpio_device() in drivers/reset/core.c:

/*
 * @args:	phandle to the GPIO provider with all the args like GPIO number
 */
static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
{
	struct property_entry properties[3] = { };
	unsigned int offset, of_flags, lflags;
	struct reset_gpio_lookup *rgpio_dev;
	struct device *parent;
	int id, ret, prop = 0;

	/*
	 * Currently only #gpio-cells=2 is supported with the meaning of:
	 * args[0]: GPIO number
	 * args[1]: GPIO flags
	 * TODO: Handle other cases.
	 */
	if (args->args_count != 2)
		return -ENOENT;

As you can see, a GPIO from controller where #gpio-cells != 2 will cause
an error.

Unfortunately, the Allwinner pinctrl is one such GPIO controller.
It uses three arguments to describe a GPIO line: bank, number, and flags.

Here’s the DT fragment that describes wifi_pwrseq in
arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-cb1.dtsi file:

wifi_pwrseq: wifi-pwrseq {
	compatible = "mmc-pwrseq-simple";
	clocks = <&rtc 1>;
	clock-names = "ext_clock";
	reset-gpios = <&pio 6 18 GPIO_ACTIVE_LOW>; /* PG18 */
	post-power-on-delay-ms = <200>;
};

Potentially this problem could also be observed on other GPIO controllers:
spacemit,k1-gpio, microchip,sparx5-sgpio, and many allwinner,___-pinctrl
variants.

I attempted to make a patch for reset/core.c (not pretty in my opinion)
that adds support for three-args GPIO phandles.
But it seems I’ve fallen down a rabbit hole, because next I hit the fact
that gpiolib-swnode expects GPIOs to be described with exactly
two arguments (swnode_gpio_get_reference()).

I also did a git bisect to find when wifi_pwrseq broke for
the BigTreeTech CB1. It turned out to be commit
73bf4b7381f7 ("mmc: pwrseq_simple: add support for one reset control")
in v6.13-rc1.

So, in theory, a patch to pwrseq_simple.c could be made to fix my issue,
but that wouldn’t solve the underlying reset-gpio problem.

I don’t know how to proceed from here and I’m asking for advice.

Best regards
Maksim

Maksim Kiselev (1):
  reset: add support the GPIO provider with #gpio-cells=3

 drivers/reset/core.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

-- 
2.51.0

[RFC PATCH 1/1] reset: add support the GPIO provider with #gpio-cells=3

[bigunclemax]

From: Maksim Kiselev <bigunclemax@gmail.com>

The __reset_add_reset_gpio_device() function currently expects a GPIO
provider with #gpio-cells=2, which prevents using the GPIO from
controllers with #gpio-cells=3, such as the Allwinner pinctrl and others.

Extend the parsing to support the GPIO phandle with three args,
where:
  args[0]: GPIO bank
  args[1]: GPIO number
  args[2]: GPIO flags

Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com>
---
 drivers/reset/core.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 352c2360603b..18fb50ca645f 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -868,7 +868,8 @@ static int reset_add_gpio_aux_device(struct device *parent,
 static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 {
 	struct property_entry properties[3] = { };
-	unsigned int offset, of_flags, lflags;
+	unsigned int offset[2], of_flags, lflags;
+	struct software_node_ref_args sw_ref[1];
 	struct reset_gpio_lookup *rgpio_dev;
 	struct device *parent;
 	int id, ret, prop = 0;
@@ -879,7 +880,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 * args[1]: GPIO flags
 	 * TODO: Handle other cases.
 	 */
-	if (args->args_count != 2)
+	if (args->args_count != 2 && args->args_count != 3)
 		return -ENOENT;
 
 	/*
@@ -889,8 +890,13 @@ 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];
+	offset[0] = args->args[0];
+	if (args->args_count == 2) {
+		of_flags = args->args[1];
+	} else {
+		offset[1] = args->args[1];
+		of_flags = args->args[2];
+	}
 
 	/*
 	 * Later we map GPIO flags between OF and Linux, however not all
@@ -902,7 +908,7 @@ static int __reset_add_reset_gpio_device(const struct of_phandle_args *args)
 	 */
 	if (of_flags > GPIO_ACTIVE_LOW) {
 		pr_err("reset-gpio code does not support GPIO flags %u for GPIO %u\n",
-		       of_flags, offset);
+		       of_flags, offset[0]);
 		return -EINVAL;
 	}
 
@@ -923,7 +929,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[prop++] = PROPERTY_ENTRY_STRING("compatible", "reset-gpio");
-	properties[prop++] = PROPERTY_ENTRY_GPIO("reset-gpios", parent->fwnode, offset, lflags);
+
+	if (args->args_count == 2)
+		sw_ref[0] = SOFTWARE_NODE_REFERENCE(parent->fwnode, offset[0], lflags);
+	else
+		sw_ref[0] = SOFTWARE_NODE_REFERENCE(parent->fwnode, offset[0], offset[1],
+						    lflags);
+	properties[prop++] = PROPERTY_ENTRY_REF_ARRAY("reset-gpios", sw_ref);
 
 	id = ida_alloc(&reset_gpio_ida, GFP_KERNEL);
 	if (id < 0)
@@ -1048,10 +1060,11 @@ __of_reset_control_get(struct device_node *node, const char *id, int index,
 		goto out_unlock;
 	}
 
-	if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
-		rstc = ERR_PTR(-EINVAL);
-		goto out_unlock;
-	}
+	if (!(rcdev->dev && fwnode_device_is_compatible(dev_fwnode(rcdev->dev), "reset-gpio")))
+		if (WARN_ON(args.args_count != rcdev->of_reset_n_cells)) {
+			rstc = ERR_PTR(-EINVAL);
+			goto out_unlock;
+		}
 
 	rstc_id = rcdev->of_xlate(rcdev, &args);
 	if (rstc_id < 0) {
-- 
2.51.0