Hi Peter, On 8 July 2015 at 09:57, Peter Griffin <peter.grif...@linaro.org> wrote: > This patch adds support for the GPIO perif found on hi6220 > SoC. > > Signed-off-by: Peter Griffin <peter.grif...@linaro.org> > --- > arch/arm/include/asm/arch-hi6220/gpio.h | 29 ++++++++++ > drivers/gpio/Makefile | 2 + > drivers/gpio/hi6220_gpio.c | 95 > +++++++++++++++++++++++++++++++++ > 3 files changed, 126 insertions(+) > create mode 100644 arch/arm/include/asm/arch-hi6220/gpio.h > create mode 100644 drivers/gpio/hi6220_gpio.c > > diff --git a/arch/arm/include/asm/arch-hi6220/gpio.h > b/arch/arm/include/asm/arch-hi6220/gpio.h > new file mode 100644 > index 0000000..98122a2 > --- /dev/null > +++ b/arch/arm/include/asm/arch-hi6220/gpio.h > @@ -0,0 +1,29 @@ > +/* > + * Copyright (C) 2015 Linaro > + * Peter Griffin <peter.grif...@linaro.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#ifndef _HI6220_GPIO_H_ > +#define _HI6220_GPIO_H_ > + > +#define HI6220_GPIO_BASE(bank) (((bank < 4) ? 0xf8011000 : \ > + 0xf7020000 - 0x4000) + (0x1000 * bank)) > + > +#define BIT(x) (1 << (x))
I thought we had this in U-Boot now but cannot find it. > + > +#define HI6220_GPIO_PER_BANK 8 > +#define HI6220_GPIO_DIR 0x400 > + > +struct gpio_bank { > + u8 *base; /* address of registers in physical memory */ > +}; We should use a C struct to access registers. See here: http://www.denx.de/wiki/U-Boot/CodingStyle > + > +/* Information about a GPIO bank */ > +struct hikey_gpio_platdata { > + int bank_index; > + unsigned int base; /* address of registers in physical memory */ > +}; > + > +#endif /* _HI6220_GPIO_H_ */ > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 5864850..b470bab 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -46,3 +46,5 @@ obj-$(CONFIG_LPC32XX_GPIO) += lpc32xx_gpio.o > obj-$(CONFIG_STM32_GPIO) += stm32_gpio.o > obj-$(CONFIG_ZYNQ_GPIO) += zynq_gpio.o > obj-$(CONFIG_VYBRID_GPIO) += vybrid_gpio.o > +obj-$(CONFIG_HIKEY_GPIO) += hi6220_gpio.o > + > diff --git a/drivers/gpio/hi6220_gpio.c b/drivers/gpio/hi6220_gpio.c > new file mode 100644 > index 0000000..3f41bff > --- /dev/null > +++ b/drivers/gpio/hi6220_gpio.c > @@ -0,0 +1,95 @@ > +/* > + * Copyright (C) 2015 Linaro > + * Peter Griffin <peter.grif...@linaro.org> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <asm/gpio.h> > +#include <asm/io.h> > +#include <errno.h> > + > +static int hi6220_gpio_direction_input(struct udevice *dev, unsigned int > gpio) > +{ > + struct gpio_bank *bank = dev_get_priv(dev); > + u8 data; > + > + data = readb(bank->base + HI6220_GPIO_DIR); > + data &= ~(1 << gpio); > + writeb(data, bank->base + HI6220_GPIO_DIR); Something like this: clrbits_8(®s->gpio_dir, 1 << gpio) > + > + return 0; > +} > + > +static int hi6220_gpio_set_value(struct udevice *dev, unsigned gpio, > + int value) > +{ > + struct gpio_bank *bank = dev_get_priv(dev); > + > + writeb(!!value << gpio, bank->base + (BIT(gpio + 2))); Ick, use struct access. > + return 0; > +} > + > +static int hi6220_gpio_direction_output(struct udevice *dev, unsigned gpio, > + int value) > +{ > + struct gpio_bank *bank = dev_get_priv(dev); > + u8 data; > + > + data = readb(bank->base + HI6220_GPIO_DIR); > + data |= 1 << gpio; > + writeb(data, bank->base + HI6220_GPIO_DIR); > + > + hi6220_gpio_set_value(dev, gpio, value); > + > + return 0; > +} > + > +static int hi6220_gpio_get_value(struct udevice *dev, unsigned gpio) > +{ > + struct gpio_bank *bank = dev_get_priv(dev); > + > + return !!readb(bank->base + (BIT(gpio + 2))); > +} > + > + > + > +static const struct dm_gpio_ops gpio_hi6220_ops = { > + .direction_input = hi6220_gpio_direction_input, > + .direction_output = hi6220_gpio_direction_output, > + .get_value = hi6220_gpio_get_value, > + .set_value = hi6220_gpio_set_value, > +}; > + > +static int hi6220_gpio_probe(struct udevice *dev) > +{ > + struct gpio_bank *bank = dev_get_priv(dev); > + struct hikey_gpio_platdata *plat = dev_get_platdata(dev); > + struct gpio_dev_priv *uc_priv = dev->uclass_priv; > + char name[18], *str; > + > + sprintf(name, "GPIO%d_", plat->bank_index); Could use just use 'GP' or 'P'? This seems quite long as a name. Also I don't think you want the %d. > + > + str = strdup(name); > + if (!str) > + return -ENOMEM; > + > + uc_priv->bank_name = str; > + uc_priv->gpio_count = HI6220_GPIO_PER_BANK; > + > + bank->base = (u8 *)plat->base; > + > + return 0; > +} > + > +U_BOOT_DRIVER(gpio_hi6220) = { > + .name = "gpio_hi6220", > + .id = UCLASS_GPIO, > + .ops = &gpio_hi6220_ops, > + .probe = hi6220_gpio_probe, > + .priv_auto_alloc_size = sizeof(struct gpio_bank), > +}; > + > + > -- > 1.9.1 > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot