[U-Boot] [PATCH 3/3 V2] EXYNOS5: GPIO: Enable GPIO Command for EXYNOS5

[Simon Glass]

Hi Rajeshwari,

On Wed, Jan 23, 2013 at 2:48 AM, Rajeshwari Shinde
<rajeshwari.s at samsung.com> wrote:
> This patch enables GPIO Command for EXYNOS5.
> Function has been added to asm/gpio.h to decode the
> input gpio name to gpio number.
> example: gpio set gpa00
> GPIO_INPUT in cmd_gpio.c has been modified to
> GPIO_DIRECTION_INPUT as GPIO_INPUT is alraedy defined in
> exynos5 and leading to a error.
>
> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>

Great to see this - some comments below.

> ---
> Chnages in V2:
>         - New patch
>  arch/arm/include/asm/arch-exynos/gpio.h |   88 +++++++++++++++++++++++++++++++
>  common/cmd_gpio.c                       |    6 +-
>  include/configs/exynos5250-dt.h         |    1 +
>  3 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-exynos/gpio.h b/arch/arm/include/asm/arch-exynos/gpio.h
> index af882dd..66e4a8e 100644
> --- a/arch/arm/include/asm/arch-exynos/gpio.h
> +++ b/arch/arm/include/asm/arch-exynos/gpio.h
> @@ -657,6 +657,94 @@ static inline unsigned int s5p_gpio_part_max(int nr)
>  void gpio_cfg_pin(int gpio, int cfg);
>  void gpio_set_pull(int gpio, int mode);
>  void gpio_set_drv(int gpio, int mode);
> +
> +#include <common.h>
> +
> +static inline int name_to_gpio(const char *name)
> +{
> +       int num;
> +
> +       name++;
> +
> +       if (*name == 'p') {
> +               ++name;
> +
> +               switch (*name) {
> +               case 'a':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_A00 + num;
> +                       break;
> +               case 'b':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_B00 + num;
> +                       break;
> +               case 'c':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 10);
> +                       if (num >= 40)
> +                               num = GPIO_C40 + (num - 40);
> +                       else {
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_C00 + num;
> +                       }
> +                       break;
> +               case 'd':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_D00 + num;
> +                       break;
> +               case 'y':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_Y00 + num;
> +                       break;
> +               case 'x':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_X00 + num;
> +                       break;
> +               case 'e':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_E00 + num;
> +                       break;
> +               case 'f':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_F00 + num;
> +                       break;
> +               case 'g':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_G00 + num;
> +                       break;
> +               case 'h':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_H00 + num;
> +                       break;
> +               case 'v':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_V00 + num;
> +                       break;
> +               case 'z':
> +                       name++;
> +                       num = simple_strtoul(name, NULL, 8);
> +                       num = GPIO_Z0 + num;
> +                       break;
> +               default:

It seems like you need a table (C struct) containing the GPIO letter
and the associated GPIO_... value. Then this code could become a for()
loop to search for the match. This would reduce code duplication. 'c'
would presumable still be a special case.

> +                       return -1;
> +               }
> +       } else
> +               return -1;
> +
> +       return num;
> +}
> +
> +#define name_to_gpio(n) name_to_gpio(n)
>  #endif
>
>  /* Pin configurations */
> diff --git a/common/cmd_gpio.c b/common/cmd_gpio.c
> index 47eee89..450e6d1 100644
> --- a/common/cmd_gpio.c
> +++ b/common/cmd_gpio.c
> @@ -16,7 +16,7 @@
>  #endif
>
>  enum gpio_cmd {
> -       GPIO_INPUT,
> +       GPIO_DIRECTION_INPUT,

Actually I think it is exynos that should change - perhaps put an
EXYNOS prefix on that one.

>         GPIO_SET,
>         GPIO_CLEAR,
>         GPIO_TOGGLE,
> @@ -44,7 +44,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>
>         /* parse the behavior */
>         switch (*str_cmd) {
> -               case 'i': sub_cmd = GPIO_INPUT;  break;
> +               case 'i': sub_cmd = GPIO_DIRECTION_INPUT;  break;
>                 case 's': sub_cmd = GPIO_SET;    break;
>                 case 'c': sub_cmd = GPIO_CLEAR;  break;
>                 case 't': sub_cmd = GPIO_TOGGLE; break;
> @@ -63,7 +63,7 @@ static int do_gpio(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>         }
>
>         /* finally, let's do it: set direction and exec command */
> -       if (sub_cmd == GPIO_INPUT) {
> +       if (sub_cmd == GPIO_DIRECTION_INPUT) {
>                 gpio_direction_input(gpio);
>                 value = gpio_get_value(gpio);
>         } else {
> diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
> index 7b9c393..b20b8f2 100644
> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -113,6 +113,7 @@
>  #define CONFIG_CMD_EXT2
>  #define CONFIG_CMD_FAT
>  #define CONFIG_CMD_NET
> +#define CONFIG_CMD_GPIO
>
>  #define CONFIG_BOOTDELAY               3
>  #define CONFIG_ZERO_BOOTDELAY_CHECK
> --
> 1.7.4.4
>

Can you also please implement gpio_info(), so it will list out the
GPIOs by name, and their state?

Regards,
Simon