| Message ID | 1505273322-26091-1-git-send-email-aford173@gmail.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,V2] dm: gpio: Add DM compatibilty to GPIO driver for Davinci | expand |
Hi Adam, On 12 September 2017 at 21:28, Adam Ford <aford173@gmail.com> wrote: > This adds DM compatibility for the davinici GPIO driver. > Tested on da850-evm. > > Signed-off-by: Adam Ford <aford173@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
On Tue, Sep 12, 2017 at 11:27 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Adam, > > On 12 September 2017 at 21:28, Adam Ford <aford173@gmail.com> wrote: >> This adds DM compatibility for the davinici GPIO driver. >> Tested on da850-evm. >> >> Signed-off-by: Adam Ford <aford173@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() Can you point me to an example board you want me to use? Several boards do it this way including omap_gpio.c, mxc_gpio.c, imx_rgpio2.c, and others. I used the omap_gpio.c file as a model for this since they are similar. > >> + >> + /* >> + * 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? > See the above comment. Some boards have a bind function while others use ofdata_to_platdata(). The readme shows two possible ways. I did it this way because the examples I followed did it this way. If it's good enough for them, why can't it be good enough this? >> + */ >> + 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
Hi Adam, On 17 September 2017 at 04:29, Adam Ford <aford173@gmail.com> wrote: > On Tue, Sep 12, 2017 at 11:27 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Adam, >> >> On 12 September 2017 at 21:28, Adam Ford <aford173@gmail.com> wrote: >>> This adds DM compatibility for the davinici GPIO driver. >>> Tested on da850-evm. >>> >>> Signed-off-by: Adam Ford <aford173@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(-) [...] >>> +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() > > Can you point me to an example board you want me to use? Several > boards do it this way including omap_gpio.c, mxc_gpio.c, imx_rgpio2.c, > and others. I used the omap_gpio.c file as a model for this since > they are similar. See for example rk_gpio which is a simple driver. One mode is tegra_gpio - that driver has an 'empty' parent device and then creates child GPIO devices in the bind() method. Another is omap_gpio - that uses platform data and U_BOOT_DEVICE() to create drivers (for some boards) rather than device tree. But here you don't see to be doing that. You just have a single GPIO device, right? If so, you should be able to put everything in ofdata_to_platdata(). If you set .platdata_auto_alloc_size it will automatically allocate the platform data. >> >>> + >>> + /* >>> + * 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? >> > > See the above comment. Some boards have a bind function while others > use ofdata_to_platdata(). The readme shows two possible ways. I did > it this way because the examples I followed did it this way. If it's > good enough for them, why can't it be good enough this? It is frustrating when you follow examples and they are not quite right. I will do a little series to tidy this stuff up a bit and cc you. Which readme are you referring to? Regards, Simon
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; + + /* + * 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. + */ + 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 */
This adds DM compatibility for the davinici GPIO driver. Tested on da850-evm. Signed-off-by: Adam Ford <aford173@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(-)