One more thing that perhaps seems more reasonable in general:
These OMAP_MAX_GPIO defines could go into the corresponding .../arch-omap*.h files, where the base addresses are defined, and the number of GPIOs is implicitly obvious. And we shall have no ugly #ifdefs in the GPIO driver.
Tom, what do you think?
Thanks, Lubo
On 21/06/13 10:29, Lubomir Popov wrote:
Hi Axel,
On 21/06/13 10:13, Axel Lin wrote:
2013/6/21 Lubomir Popov lpopov@mm-sol.com:
Hi Axel,
On 21/06/13 06:07, Axel Lin wrote:
AM33XX has 4 gpio banks, thus the valid gpio range should be 0 ... 127.
Signed-off-by: Axel Lin axel.lin@ingics.com
v2: define OMAP_MAX_GPIO and use it. This change is mainly based on Stefan's comment, however I use OMAP_MAX_GPIO instead of CONFIG_OMAP_MAX_GPIO because having CONFIG_ prefix seems meaning it can be configurable in configs.
drivers/gpio/omap_gpio.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c index a30d7f0..6fa57c9 100644 --- a/drivers/gpio/omap_gpio.c +++ b/drivers/gpio/omap_gpio.c @@ -40,6 +40,12 @@ #include <asm/io.h> #include <asm/errno.h>
+#if defined(CONFIG_AM33XX) +#define OMAP_MAX_GPIO 128 +#else +#define OMAP_MAX_GPIO 192 +#endif
Please be aware that OMAP54XX and DRA7XX SoCs have 8 banks per 32 GPIOs, that is, 256 in total. The DRA7xx config defines CONFIG_DRA7XX, but also includes omap5_common.h, where CONFIG_OMAP54XX is defined (due to sharing of many internal IPs with the OMAP5, including GPIO). The above definitions should be changed to something like:
#if defined(CONFIG_OMAP54XX) #define OMAP_MAX_GPIO 256 /* OMAP54XX and DRA7XX */
In arch/arm/cpu/armv7/omap5/hwinit.c we have below settings:
static struct gpio_bank gpio_bank_54xx[6] = { { (void *)OMAP54XX_GPIO1_BASE, METHOD_GPIO_24XX }, { (void *)OMAP54XX_GPIO2_BASE, METHOD_GPIO_24XX }, { (void *)OMAP54XX_GPIO3_BASE, METHOD_GPIO_24XX }, { (void *)OMAP54XX_GPIO4_BASE, METHOD_GPIO_24XX }, { (void *)OMAP54XX_GPIO5_BASE, METHOD_GPIO_24XX }, { (void *)OMAP54XX_GPIO6_BASE, METHOD_GPIO_24XX }, };
const struct gpio_bank *const omap_gpio_bank = gpio_bank_54xx;
So gpio_bank_54xx only has 6 entries rather than 8 entries. Seems need to fix this before setting OMAP_MAX_GPIO to 256.
Sure. File arch/arm/include/asm/arch-omap5/gpio.h shall need fixing as well, by adding:
#define OMAP54XX_GPIO7_BASE 0x48051000 #define OMAP54XX_GPIO8_BASE 0x48053000
I'm wondering if it's ok to have this patch as is, and then an incremental patch to set gpio_bank_54xx[] and OMAP_MAX_GPIO for OMAP54XX and DRA7XX.
Feel free to proceed as you wish. If you modify hwint.c and gpio.h, then the three patches could be combined in a series, e.g. "Fix valid GPIO range for OMAP".
Tom?
Regards, Axel
Thanks, Lubo