diff mbox series

[RFC,v4,4/6] ARM: pxa: Convert reset driver to GPIO descriptors

Message ID 20231001-pxa-gpio-v4-4-0f3b975e6ed5@skole.hr (mailing list archive)
State Superseded
Headers show
Series ARM: pxa: GPIO descriptor conversions | expand

Commit Message

Duje Mihanović Oct. 1, 2023, 2:12 p.m. UTC 
The PXA reset driver still uses the legacy GPIO interface for
configuring and asserting the reset pin.

Convert it to use the GPIO descriptor interface.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
 arch/arm/mach-pxa/reset.c | 39 +++++++++++++--------------------------
 arch/arm/mach-pxa/reset.h |  3 +--
 arch/arm/mach-pxa/spitz.c |  6 +++++-
 3 files changed, 19 insertions(+), 29 deletions(-)

Comments

Andy Shevchenko Oct. 1, 2023, 2:40 p.m. UTC | #1 
On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:
>
> The PXA reset driver still uses the legacy GPIO interface for
> configuring and asserting the reset pin.
>
> Convert it to use the GPIO descriptor interface.

> Acked-by: Linus Walleij <linus.walleij@linaro.org>

I dunno how.

...

> +       reset_gpio = gpiod_get(NULL, "reset generator", GPIOD_ASIS);
> +       if (IS_ERR(reset_gpio)) {
> +               pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
> +               return PTR_ERR(reset_gpio);
>         }

Here you asked for the GPIO named as "reset generator-gpio(s)" (The
"(s)" part is for new bindings), but you must not use spaces in the
GPIO names. Moreover the string literal there is for labeling, and not
for matching.

...

> +GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",

And here should be gpios. That's what you have to request, but because
of the global (device-less) nature of this, you have to be very
careful to avoid any clashes.

> +               SPITZ_GPIO_ON_RESET, "reset generator", GPIO_ACTIVE_HIGH);

...

TBH, I don't know how it is supposed to work with your current code
and if Linus really was okay with this.
Andy Shevchenko Oct. 1, 2023, 2:50 p.m. UTC | #2 
On Sun, Oct 1, 2023 at 5:40 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Oct 1, 2023 at 5:13 PM Duje Mihanović <duje.mihanovic@skole.hr> wrote:

...

> TBH, I don't know how it is supposed to work with your current code
> and if Linus really was okay with this.

Okay, it seems I have to refresh my memories about GPIO lookup tables.
First of all, we indeed require a connection ID just to match and no
matter if it has or hasn't the suffix.Second, the key is a label of
the GPIO controller according to the device driver (device tree?) and
pxa-gpio is that one. Seems it should work.
diff mbox series

Patch

diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
index 27293549f8ad..2bfa66f99555 100644
--- a/arch/arm/mach-pxa/reset.c
+++ b/arch/arm/mach-pxa/reset.c
@@ -2,7 +2,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/io.h>
 #include <asm/proc-fns.h>
 #include <asm/system_misc.h>
@@ -14,33 +14,20 @@ 
 
 static void do_hw_reset(void);
 
-static int reset_gpio = -1;
+static struct gpio_desc *reset_gpio;
 
-int init_gpio_reset(int gpio, int output, int level)
+int init_gpio_reset(int output, int level)
 {
-	int rc;
-
-	rc = gpio_request(gpio, "reset generator");
-	if (rc) {
-		printk(KERN_ERR "Can't request reset_gpio\n");
-		goto out;
+	reset_gpio = gpiod_get(NULL, "reset generator", GPIOD_ASIS);
+	if (IS_ERR(reset_gpio)) {
+		pr_err("Can't request reset_gpio: %pe\n", reset_gpio);
+		return PTR_ERR(reset_gpio);
 	}
 
 	if (output)
-		rc = gpio_direction_output(gpio, level);
+		return gpiod_direction_output(reset_gpio, level);
 	else
-		rc = gpio_direction_input(gpio);
-	if (rc) {
-		printk(KERN_ERR "Can't configure reset_gpio\n");
-		gpio_free(gpio);
-		goto out;
-	}
-
-out:
-	if (!rc)
-		reset_gpio = gpio;
-
-	return rc;
+		return gpiod_direction_input(reset_gpio);
 }
 
 /*
@@ -50,16 +37,16 @@  int init_gpio_reset(int gpio, int output, int level)
  */
 static void do_gpio_reset(void)
 {
-	BUG_ON(reset_gpio == -1);
+	BUG_ON(IS_ERR(reset_gpio));
 
 	/* drive it low */
-	gpio_direction_output(reset_gpio, 0);
+	gpiod_direction_output(reset_gpio, 0);
 	mdelay(2);
 	/* rising edge or drive high */
-	gpio_set_value(reset_gpio, 1);
+	gpiod_set_value(reset_gpio, 1);
 	mdelay(2);
 	/* falling edge */
-	gpio_set_value(reset_gpio, 0);
+	gpiod_set_value(reset_gpio, 0);
 
 	/* give it some time */
 	mdelay(10);
diff --git a/arch/arm/mach-pxa/reset.h b/arch/arm/mach-pxa/reset.h
index 963dd190bc13..5864f61a0e94 100644
--- a/arch/arm/mach-pxa/reset.h
+++ b/arch/arm/mach-pxa/reset.h
@@ -13,10 +13,9 @@  extern void pxa_register_wdt(unsigned int reset_status);
 
 /**
  * init_gpio_reset() - register GPIO as reset generator
- * @gpio: gpio nr
  * @output: set gpio as output instead of input during normal work
  * @level: output level
  */
-extern int init_gpio_reset(int gpio, int output, int level);
+extern int init_gpio_reset(int output, int level);
 
 #endif /* __ASM_ARCH_RESET_H */
diff --git a/arch/arm/mach-pxa/spitz.c b/arch/arm/mach-pxa/spitz.c
index 965354e64c68..701fba130ac4 100644
--- a/arch/arm/mach-pxa/spitz.c
+++ b/arch/arm/mach-pxa/spitz.c
@@ -1024,9 +1024,13 @@  static void spitz_restart(enum reboot_mode mode, const char *cmd)
 	spitz_poweroff();
 }
 
+GPIO_LOOKUP_SINGLE(spitz_reset_gpio_table, NULL, "pxa-gpio",
+		SPITZ_GPIO_ON_RESET, "reset generator", GPIO_ACTIVE_HIGH);
+
 static void __init spitz_init(void)
 {
-	init_gpio_reset(SPITZ_GPIO_ON_RESET, 1, 0);
+	gpiod_add_lookup_table(&spitz_reset_gpio_table);
+	init_gpio_reset(1, 0);
 	pm_power_off = spitz_poweroff;
 
 	PMCR = 0x00;