S5P: Exynos: Add GPIO numbering feature

PATCH Superseded Akshay Saraswat 2 participants 2 messages Apr 25, 2014 – Apr 28, 2014

lore

Share & tools

lore

Akshay Saraswat

author

2014-04-25 15:31 UTC #0 lore raw

v9 cover S5P: Exynos: Add GPIO numbering feature

Hi Przemyslaw Marczak,

Quoted 86 lines

Hello,
Quoted 65 lines · Akshay Saraswat
Used a script to recheck/verify pin mapping and existing mapping appears to be fine, returning correct bank and pin values. Did ./MAKEALL -a arm and found all Exynos/S5P related boards compiled successfully.
Couldn't test booting over all of them.
Tested U-Boot bootup over SMDK5420, SMDK5250, Snow. Requesting maintaners to please test over affected SoCs/boards.
Changes in V2:
- Enabled CMD_GPIO as suggested by Simon Glass
and supported same for EXYNOS5.
Changes in V3:
- New patch added to rename S5P GPIO definitions
to S5P_GPIO.
- GPIO Table added to calculate the base address
of input gpio bank.
Changes in V4:
- To have consistent 0..n-1 GPIO numbering the banks are divided into different parts where ever they have holes in them.
- Function and table to support gpio command moved to s5p-gpio driver.
- Rebased on latest u-boot-samsung tree.
Changes in V5:
- Rebased on latest u-boot-samsung tree.
- Removed Exynos5 specific code in gpio driver api to get bank.
- Added #define HAVE_GENERIC_GPIO in config file
to remove conditinal CPU check in gpio driver.
Changes in V6:
- Isolated config changes in a new patch.
- Updated patches with corresponding changes for Exynos 5420.
Changes in V7:
- Added changes for other SoCs like Exynos 4412, 4210 etc.
Changes in V8:
- Changed Subject of patch 2/2 to reflect affected SoCs/boards.
- Fixed arndale board compile time errors introduced due to patch-set v7.
Changes in V9:
- Fixed checkpatch errors.
- Fixed naming error in exynosxxxx_gpio_data arrays which could be the possible reason behind data abort witnessed over Exynos4 boards.
Akshay Saraswat (2):
EXYNOS: Add GPIO pin numbering and rename definitions
S5P: Exynos: Config: Enable Generic GPIO and CMD configs
  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  | 1786 +++++++++++++++++++++++++-----
  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                  |  195 +++-
  include/configs/arndale.h                |    1 +
  include/configs/exynos5-dt.h             |    3 +
Quoted 9 lines
  include/configs/origen.h                 |    1 +
  include/configs/s5p_goni.h               |    5 +-
  include/configs/s5pc210_universal.h      |   17 +-
  include/configs/smdkc100.h               |    1 +
  include/configs/smdkv310.h               |    2 +
  include/configs/trats.h                  |    9 +-
  include/configs/trats2.h                 |    5 +-
  24 files changed, 2806 insertions(+), 857 deletions(-)
I think that you missed my last few comments.
You are still using exynos_gpio_get() - what for?
It returns some number and next it's interpreted as
gpio continuous number - but it isn't continuous.
It's because each samsung board has now defined "HAVE_GENERIC_GPIO", so gpio numbers are interpreted as linear.
We don't need additional macro since you introduce linear numbering.

Quoted 1 line

Please read my last comments again.

I have read your comments and replied to it. I am sorry may be I was not clear. Please look at the patch-set 2/2 again, I am not introducing I just replaced bank and pin number with sequential pin number. exynos_gpio_get() function was already there. I can't afford to remove it because I2C driver uses it and also I don't have boards to test outcomes of such changes.
In opinion it would be better if the maintainer of the board could do these changes.

Quoted 5 lines

I tried test this on trats2 device but data abort occurs at PMIC init, it's probably because exynos_gpio_get(). Please use just gpio numbering as you defined in this patchset.

As far as I could understand, exynos_gpio_get still returns what it used to before this patch because we still extract bank and pin number from linear number to do the needfull.
But I will definitely try to look for the bug. Thanks for the suggestion.

Quoted 7 lines

Thank you

Signature · 4 lines

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

Regards,
Akshay Saraswat

Przemyslaw Marczak

2014-04-28 09:47 UTC #1 lore raw

v9 cover S5P: Exynos: Add GPIO numbering feature

Hello Akshay,

Quoted 104 lines · Akshay Saraswat

