Re: [U-Boot] [PATCH v8 1/2] S5P: Exynos: Add GPIO pin numbering and rename definitions
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@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com Signed-off-by: Akshay Saraswat akshay.s@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.
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.
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@samsung.com
Regards, Akshay Saraswat
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@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com Signed-off-by: Akshay Saraswat akshay.s@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@samsung.com
Regards, Akshay Saraswat
Thank you
participants (2)
-
Akshay Saraswat -
Przemyslaw Marczak