Hi Marek,
Thanks for reviewing.
This patch adds support for the GPIO perif found on hi6220 SoC.
Signed-off-by: Peter Griffin peter.griffin@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.griffin@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.
If I drop the cast I get lots of
include/asm/arch/gpio.h:11:32: warning: initialization makes pointer from integer without a cast
compiler warnings.
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))
Yes good idea, I will do it like you suggest in V2. Currently I have
#define HI6220_GPIO_BASE(bank) (void *)(((bank < 4) ? 0xf8011000 : \ 0xf7020000 - 0x4000) + (0x1000 * bank))
To avoid the warnings mentioned above.
<snip>
+#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 ?
They are currently used in the hikey.c board file which defines the hikey_gpio_platdata structure.
Although thinking about it some more this should be moved from the hikey board file to the driver as it is SoC specific rather than board specific.
+#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 ?
Will fix in v2.
regards,
Peter.