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
