Re: [RFC PATCH 1/1] reset: add support the GPIO provider with #gpio-cells=3
From: Marc Taylor
Date: Tue Nov 03 2026 - 22:49:53 EST
Hi Maksim,
(Re: <20260423230338.442497-2-bigunclemax@xxxxxxxxx>. 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@xxxxxxxxx>
--- 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
--- 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;
}