Hi Przemyslaw Marczak,
Quoted 86 lines
Hello,
Quoted 65 lines · Akshay Saraswat
Used a script to recheck/verify pin mapping and existing mapping appears to be fine, returning correct bank and pin values. Did ./MAKEALL -a arm and found all Exynos/S5P related boards compiled successfully.
Couldn't test booting over all of them.
Tested U-Boot bootup over SMDK5420, SMDK5250, Snow. Requesting maintaners to please test over affected SoCs/boards.
Changes in V2:
- Enabled CMD_GPIO as suggested by Simon Glass
and supported same for EXYNOS5.
Changes in V3:
- New patch added to rename S5P GPIO definitions
to S5P_GPIO.
- GPIO Table added to calculate the base address
of input gpio bank.
Changes in V4:
- To have consistent 0..n-1 GPIO numbering the banks are divided into different parts where ever they have holes in them.
- Function and table to support gpio command moved to s5p-gpio driver.
- Rebased on latest u-boot-samsung tree.
Changes in V5:
- Rebased on latest u-boot-samsung tree.
- Removed Exynos5 specific code in gpio driver api to get bank.
- Added #define HAVE_GENERIC_GPIO in config file
to remove conditinal CPU check in gpio driver.
Changes in V6:
- Isolated config changes in a new patch.
- Updated patches with corresponding changes for Exynos 5420.
Changes in V7:
- Added changes for other SoCs like Exynos 4412, 4210 etc.
Changes in V8:
- Changed Subject of patch 2/2 to reflect affected SoCs/boards.
- Fixed arndale board compile time errors introduced due to patch-set v7.
Changes in V9:
- Fixed checkpatch errors.
- Fixed naming error in exynosxxxx_gpio_data arrays which could be the possible reason behind data abort witnessed over Exynos4 boards.
Akshay Saraswat (2):
EXYNOS: Add GPIO pin numbering and rename definitions
S5P: Exynos: Config: Enable Generic GPIO and CMD configs
   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  | 1786 +++++++++++++++++++++++++-----
   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                  |  195 +++-
   include/configs/arndale.h                |    1 +
   include/configs/exynos5-dt.h             |    3 +
Quoted 9 lines
   include/configs/origen.h                 |    1 +
   include/configs/s5p_goni.h               |    5 +-
   include/configs/s5pc210_universal.h      |   17 +-
   include/configs/smdkc100.h               |    1 +
   include/configs/smdkv310.h               |    2 +
   include/configs/trats.h                  |    9 +-
   include/configs/trats2.h                 |    5 +-
   24 files changed, 2806 insertions(+), 857 deletions(-)
I think that you missed my last few comments.
You are still using exynos_gpio_get() - what for?
It returns some number and next it's interpreted as
gpio continuous number - but it isn't continuous.
It's because each samsung board has now defined "HAVE_GENERIC_GPIO", so gpio numbers are interpreted as linear.
We don't need additional macro since you introduce linear numbering.
Quoted 1 line
Please read my last comments again.
I have read your comments and replied to it. I am sorry may be I was not clear. Please look at the patch-set 2/2 again, I am not introducing I just replaced bank and pin number with sequential pin number. exynos_gpio_get() function was already there. I can't afford to remove it because I2C driver uses it and also I don't have boards to test outcomes of such changes.
In opinion it would be better if the maintainer of the board could do these changes.
Quoted 5 lines
I tried test this on trats2 device but data abort occurs at PMIC init, it's probably because exynos_gpio_get(). Please use just gpio numbering as you defined in this patchset.

Quoted 5 lines

As far as I could understand, exynos_gpio_get still returns what it used to before this patch because we still extract bank and pin number from linear number to do the needfull.
But I will definitely try to look for the bug. Thanks for the suggestion.

If you replace every call like: "exynos_gpio_get(AAA, BBB)" with just "BBB" - then this code will work.

example for trats2.h:
---------------------------------------
exynos_gpio_get(2, EXYNOS4X12_GPIO_X22)
needs change to -> EXYNOS4X12_GPIO_X22
---------------------------------------
and one of functions in trats2.c:
---------------------------------------
int get_soft_i2c_scl_pin(void)

{
         if (I2C_ADAP_HWNR)

- return exynos_gpio_get(2, EXYNOS4X12_GPIO_M21);
+ return EXYNOS4X12_GPIO_M21;
else
- return exynos_gpio_get(1, EXYNOS4X12_GPIO_F14);
+ return EXYNOS4X12_GPIO_F14;
}
---------------------------------------

I2C will works fine because it uses common gpio calls like "gpio_get_value("BBB")"

You don't need to define GPIOs numbers by "exynos_gpio_get()" since you have defined each PIN directly like this: "EXYNOS4X12_GPIO_D00"

Even if exynos_gpio_get() returns a proper value code will fail because of this part of code:"
(driver/gpio/s5p_gpio.c)
#ifdef HAVE_GENERIC_GPIO (defined in every samsung config) struct s5p_gpio_bank *s5p_gpio_get_bank(unsigned int gpio)
...
...
and next this part of code:

         data = get_gpio_data();
         count = get_bank_num();
         for (i = upto = 0; i < count;
                         i++, upto = data->max_gpio, data++) {
                 debug("i=%d, upto=%d\n", i, upto);
                 if (gpio < data->max_gpio) {
		(This condition above probably will never met)
		..
		}
	}
	return NULL;

...
This probably will always return NULL because "gpio" returned by exynos_gpio_get() is a very big number(part mask -> 0xff000000).

So if you remove exynos_gpio_get() - it should works.

And also please check .dts files. Each gpio number from .dts you should decode by follow current gpio code and it should be replaced by a proper PIN number from gpio enum.

Gpio coding mask:
0x000000ff - pin number
0x00ffff00 - bank offset
0xff000000 - part number

Akshay, do you now understand what I mean?

Quoted 11 lines

Quoted 7 lines
Thank you
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com
Regards,
Akshay Saraswat

Thank you

Signature · 4 lines

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