[U-Boot] [PATCH 2/2] gpio: tegra: use named constants

[Simon Glass]

Hi Stephen.

On Friday, 25 September 2015, Stephen Warren <swarren at wwwdotorg.org> wrote:
>
> From: Stephen Warren <swarren at nvidia.com>
>
> In order to make it clear what the parameters to set_config() and
> set_direction() mean, and similarly for the return values from the
> respective get_*(), define named constants for these values.
>
> Disassembly shows no diff in the generated code, except that the
> order of the code in the branches of tegra_gpio_get_function() gets
> modified without affecting behaviour.
>
> Suggested-by: Tom Warren <twarren at nvidia.com>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
>  drivers/gpio/tegra_gpio.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
> index f9c06fe88b71..8e880e276f0a 100644
> --- a/drivers/gpio/tegra_gpio.c
> +++ b/drivers/gpio/tegra_gpio.c
> @@ -1,6 +1,6 @@
>  /*
>   * NVIDIA Tegra20 GPIO handling.
> - *  (C) Copyright 2010-2012
> + *  (C) Copyright 2010-2012,2015
>   *  NVIDIA Corporation <www.nvidia.com>
>   *
>   * SPDX-License-Identifier:    GPL-2.0+
> @@ -25,6 +25,11 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +static const int CONFIG_SFIO = 0;
> +static const int CONFIG_GPIO = 1;
> +static const int DIRECTION_INPUT = 0;
> +static const int DIRECTION_OUTPUT = 1;
> +


Why not use an enum?

>
>  struct tegra_gpio_platdata {
>         struct gpio_ctlr_bank *bank;
>         const char *port_name;  /* Name of port, e.g. "B" */
> @@ -37,7 +42,7 @@ struct tegra_port_info {
>         int base_gpio;          /* Port number for this port (0, 1,.., n-1) */
>  };
>
> -/* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */
> +/* Return config of pin 'gpio' as GPIO (1) or SFIO (0) */
>  static int get_config(unsigned gpio)
>  {
>         struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
> @@ -46,15 +51,15 @@ static int get_config(unsigned gpio)
>         int type;
>
>         u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
> -       type =  (u >> GPIO_BIT(gpio)) & 1;
> +       type = (u >> GPIO_BIT(gpio)) & 1;
>
>         debug("get_config: port = %d, bit = %d is %s\n",
>                 GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
>
> -       return type;
> +       return type ? CONFIG_GPIO : CONFIG_SFIO;
>  }
>
> -/* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */
> +/* Config pin 'gpio' as GPIO or SFIO, based on 'type' */
>  static void set_config(unsigned gpio, int type)
>  {
>         struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
> @@ -65,7 +70,7 @@ static void set_config(unsigned gpio, int type)
>                 GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
>
>         u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
> -       if (type)                               /* GPIO */
> +       if (type != CONFIG_SFIO)
>                 u |= 1 << GPIO_BIT(gpio);
>         else
>                 u &= ~(1 << GPIO_BIT(gpio));
> @@ -86,7 +91,7 @@ static int get_direction(unsigned gpio)
>         debug("get_direction: port = %d, bit = %d, %s\n",
>                 GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN");
>
> -       return dir;
> +       return dir ? DIRECTION_OUTPUT : DIRECTION_INPUT;
>  }
>
>  /* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */
> @@ -100,7 +105,7 @@ static void set_direction(unsigned gpio, int output)
>                 GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN");
>
>         u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]);
> -       if (output)
> +       if (output != DIRECTION_INPUT)
>                 u |= 1 << GPIO_BIT(gpio);
>         else
>                 u &= ~(1 << GPIO_BIT(gpio));
> @@ -135,7 +140,7 @@ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset)
>         struct tegra_port_info *state = dev_get_priv(dev);
>
>         /* Configure GPIO direction as input. */
> -       set_direction(state->base_gpio + offset, 0);
> +       set_direction(state->base_gpio + offset, DIRECTION_INPUT);
>
>         /* Enable the pin as a GPIO */
>         set_config(state->base_gpio + offset, 1);
> @@ -154,7 +159,7 @@ static int tegra_gpio_direction_output(struct udevice *dev, unsigned offset,
>         set_level(gpio, value);
>
>         /* Configure GPIO direction as output. */
> -       set_direction(gpio, 1);
> +       set_direction(gpio, DIRECTION_OUTPUT);
>
>         /* Enable the pin as a GPIO */
>         set_config(state->base_gpio + offset, 1);
> @@ -199,18 +204,18 @@ void gpio_config_table(const struct tegra_gpio_config *config, int len)
>         for (i = 0; i < len; i++) {
>                 switch (config[i].init) {
>                 case TEGRA_GPIO_INIT_IN:
> -                       set_direction(config[i].gpio, 0);
> +                       set_direction(config[i].gpio, DIRECTION_INPUT);
>                         break;
>                 case TEGRA_GPIO_INIT_OUT0:
>                         set_level(config[i].gpio, 0);
> -                       set_direction(config[i].gpio, 1);
> +                       set_direction(config[i].gpio, DIRECTION_OUTPUT);
>                         break;
>                 case TEGRA_GPIO_INIT_OUT1:
>                         set_level(config[i].gpio, 1);
> -                       set_direction(config[i].gpio, 1);
> +                       set_direction(config[i].gpio, DIRECTION_OUTPUT);
>                         break;
>                 }
> -               set_config(config[i].gpio, 1);
> +               set_config(config[i].gpio, CONFIG_GPIO);
>         }
>  }
>
> --
> 1.9.1
>
Regards,
Simon