[U-Boot] [PATCH V2] dm: gpio: Add DM compatibilty to GPIO driver for Davinci

Simon Glass sjg at chromium.org
Wed Sep 13 04:27:27 UTC 2017


Hi Adam,

On 12 September 2017 at 21:28, Adam Ford <aford173 at gmail.com> wrote:
> This adds DM compatibility for the davinici GPIO driver.
> Tested on da850-evm.
>
> Signed-off-by: Adam Ford <aford173 at gmail.com>
> ---
> V2:  The bank calculation needs to take into account the size of the struct
>      Whitespace fixes
>
>  arch/arm/mach-davinci/include/mach/gpio.h |  14 +-
>  board/davinci/da8xxevm/da850evm.c         |   2 +
>  configs/da850evm_defconfig                |   3 +-
>  drivers/gpio/da8xx_gpio.c                 | 208 +++++++++++++++++++++++++++---
>  include/configs/da850evm.h                |   1 +
>  5 files changed, 205 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/include/mach/gpio.h b/arch/arm/mach-davinci/include/mach/gpio.h
> index 7da0060..32cae3a 100644
> --- a/arch/arm/mach-davinci/include/mach/gpio.h
> +++ b/arch/arm/mach-davinci/include/mach/gpio.h
> @@ -40,7 +40,7 @@ struct davinci_gpio_bank {
>         unsigned int irq_num;
>         unsigned int irq_mask;
>         unsigned long *in_use;
> -       unsigned long base;
> +       struct davinci_gpio *base;
>  };
>
>  #define davinci_gpio_bank01 ((struct davinci_gpio *)DAVINCI_GPIO_BANK01)
> @@ -49,7 +49,9 @@ struct davinci_gpio_bank {
>  #define davinci_gpio_bank67 ((struct davinci_gpio *)DAVINCI_GPIO_BANK67)
>  #define davinci_gpio_bank8 ((struct davinci_gpio *)DAVINCI_GPIO_BANK8)
>
> +#ifndef CONFIG_DM_GPIO
>  #define gpio_status()          gpio_info()
> +#endif
>  #define GPIO_NAME_SIZE         20
>  #if defined(CONFIG_SOC_DM644X)
>  /* GPIO0 to GPIO53, omit the V3.3 volts one */
> @@ -64,4 +66,14 @@ struct davinci_gpio_bank {
>
>  void gpio_info(void);
>
> +#ifdef CONFIG_DM_GPIO
> +
> +/* Information about a GPIO bank */
> +struct davinci_gpio_platdata {
> +       int bank_index;
> +       ulong base;     /* address of registers in physical memory */
> +       const char *port_name;
> +};
> +#endif
> +
>  #endif
> diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c
> index 42bc0f4..2a54d0f 100644
> --- a/board/davinci/da8xxevm/da850evm.c
> +++ b/board/davinci/da8xxevm/da850evm.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <i2c.h>
>  #include <net.h>
>  #include <netdev.h>
> @@ -24,6 +25,7 @@
>  #include <linux/errno.h>
>  #include <hwconfig.h>
>  #include <asm/mach-types.h>
> +#include <asm/gpio.h>
>
>  #ifdef CONFIG_MMC_DAVINCI
>  #include <mmc.h>
> diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig
> index b966fa7..24507da 100644
> --- a/configs/da850evm_defconfig
> +++ b/configs/da850evm_defconfig
> @@ -20,14 +20,15 @@ CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="U-Boot > "
>  # CONFIG_CMD_IMLS is not set
>  CONFIG_CRC32_VERIFY=y
> +# CONFIG_CMD_EEPROM is not set
>  # CONFIG_CMD_FLASH is not set
> -# CONFIG_CMD_GPIO is not set
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_MTDPARTS=y
>  CONFIG_CMD_DIAG=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_ENV_IS_IN_SPI_FLASH=y
>  CONFIG_DM=y
> +CONFIG_DM_GPIO=y
>  CONFIG_DM_I2C=y
>  CONFIG_DM_I2C_COMPAT=y
>  CONFIG_I2C_SET_DEFAULT_BUS_NUM=y
> diff --git a/drivers/gpio/da8xx_gpio.c b/drivers/gpio/da8xx_gpio.c
> index fa3a394..b64f936 100644
> --- a/drivers/gpio/da8xx_gpio.c
> +++ b/drivers/gpio/da8xx_gpio.c
> @@ -8,16 +8,20 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
>  #include <asm/io.h>
>  #include <asm/gpio.h>
>  #include <asm/arch/hardware.h>
>  #include <asm/arch/davinci_misc.h>
>
> +#ifndef CONFIG_DM_GPIO
>  static struct gpio_registry {
>         int is_registered;
>         char name[GPIO_NAME_SIZE];
>  } gpio_registry[MAX_NUM_GPIOS];
>
> +
>  #if defined(CONFIG_SOC_DA8XX)
>  #define pinmux(x)       (&davinci_syscfg_regs->pinmux[x])
>
> @@ -334,42 +338,30 @@ int gpio_free(unsigned gpio)
>         /* Do not configure as input or change pin mux here */
>         return 0;
>  }
> +#endif
>
> -int gpio_direction_input(unsigned gpio)
> +static int _gpio_direction_output(struct davinci_gpio *bank, unsigned gpio, int value)
>  {
> -       struct davinci_gpio *bank;
> -
> -       bank = GPIO_BANK(gpio);
> -       setbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
> +       clrbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
> +       gpio_set_value(gpio, value);
>         return 0;
>  }
>
> -int gpio_direction_output(unsigned gpio, int value)
> +static int _gpio_direction_input(struct davinci_gpio *bank, unsigned gpio)
>  {
> -       struct davinci_gpio *bank;
> -
> -       bank = GPIO_BANK(gpio);
> -       clrbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
> -       gpio_set_value(gpio, value);
> +       setbits_le32(&bank->dir, 1U << GPIO_BIT(gpio));
>         return 0;
>  }
>
> -int gpio_get_value(unsigned gpio)
> +static int _gpio_get_value(struct davinci_gpio *bank, unsigned gpio)
>  {
> -       struct davinci_gpio *bank;
>         unsigned int ip;
> -
> -       bank = GPIO_BANK(gpio);
>         ip = in_le32(&bank->in_data) & (1U << GPIO_BIT(gpio));
>         return ip ? 1 : 0;
>  }
>
> -int gpio_set_value(unsigned gpio, int value)
> +static int _gpio_set_value(struct davinci_gpio *bank, unsigned gpio, int value)
>  {
> -       struct davinci_gpio *bank;
> -
> -       bank = GPIO_BANK(gpio);
> -
>         if (value)
>                 bank->set_data = 1U << GPIO_BIT(gpio);
>         else
> @@ -378,6 +370,13 @@ int gpio_set_value(unsigned gpio, int value)
>         return 0;
>  }
>
> +static int _gpio_get_dir(struct davinci_gpio *bank, unsigned gpio)
> +{
> +       return in_le32(&bank->dir) & (1U << GPIO_BIT(gpio));
> +}
> +
> +#ifndef CONFIG_DM_GPIO
> +
>  void gpio_info(void)
>  {
>         unsigned gpio, dir, val;
> @@ -385,7 +384,8 @@ void gpio_info(void)
>
>         for (gpio = 0; gpio < MAX_NUM_GPIOS; ++gpio) {
>                 bank = GPIO_BANK(gpio);
> -               dir = in_le32(&bank->dir) & (1U << GPIO_BIT(gpio));
> +               printf("gpio_info: Bank=%x\n", (int) bank);
> +               dir = _gpio_get_dir(bank, gpio);
>                 val = gpio_get_value(gpio);
>
>                 printf("% 4d: %s: %d [%c] %s\n",
> @@ -394,3 +394,169 @@ void gpio_info(void)
>                         gpio_registry[gpio].name);
>         }
>  }
> +
> +int gpio_direction_input(unsigned gpio)
> +{
> +       struct davinci_gpio *bank;
> +
> +       bank = GPIO_BANK(gpio);
> +       return _gpio_direction_input(bank, gpio);
> +}
> +
> +int gpio_direction_output(unsigned gpio, int value)
> +{
> +       struct davinci_gpio *bank;
> +
> +       bank = GPIO_BANK(gpio);
> +       return _gpio_direction_output(bank, gpio, value);
> +}
> +
> +int gpio_get_value(unsigned gpio)
> +{
> +       struct davinci_gpio *bank;
> +
> +       bank = GPIO_BANK(gpio);
> +       return _gpio_get_value(bank, gpio);
> +}
> +
> +int gpio_set_value(unsigned gpio, int value)
> +{
> +       struct davinci_gpio *bank;
> +
> +       bank = GPIO_BANK(gpio);
> +       return _gpio_set_value(bank, gpio, value);
> +}
> +
> +#else /* CONFIG_DM_GPIO */
> +/* set GPIO pin 'gpio' as an input */
> +
> +static struct davinci_gpio *davinci_get_gpio_bank(struct udevice *dev, unsigned offset)
> +{
> +       struct davinci_gpio_bank *bank = dev_get_priv(dev);
> +
> +       /* The device tree is not broken into banks but the infrastructure is
> +        * expecting it this way, so we'll first include the 0x10 offset, then
> +        * calculate the bank manually based on the offset.
> +        */
> +
> +       return ((struct davinci_gpio *)bank->base) + 0x10 +
> +                       ((offset >> 5) * sizeof(struct davinci_gpio));
> +}
> +
> +static int davinci_gpio_direction_input(struct udevice *dev, unsigned offset)
> +{
> +       struct davinci_gpio *base = davinci_get_gpio_bank(dev, offset);
> +
> +       _gpio_direction_input(base, offset);
> +       return 0;
> +}
> +
> +/* set GPIO pin 'gpio' as an output, with polarity 'value' */
> +static int davinci_gpio_direction_output(struct udevice *dev, unsigned offset,
> +                                      int value)
> +{
> +       struct davinci_gpio *base = davinci_get_gpio_bank(dev, offset);
> +
> +       _gpio_direction_output(base, offset, value);
> +       return 0;
> +}
> +
> +/* read GPIO IN value of pin 'gpio' */
> +static int davinci_gpio_get_value(struct udevice *dev, unsigned offset)
> +{
> +       struct davinci_gpio *base = davinci_get_gpio_bank(dev, offset);
> +
> +       return _gpio_get_value(base, offset);
> +}
> +
> +/* write GPIO OUT value to pin 'gpio' */
> +static int davinci_gpio_set_value(struct udevice *dev, unsigned offset,
> +                                int value)
> +{
> +       struct davinci_gpio *base = davinci_get_gpio_bank(dev, offset);
> +
> +       _gpio_set_value(base, offset, value);
> +
> +       return 0;
> +}
> +
> +static int davinci_gpio_get_function(struct udevice *dev, unsigned offset)
> +{
> +       unsigned int dir;
> +       struct davinci_gpio *base = davinci_get_gpio_bank(dev, offset);
> +
> +       dir = _gpio_get_dir(base, offset);
> +
> +       if (dir)
> +               return GPIOF_INPUT;
> +
> +       return GPIOF_OUTPUT;
> +}
> +
> +static const struct dm_gpio_ops gpio_davinci_ops = {
> +       .direction_input        = davinci_gpio_direction_input,
> +       .direction_output       = davinci_gpio_direction_output,
> +       .get_value              = davinci_gpio_get_value,
> +       .set_value              = davinci_gpio_set_value,
> +       .get_function           = davinci_gpio_get_function,
> +};
> +
> +static int davinci_gpio_probe(struct udevice *dev)
> +{
> +       struct davinci_gpio_bank *bank = dev_get_priv(dev);
> +       struct davinci_gpio_platdata *plat = dev_get_platdata(dev);
> +       struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +       const void *fdt = gd->fdt_blob;
> +       int node = dev_of_offset(dev);
> +
> +       uc_priv->bank_name = plat->port_name;
> +       uc_priv->gpio_count = fdtdec_get_int(fdt, node, "ti,ngpio", -1);
> +       bank->base = (struct davinci_gpio *)plat->base;
> +       return 0;
> +}
> +
> +static int davinci_gpio_bind(struct udevice *dev)
> +{
> +       struct davinci_gpio_platdata *plat = dev->platdata;
> +       fdt_addr_t base_addr;
> +
> +       if (plat)
> +               return 0;
> +
> +       base_addr = devfdt_get_addr(dev);
> +       if (base_addr == FDT_ADDR_T_NONE)
> +               return -ENODEV;

-EINVAL. There is definitely a device.

Also we should not be reading the DT in the bind() method. This should
happen in ofdata_to_platdata()

> +
> +       /*
> +       * TODO:
> +       * When every board is converted to driver model and DT is
> +       * supported, this can be done by auto-alloc feature, but
> +       * not using calloc to alloc memory for platdata.

I don't really get this because we are in a driver-model method here.
Can we not use the plat data here?

> +       */
> +       plat = calloc(1, sizeof(*plat));
> +       if (!plat)
> +               return -ENOMEM;
> +
> +       plat->base = base_addr;
> +       plat->port_name = fdt_get_name(gd->fdt_blob, dev_of_offset(dev), NULL);
> +       dev->platdata = plat;
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id davinci_gpio_ids[] = {
> +       { .compatible = "ti,dm6441-gpio" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(gpio_davinci) = {
> +       .name   = "gpio_davinci",
> +       .id     = UCLASS_GPIO,
> +       .ops    = &gpio_davinci_ops,
> +       .of_match = davinci_gpio_ids,
> +       .bind   = davinci_gpio_bind,
> +       .probe  = davinci_gpio_probe,
> +       .priv_auto_alloc_size = sizeof(struct davinci_gpio_bank),
> +};
> +
> +#endif
> diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
> index ab2e6ae..ea8c441 100644
> --- a/include/configs/da850evm.h
> +++ b/include/configs/da850evm.h
> @@ -262,6 +262,7 @@
>  #define CONFIG_ENV_SECT_SIZE           (64 << 10)
>  #endif
>
> +#define CONFIG_DA8XX_GPIO
>  /*
>   * U-Boot general configuration
>   */
> --
> 2.7.4

Regards,
Simon


More information about the U-Boot mailing list