On Tuesday, May 12, 2015 at 03:38:28 PM, Peter Griffin wrote:
Hi!
> 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-armv8/gpio.h | 47 +++++++++++++++++
> drivers/gpio/Makefile | 2 +
> drivers/gpio/hi6220_gpio.c | 95
> ++++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+)
> create mode 100644 arch/arm/include/asm/arch-armv8/gpio.h
> create mode 100644 drivers/gpio/hi6220_gpio.c
>
> diff --git a/arch/arm/include/asm/arch-armv8/gpio.h
> b/arch/arm/include/asm/arch-armv8/gpio.h new file mode 100644
> index 0000000..162c2d9
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armv8/gpio.h
> @@ -0,0 +1,47 @@
> +/*
> + * 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_GPIO0_BASE (void *)0xf8011000
You should drop the explicit cast, that's nasty.
Also, why don't you define this as a HI6220_GPIO_BASE(bank) instead?
That'd trim down the number of macros and from what I see, it should
be rather easy to do ...
#define HI6220_GPIO_BASE(bank)
(((bank < 4) ? 0xf8012000 : (0xf7020000 - 0x4000)) + (0x1000 * bank))
> +#define HI6220_GPIO1_BASE (void *)0xf8012000
> +#define HI6220_GPIO2_BASE (void *)0xf8013000
> +#define HI6220_GPIO3_BASE (void *)0xf8014000
> +#define HI6220_GPIO4_BASE (void *)0xf7020000
> +#define HI6220_GPIO5_BASE (void *)0xf7021000
> +#define HI6220_GPIO6_BASE (void *)0xf7022000
> +#define HI6220_GPIO7_BASE (void *)0xf7023000
> +#define HI6220_GPIO8_BASE (void *)0xf7024000
> +#define HI6220_GPIO9_BASE (void *)0xf7025000
> +#define HI6220_GPIO10_BASE (void *)0xf7026000
> +#define HI6220_GPIO11_BASE (void *)0xf7027000
> +#define HI6220_GPIO12_BASE (void *)0xf7028000
> +#define HI6220_GPIO13_BASE (void *)0xf7029000
> +#define HI6220_GPIO14_BASE (void *)0xf702a000
> +#define HI6220_GPIO15_BASE (void *)0xf702b000
> +#define HI6220_GPIO16_BASE (void *)0xf702c000
> +#define HI6220_GPIO17_BASE (void *)0xf702d000
> +#define HI6220_GPIO18_BASE (void *)0xf702e000
> +#define HI6220_GPIO19_BASE (void *)0xf702f000
But are these even used in the driver anywhere ?
> +#define BIT(x) (1 << (x))
This macro should be placed into common header files.
> +#define HI6220_GPIO_PER_BANK 8
> +#define HI6220_GPIO_DIR 0x400
> +
> +struct gpio_bank {
> + u8 *base; /* address of registers in physical memory */
Should be void __iomem *, no ?
> +};
> +
> +/* Information about a GPIO bank */
> +struct hikey_gpio_platdata {
> + int bank_index;
> + void *base; /* address of registers in physical memory */
> +};
> +
> +#endif /* _HI6220_GPIO_H_ */
[...]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
