[U-Boot] [PATCH v2] gpio: omap_gpio: Fix valid gpio range for AM33XX

[Lubomir Popov]

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 at 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 at 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
>