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

[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