Re: [PATCH RFC v5 4/6] ARM: pxa: Convert reset driver to GPIO descriptors

[Bartosz Golaszewski]

On Wed, Oct 4, 2023 at 4:56 PM Duje Mihanović <duje.mihanovic@xxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Duje Mihanović <duje.mihanovic@xxxxxxxx>
> ---
>  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(-)
>
> diff --git a/arch/arm/mach-pxa/reset.c b/arch/arm/mach-pxa/reset.c
> index 27293549f8ad..08bc104b9411 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", 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));

Crashing the kernel on a GPIO error? I guess it just keeps the old
behavior but still...

>
>         /* 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..26ec29c9cd1b 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", 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;
>
> --
> 2.42.0
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>