[U-Boot] [PATCH v8 1/2] S5P: Exynos: Add GPIO pin numbering and rename definitions

[Przemyslaw Marczak]

Dear Akshay,

On 04/16/2014 03:20 PM, Akshay Saraswat wrote:
> Thanks for testing.
> Please read inline replies.
>
>> Hello Akshay,
>>   04/16/2014 07:32 AM, Akshay Saraswat wrote:
>>> This patch includes following changes :
>>> * Adds gpio pin numbering support for EXYNOS SOCs.
>>>     To have consistent 0..n-1 GPIO numbering the banks are divided
>>>     into different parts where ever they have holes in them.
>>>
>>> * Rename GPIO definitions from GPIO_... to S5P_GPIO_...
>>>     These changes were done to enable cmd_gpio for EXYNOS and
>>>     cmd_gpio has GPIO_INPUT same as s5p_gpio driver and hence
>>>     getting a error during compilation.
>>>
>>> * Adds support for name to gpio conversion in s5p_gpio to enable
>>>     gpio command EXYNOS SoCs. Function has been added to asm/gpio.h
>>>     to decode the input gpio name to gpio number.
>>>     Example: SMDK5420 # gpio set gpa00
>>>
>>> Signed-off-by: Leela Krishna Amudala <l.krishna at samsung.com>
>>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s at samsung.com>
>>> Signed-off-by: Akshay Saraswat <akshay.s at samsung.com>
>>> ---
>>>    arch/arm/cpu/armv7/exynos/pinmux.c       |  403 +++----
>>>    arch/arm/include/asm/arch-exynos/cpu.h   |   17 +-
>>>    arch/arm/include/asm/arch-exynos/gpio.h  | 1778 +++++++++++++++++++++++++-----
>>>    arch/arm/include/asm/arch-s5pc1xx/gpio.h |  941 +++++++++++++---
>>>    board/samsung/arndale/arndale.c          |   11 +-
>>>    board/samsung/goni/goni.c                |   26 +-
>>>    board/samsung/smdk5250/exynos5-dt.c      |   20 +-
>>>    board/samsung/smdk5250/smdk5250.c        |   19 +-
>>>    board/samsung/smdk5420/smdk5420.c        |   15 +-
>>>    board/samsung/smdkc100/smdkc100.c        |    5 +-
>>>    board/samsung/smdkv310/smdkv310.c        |   17 +-
>>>    board/samsung/trats/trats.c              |   39 +-
>>>    board/samsung/trats2/trats2.c            |   74 +-
>>>    board/samsung/universal_c210/universal.c |   51 +-
>>>    drivers/gpio/s5p_gpio.c                  |  194 +++-
>>>    15 files changed, 2779 insertions(+), 831 deletions(-)
>>>
>>>
>>
>> You introduced new method for gpio numbering and I don't know why are
>> you using macro like this:
>> "exynos_gpio_get(2, EXYNOS4X12_GPIO_X33)"
>> and also new style like only "EXYNOS4X12_GPIO_X33".
>>
>
> Even I was wondering why such a macro lies in config file.
> But i2c driver is looking for it, so had to modify and keep it.
>

Current macros like exynos4x12_gpio_get(part, bank, pin) returns some 
masked number with masks:
0x000000ff - pin number
0x00ffff00 - bank offset
0xff000000 - part number

so as you can see this is not an linear numbering and this can't be just 
replaced with your gpio numbering method.

The previous gpio numbering method was broken because it was returning 
bad addresses for some gpios. Current method is simple and is just working.

Your idea with gpio enum is good, but the returned banks addresses are 
probably bad for some reasons.

>>
>> Those are two different numbers and can't be given to s5p_gpio driver
>> which interprets only new gpio numbering method.
>> This code can't work properly.
>>
>
> Since, sequential pin number to bank+pin generation is same everywhere,
> I thought it's ok to just replace it. Probably the enum mapping has faults.
> Need to recalculate.
>

At current macros "bank" is a direct address offset of proper gpio part 
base address - it's not a sequential number. Look at this macro:

>>
>> Please remove old macros and use only proper gpio enum in boards config
>> files.
>> We don't need any "get" macro if all gpio numbers are defined explicitly.
>>
>> You should also modify proper dts files in arch/arm/dts/* .
>>
>>
>> I tested your changes with enums only and after change proper dts file
>> for trats2 there is still data abort.
>> So this means that probably enums are not mapped correctly to proper
>> gpio bank addresses.
>>
>> Do you have an documentation for Exynos4 socs?
>>
>> I think that this should be checked again with documentation and
>> compiled next to the current code which is simple and just works.
>>
>> There are also some warnings when checking by checkpatch script.
>>
>
> Sure, will fix it. :-)
>
>>
>> Thanks
>> --
>> Przemyslaw Marczak
>> Samsung R&D Institute Poland
>> Samsung Electronics
>> p.marczak at samsung.com
>>
>
> Regards,
> Akshay Saraswat
>

Thank you
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com