Skip to contentSkip to conversation

ARM: EXYNOS: support GPIO for EXYNOS5250

PATCH Superseded Kukjin Kim 6 participants 11 messages Jan 31, 2012 – Feb 9, 2012

Subsystems (4)
Arm (32-bit) Arm sub-architectures GPIO Arm Samsung Exynos

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Kukjin Kim

author
#0 lore raw

cover ARM: EXYNOS: support GPIO for EXYNOS5250

This patch adds support GPIOlib for EXYNOS5250 SoC.

Note: this patch depends on supporting EXYNOS5250 patches and will be updated when EXYNOS5 DT is supported.

[PATCH 1/2] ARM: EXYNOS: add support GPIO for EXYNOS5250 [PATCH 2/2] gpio/samsung: add support GPIOlib for EXYNOS5250

author
#1 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>
---
 arch/arm/mach-exynos/common.c                |   20 +++
 arch/arm/mach-exynos/include/mach/gpio.h     |  221 ++++++++++++++++++++------
 arch/arm/mach-exynos/include/mach/map.h      |    4 +
 arch/arm/plat-samsung/include/plat/map-s5p.h |    1 +
 4 files changed, 199 insertions(+), 47 deletions(-)
arch/arm/mach-exynos/common.c +20 -0
index 6ab3c5a..225acc7 100644
@@ -271,6 +271,26 @@ static struct map_desc exynos5_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(EXYNOS5_PA_GIC_DIST),
 		.length		= SZ_64K,
 		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO1,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO1),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO2,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO2),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO3,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO3),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO4,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO4),
+		.length		= SZ_256,
+		.type		= MT_DEVICE,
 	},
 };
arch/arm/mach-exynos/include/mach/gpio.h +174 -47

243 lines not on this page · raw message

arch/arm/mach-exynos/include/mach/map.h +4 -0
index 300ed7e..b157eb4 100644
@@ -107,6 +107,10 @@
 #define EXYNOS4_PA_GPIO1		0x11400000
 #define EXYNOS4_PA_GPIO2		0x11000000
 #define EXYNOS4_PA_GPIO3		0x03860000
+#define EXYNOS5_PA_GPIO1		0x11400000
+#define EXYNOS5_PA_GPIO2		0x13400000
+#define EXYNOS5_PA_GPIO3		0x10D10000
+#define EXYNOS5_PA_GPIO4		0x03680000
 
 #define EXYNOS4_PA_MIPI_CSIS0		0x11880000
 #define EXYNOS4_PA_MIPI_CSIS1		0x11890000
arch/arm/plat-samsung/include/plat/map-s5p.h +1 -0
index c2d7bda..01e8d83 100644
@@ -20,6 +20,7 @@
 #define S5P_VA_GPIO1		S5P_VA_GPIO
 #define S5P_VA_GPIO2		S3C_ADDR(0x02240000)
 #define S5P_VA_GPIO3		S3C_ADDR(0x02280000)
+#define S5P_VA_GPIO4		S3C_ADDR(0x022C0000)
 
 #define S5P_VA_SYSRAM		S3C_ADDR(0x02400000)
 #define S5P_VA_DMC0		S3C_ADDR(0x02440000)
Signature · 1 line
1.7.4.4
#2 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Hi Kukjin,

I have few comments below...

Quoted 3 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.

s/follwing/following.

nit: AFAIK it's a good habit not to start a commit description with "This patch.."

Quoted 135 lines

- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>
---
 arch/arm/mach-exynos/common.c                |   20 +++
 arch/arm/mach-exynos/include/mach/gpio.h     |  221 ++++++++++++++++++++------
 arch/arm/mach-exynos/include/mach/map.h      |    4 +
 arch/arm/plat-samsung/include/plat/map-s5p.h |    1 +
 4 files changed, 199 insertions(+), 47 deletions(-)
arch/arm/mach-exynos/common.c +20 -0
index 6ab3c5a..225acc7 100644
@@ -271,6 +271,26 @@ static struct map_desc exynos5_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(EXYNOS5_PA_GIC_DIST),
 		.length		= SZ_64K,
 		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO1,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO1),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO2,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO2),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO3,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO3),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GPIO4,
+		.pfn		= __phys_to_pfn(EXYNOS5_PA_GPIO4),
+		.length		= SZ_256,
+		.type		= MT_DEVICE,
 	},
 };
arch/arm/mach-exynos/include/mach/gpio.h +32 -42
index 80523ca..a9c3944 100644
@@ -13,8 +13,11 @@
 #ifndef __ASM_ARCH_GPIO_H
 #define __ASM_ARCH_GPIO_H __FILE__
 
-/* Practically, GPIO banks up to GPZ are the configurable gpio banks */
+/* MACRO for EXYNOS GPIO numbering */
+#define EXYNOS_GPIO_NEXT(__gpio) \
+	((__gpio##_START) + (__gpio##_NR) + CONFIG_S3C_GPIO_SPACE + 1)
 
+/* EXYNOS4 serise */
 /* GPIO bank sizes */
 #define EXYNOS4_GPIO_A0_NR	(8)
 #define EXYNOS4_GPIO_A1_NR	(6)

@@ -55,51 +58,47 @@
 #define EXYNOS4_GPIO_Z_NR	(7)
 
 /* GPIO bank numbers */
-
-#define EXYNOS4_GPIO_NEXT(__gpio) \
-	((__gpio##_START) + (__gpio##_NR) + CONFIG_S3C_GPIO_SPACE + 1)
-
-enum s5p_gpio_number {
+enum exynos4_gpio_number {
 	EXYNOS4_GPIO_A0_START	= 0,
-	EXYNOS4_GPIO_A1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_A0),
-	EXYNOS4_GPIO_B_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_A1),
-	EXYNOS4_GPIO_C0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_B),
-	EXYNOS4_GPIO_C1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_C0),
-	EXYNOS4_GPIO_D0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_C1),
-	EXYNOS4_GPIO_D1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_D0),
-	EXYNOS4_GPIO_E0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_D1),
-	EXYNOS4_GPIO_E1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_E0),
-	EXYNOS4_GPIO_E2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_E1),
-	EXYNOS4_GPIO_E3_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_E2),
-	EXYNOS4_GPIO_E4_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_E3),
-	EXYNOS4_GPIO_F0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_E4),
-	EXYNOS4_GPIO_F1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_F0),
-	EXYNOS4_GPIO_F2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_F1),
-	EXYNOS4_GPIO_F3_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_F2),
-	EXYNOS4_GPIO_J0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_F3),
-	EXYNOS4_GPIO_J1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_J0),
-	EXYNOS4_GPIO_K0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_J1),
-	EXYNOS4_GPIO_K1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_K0),
-	EXYNOS4_GPIO_K2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_K1),
-	EXYNOS4_GPIO_K3_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_K2),
-	EXYNOS4_GPIO_L0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_K3),
-	EXYNOS4_GPIO_L1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_L0),
-	EXYNOS4_GPIO_L2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_L1),
-	EXYNOS4_GPIO_X0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_L2),
-	EXYNOS4_GPIO_X1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_X0),
-	EXYNOS4_GPIO_X2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_X1),
-	EXYNOS4_GPIO_X3_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_X2),
-	EXYNOS4_GPIO_Y0_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_X3),
-	EXYNOS4_GPIO_Y1_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y0),
-	EXYNOS4_GPIO_Y2_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y1),
-	EXYNOS4_GPIO_Y3_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y2),
-	EXYNOS4_GPIO_Y4_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y3),
-	EXYNOS4_GPIO_Y5_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y4),
-	EXYNOS4_GPIO_Y6_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y5),
-	EXYNOS4_GPIO_Z_START	= EXYNOS4_GPIO_NEXT(EXYNOS4_GPIO_Y6),
+	EXYNOS4_GPIO_A1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_A0),
+	EXYNOS4_GPIO_B_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_A1),
+	EXYNOS4_GPIO_C0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_B),
+	EXYNOS4_GPIO_C1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_C0),
+	EXYNOS4_GPIO_D0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_C1),
+	EXYNOS4_GPIO_D1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_D0),
+	EXYNOS4_GPIO_E0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_D1),
+	EXYNOS4_GPIO_E1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_E0),
+	EXYNOS4_GPIO_E2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_E1),
+	EXYNOS4_GPIO_E3_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_E2),
+	EXYNOS4_GPIO_E4_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_E3),
+	EXYNOS4_GPIO_F0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_E4),
+	EXYNOS4_GPIO_F1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_F0),
+	EXYNOS4_GPIO_F2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_F1),
+	EXYNOS4_GPIO_F3_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_F2),
+	EXYNOS4_GPIO_J0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_F3),
+	EXYNOS4_GPIO_J1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_J0),
+	EXYNOS4_GPIO_K0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_J1),
+	EXYNOS4_GPIO_K1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_K0),
+	EXYNOS4_GPIO_K2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_K1),
+	EXYNOS4_GPIO_K3_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_K2),
+	EXYNOS4_GPIO_L0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_K3),
+	EXYNOS4_GPIO_L1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_L0),
+	EXYNOS4_GPIO_L2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_L1),
+	EXYNOS4_GPIO_X0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_L2),
+	EXYNOS4_GPIO_X1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_X0),
+	EXYNOS4_GPIO_X2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_X1),

arch/arm/mach-exynos/include/mach/gpio.h line 99

Quoted 10 lines
arch/arm/mach-exynos/include/mach/gpio.h +9 -0
+	EXYNOS4_GPIO_X3_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_X2),
+	EXYNOS4_GPIO_Y0_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_X3),
+	EXYNOS4_GPIO_Y1_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y0),
+	EXYNOS4_GPIO_Y2_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y1),
+	EXYNOS4_GPIO_Y3_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y2),
+	EXYNOS4_GPIO_Y4_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y3),
+	EXYNOS4_GPIO_Y5_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y4),
+	EXYNOS4_GPIO_Y6_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y5),
+	EXYNOS4_GPIO_Z_START	= EXYNOS_GPIO_NEXT(EXYNOS4_GPIO_Y6),
 };

Are you planning to add support for exynos4212/exynos4412 as well anytime soon ? I've done some work towards this and I'd like to avoid duplicating efforts.

Quoted 7 lines
arch/arm/mach-exynos/include/mach/gpio.h +1 -1
-/* EXYNOS4 GPIO number definitions */
+/* GPIO number definitions */
 #define EXYNOS4_GPA0(_nr)	(EXYNOS4_GPIO_A0_START + (_nr))
 #define EXYNOS4_GPA1(_nr)	(EXYNOS4_GPIO_A1_START + (_nr))
 #define EXYNOS4_GPB(_nr)	(EXYNOS4_GPIO_B_START + (_nr))

@@ -140,10 +139,138 @@ enum s5p_gpio_number {

arch/arm/mach-exynos/include/mach/gpio.h enum s5p_gpio_number { · line 145

Quoted 10 lines
arch/arm/mach-exynos/include/mach/gpio.h +3 -4
 /* the end of the EXYNOS4 specific gpios */
 #define EXYNOS4_GPIO_END	(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) + 1)
-#define S3C_GPIO_END		EXYNOS4_GPIO_END
 
-/* define the number of gpios we need to the one after the GPZ() range */
-#define ARCH_NR_GPIOS		(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) +	\
-				 CONFIG_SAMSUNG_GPIO_EXTRA + 1)
+/* EXYNOS5 serise */
+/* GPIO bank sizes */
+#define EXYNOS5_GPIO_A0_NR	(8)

nit: It's been always a mystery to me, what are the parentheses around the numbers helpful for ? IMHO even if there is more things like this in the file it might be better to skip extra parentheses here.

Quoted 28 lines
arch/arm/mach-exynos/include/mach/gpio.h +28 -0
+#define EXYNOS5_GPIO_A1_NR	(6)
+#define EXYNOS5_GPIO_A2_NR	(8)
+#define EXYNOS5_GPIO_B0_NR	(5)
+#define EXYNOS5_GPIO_B1_NR	(5)
+#define EXYNOS5_GPIO_B2_NR	(4)
+#define EXYNOS5_GPIO_B3_NR	(4)
+#define EXYNOS5_GPIO_C0_NR	(7)
+#define EXYNOS5_GPIO_C1_NR	(7)
+#define EXYNOS5_GPIO_C2_NR	(7)
+#define EXYNOS5_GPIO_C3_NR	(7)
+#define EXYNOS5_GPIO_D0_NR	(8)
+#define EXYNOS5_GPIO_D1_NR	(8)
+#define EXYNOS5_GPIO_Y0_NR	(6)
+#define EXYNOS5_GPIO_Y1_NR	(4)
+#define EXYNOS5_GPIO_Y2_NR	(6)
+#define EXYNOS5_GPIO_Y3_NR	(8)
+#define EXYNOS5_GPIO_Y4_NR	(8)
+#define EXYNOS5_GPIO_Y5_NR	(8)
+#define EXYNOS5_GPIO_Y6_NR	(8)
+#define EXYNOS5_GPIO_X0_NR	(8)
+#define EXYNOS5_GPIO_X1_NR	(8)
+#define EXYNOS5_GPIO_X2_NR	(8)
+#define EXYNOS5_GPIO_X3_NR	(8)
+#define EXYNOS5_GPIO_E0_NR	(8)
+#define EXYNOS5_GPIO_E1_NR	(2)
+#define EXYNOS5_GPIO_F0_NR	(4)
+#define EXYNOS5_GPIO_F1_NR	(4)
+#define EXYNOS5_GPIO_G0_NR	(8)

arch/arm/mach-exynos/include/mach/gpio.h enum s5p_gpio_number { · line 183

Quoted 10 lines
arch/arm/mach-exynos/include/mach/gpio.h +10 -0
+#define EXYNOS5_GPIO_G1_NR	(8)
+#define EXYNOS5_GPIO_G2_NR	(2)
+#define EXYNOS5_GPIO_H0_NR	(4)
+#define EXYNOS5_GPIO_H1_NR	(8)
+#define EXYNOS5_GPIO_V0_NR	(8)
+#define EXYNOS5_GPIO_V1_NR	(8)
+#define EXYNOS5_GPIO_V2_NR	(8)
+#define EXYNOS5_GPIO_V3_NR	(8)
+#define EXYNOS5_GPIO_V4_NR	(2)
+#define EXYNOS5_GPIO_Z_NR	(7)

Thanks,

Signature · 2 lines
Sylwester Nawrocki
Samsung Poland R&D Center
#3 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Quoted 5 lines · Sylwester Nawrocki

Hi Kukjin,

I have few comments below...

On 01/31/2012 04:50 PM, Kukjin Kim wrote:

Quoted 9 lines
Quoted 3 lines

From: Sangsu Park <[email protected]>

This patch adds follwing.

s/follwing/following.

nit: AFAIK it's a good habit not to start a commit description with "This patch.."

"This patch..." descriptions are just fine. I write a lot of patches that way, but I do expect description of not just what a patch does, but why the patch is necessary.

Quoted 11 lines
Quoted 11 lines
arch/arm/mach-exynos/include/mach/gpio.h +1 -2
-/* EXYNOS4 GPIO number definitions */
+/* GPIO number definitions */
 #define EXYNOS4_GPA0(_nr)	(EXYNOS4_GPIO_A0_START + (_nr))
 #define EXYNOS4_GPA1(_nr)	(EXYNOS4_GPIO_A1_START + (_nr))
 #define EXYNOS4_GPB(_nr)	(EXYNOS4_GPIO_B_START + (_nr))

@@ -140,10 +139,138 @@ enum s5p_gpio_number {
 
 /* the end of the EXYNOS4 specific gpios */
 #define EXYNOS4_GPIO_END	(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) + 1)
-#define S3C_GPIO_END		EXYNOS4_GPIO_END
Quoted 10 lines

arch/arm/mach-exynos/include/mach/gpio.h enum s5p_gpio_number { · line 145

Quoted 6 lines
arch/arm/mach-exynos/include/mach/gpio.h +3 -3
-/* define the number of gpios we need to the one after the GPZ() range */
-#define ARCH_NR_GPIOS		(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) +	\
-				 CONFIG_SAMSUNG_GPIO_EXTRA + 1)
+/* EXYNOS5 serise */
+/* GPIO bank sizes */
+#define EXYNOS5_GPIO_A0_NR	(8)

nit: It's been always a mystery to me, what are the parentheses around the numbers helpful for ? IMHO even if there is more things like this in the file it might be better to skip extra parentheses here.

It protects against the preprocessor combining a macro with other code in unpredictable ways. For example:

#define SIZE 10 + 20
int i = SIZE * 5;

Without the parenthesis the result of i is 110, when the programmer would expect 150.

For single integers like these, the parenthesis aren't actually necessary, but I given that for every other #define it is good practice, I don't object to seeing them on single integers also.

g.

#4 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Quoted 15 lines · Grant Likely
Quoted 4 lines
Quoted 4 lines

/* the end of the EXYNOS4 specific gpios */
#define EXYNOS4_GPIO_END (EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) + 1)
-#define S3C_GPIO_END EXYNOS4_GPIO_END

Quoted 10 lines

arch/arm/mach-exynos/include/mach/gpio.h enum s5p_gpio_number { · line 145

Quoted 6 lines
arch/arm/mach-exynos/include/mach/gpio.h +3 -3
-/* define the number of gpios we need to the one after the GPZ() range */
-#define ARCH_NR_GPIOS		(EXYNOS4_GPZ(EXYNOS4_GPIO_Z_NR) +	\
-				 CONFIG_SAMSUNG_GPIO_EXTRA + 1)
+/* EXYNOS5 serise */
+/* GPIO bank sizes */
+#define EXYNOS5_GPIO_A0_NR	(8)

nit: It's been always a mystery to me, what are the parentheses around the numbers helpful for ? IMHO even if there is more things like this in the file it might be better to skip extra parentheses here.

Quoted 8 lines

It protects against the preprocessor combining a macro with other code in unpredictable ways. For example:

#define SIZE 10 + 20
int i = SIZE * 5;

Without the parenthesis the result of i is 110, when the programmer would expect 150.

Right, I guess it's a fundamental requirement most people are aware about. Nevertheless my point were only single integers.

Quoted 3 lines

For single integers like these, the parenthesis aren't actually necessary, but I given that for every other #define it is good practice, I don't object to seeing them on single integers also.

I respect that but I have a different opinion. :-) Those parentheses have always been bugging me, they decrease readability for virtually no benefit. They're more an aesthetic issue though so I wouldn't argue more about it. Just will try to get used, and I'll avoid them where possible. :-)

--
Regards,
Sylwester

#5 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

In reply to Kukjin Kim
Quoted 9 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>

Do you actually need these static mapping definitions? The samsung gpiolib initialization is called from a core_initcall(), and at this time ioremap() is fully capable of working.

#6 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Quoted 8 lines · Russell King - ARM Linux
Quoted 7 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Quoted 6 lines
Quoted 2 lines
Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>

Do you actually need these static mapping definitions? The samsung gpiolib initialization is called from a core_initcall(), and at this time ioremap() is fully capable of working.

If it assumes it has 8 gpios, you can make it simple calculate it like this.

enum exynos5_gpios {
     EXYNOS5_GPIO_A,
     ...
};

EXYNOS5_GPIO_A_START(n) (EXYNOS5_GPIO_A * 8)

with this one, it can make a simple gpio driver when using irq domain for GPIO.

Thank you,
Kyungmin Park

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#7 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Quoted 13 lines · Kyungmin Park
Quoted 11 lines · Russell King - ARM Linux
Quoted 9 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>
Quoted 7 lines
Quoted 3 lines

Do you actually need these static mapping definitions? The samsung gpiolib initialization is called from a core_initcall(), and at this time ioremap() is fully capable of working.

If it assumes it has 8 gpios, you can make it simple calculate it like this.

But unfortunately, it has not 8 gpios in each gpio bank.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Quoted 10 lines
enum exynos5_gpios {
     EXYNOS5_GPIO_A,
     ...
};

EXYNOS5_GPIO_A_START(n) (EXYNOS5_GPIO_A * 8)

with this one, it can make a simple gpio driver when using irq domain for GPIO.

#8 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

In reply to Russell King - ARM Linux
Quoted 8 lines · Russell King - ARM Linux
Quoted 7 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Quoted 6 lines
Quoted 2 lines
Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>

Do you actually need these static mapping definitions? The samsung gpiolib initialization is called from a core_initcall(), and at this time ioremap() is fully capable of working.

Indeed. Please get rid of the S5P_VA_GPIO* macros and use ioremap to set up the registers mappings. I understand that the existing code already does that, but I don't want to see any more added. Also, it is easy to fix by adding the ioremap calls to samsung_gpio_init()

g.

author
#9 lore raw

PATCH 1/2 ARM: EXYNOS: add support GPIO for EXYNOS5250

Quoted 13 lines · Grant Likely
Quoted 11 lines · Russell King - ARM Linux
Quoted 9 lines · Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds follwing.
- IO-map for EXYNOS5250 GPIO support
- EXYNOS5250 GPIO bank size/number definitions
- memory map definition for S5P GPIO4

Signed-off-by Sangsu Park <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>
Quoted 9 lines
Quoted 3 lines

Do you actually need these static mapping definitions? The samsung gpiolib initialization is called from a core_initcall(), and at this time ioremap() is fully capable of working.

Indeed. Please get rid of the S5P_VA_GPIO* macros and use ioremap to set up the registers mappings. I understand that the existing code already does that, but I don't want to see any more added. Also, it is easy to fix by adding the ioremap calls to samsung_gpio_init()

Yes, I agree. Will be updated with ioremap().

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

author
#10 lore raw

PATCH 2/2 gpio/samsung: add support GPIOlib for EXYNOS5250

In reply to Kukjin Kim

From: Sangsu Park <[email protected]>

This patch adds gpio_chips for EXYNOS5250 and replaces exynos4_xxx() with exynos_xxx() and variables to support exynos4 and exynos5 together.

Signed-off-by Sangsu Park <[email protected]>
Cc Grant Likely <[email protected]>
Signed-off-by Kukjin Kim <[email protected]>
---
 drivers/gpio/gpio-samsung.c |  356 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 343 insertions(+), 13 deletions(-)
drivers/gpio/gpio-samsung.c +343 -13
index a766177..f81f567 100644
@@ -169,7 +169,7 @@ int s3c24xx_gpio_setpull_1down(struct samsung_gpio_chip *chip,
 	return s3c24xx_gpio_setpull_1(chip, off, pull, S3C_GPIO_PULL_DOWN);
 }
 
-static int exynos4_gpio_setpull(struct samsung_gpio_chip *chip,
+static int exynos_gpio_setpull(struct samsung_gpio_chip *chip,
 				unsigned int off, samsung_gpio_pull_t pull)
 {
 	if (pull == S3C_GPIO_PULL_UP)

@@ -178,7 +178,7 @@ static int exynos4_gpio_setpull(struct samsung_gpio_chip *chip,
 	return samsung_gpio_setpull_updown(chip, off, pull);
 }
 
-static samsung_gpio_pull_t exynos4_gpio_getpull(struct samsung_gpio_chip *chip,
+static samsung_gpio_pull_t exynos_gpio_getpull(struct samsung_gpio_chip *chip,
 						unsigned int off)
 {
 	samsung_gpio_pull_t pull;

@@ -452,9 +452,9 @@ static struct samsung_gpio_cfg s3c24xx_gpiocfg_banka = {
 };
 #endif
 
-static struct samsung_gpio_cfg exynos4_gpio_cfg = {
-	.set_pull	= exynos4_gpio_setpull,
-	.get_pull	= exynos4_gpio_getpull,
+static struct samsung_gpio_cfg exynos_gpio_cfg = {
+	.set_pull	= exynos_gpio_setpull,
+	.get_pull	= exynos_gpio_getpull,
 	.set_config	= samsung_gpio_setcfg_4bit,
 	.get_config	= samsung_gpio_getcfg_4bit,
 };

@@ -502,13 +502,13 @@ static struct samsung_gpio_cfg samsung_gpio_cfgs[] = {
 		.get_config	= samsung_gpio_getcfg_2bit,
 	},
 	[8] = {
-		.set_pull	= exynos4_gpio_setpull,
-		.get_pull	= exynos4_gpio_getpull,
+		.set_pull	= exynos_gpio_setpull,
+		.get_pull	= exynos_gpio_getpull,
 	},
 	[9] = {
 		.cfg_eint	= 0x3,
-		.set_pull	= exynos4_gpio_setpull,
-		.get_pull	= exynos4_gpio_getpull,
+		.set_pull	= exynos_gpio_setpull,
+		.get_pull	= exynos_gpio_getpull,
 	}
 };
 

@@ -2113,7 +2113,7 @@ static struct samsung_gpio_chip s5pv210_gpios_4bit[] = {
 };
 
 /*
- * Followings are the gpio banks in EXYNOS4210
+ * Followings are the gpio banks in EXYNOS
  *
  * The 'config' member when left to NULL, is initialized to the default
  * structure samsung_gpio_cfgs[3] in the init function below.

@@ -2386,6 +2386,282 @@ static struct samsung_gpio_chip exynos4_gpios_3[] = {
 #endif
 };
 
+static struct samsung_gpio_chip exynos5_gpios_1[] = {
+#ifdef CONFIG_ARCH_EXYNOS5
+	{
+		.chip	= {
+			.base	= EXYNOS5_GPA0(0),
+			.ngpio	= EXYNOS5_GPIO_A0_NR,
+			.label	= "GPA0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPA1(0),
+			.ngpio	= EXYNOS5_GPIO_A1_NR,
+			.label	= "GPA1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPA2(0),
+			.ngpio	= EXYNOS5_GPIO_A2_NR,
+			.label	= "GPA2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPB0(0),
+			.ngpio	= EXYNOS5_GPIO_B0_NR,
+			.label	= "GPB0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPB1(0),
+			.ngpio	= EXYNOS5_GPIO_B1_NR,
+			.label	= "GPB1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPB2(0),
+			.ngpio	= EXYNOS5_GPIO_B2_NR,
+			.label	= "GPB2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPB3(0),
+			.ngpio	= EXYNOS5_GPIO_B3_NR,
+			.label	= "GPB3",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPC0(0),
+			.ngpio	= EXYNOS5_GPIO_C0_NR,
+			.label	= "GPC0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPC1(0),
+			.ngpio	= EXYNOS5_GPIO_C1_NR,
+			.label	= "GPC1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPC2(0),
+			.ngpio	= EXYNOS5_GPIO_C2_NR,
+			.label	= "GPC2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPC3(0),
+			.ngpio	= EXYNOS5_GPIO_C3_NR,
+			.label	= "GPC3",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPD0(0),
+			.ngpio	= EXYNOS5_GPIO_D0_NR,
+			.label	= "GPD0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPD1(0),
+			.ngpio	= EXYNOS5_GPIO_D1_NR,
+			.label	= "GPD1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY0(0),
+			.ngpio	= EXYNOS5_GPIO_Y0_NR,
+			.label	= "GPY0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY1(0),
+			.ngpio	= EXYNOS5_GPIO_Y1_NR,
+			.label	= "GPY1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY2(0),
+			.ngpio	= EXYNOS5_GPIO_Y2_NR,
+			.label	= "GPY2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY3(0),
+			.ngpio	= EXYNOS5_GPIO_Y3_NR,
+			.label	= "GPY3",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY4(0),
+			.ngpio	= EXYNOS5_GPIO_Y4_NR,
+			.label	= "GPY4",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY5(0),
+			.ngpio	= EXYNOS5_GPIO_Y5_NR,
+			.label	= "GPY5",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPY6(0),
+			.ngpio	= EXYNOS5_GPIO_Y6_NR,
+			.label	= "GPY6",
+		},
+	}, {
+		.base	= (S5P_VA_GPIO1 + 0xC00),
+		.config	= &samsung_gpio_cfgs[9],
+		.irq_base = IRQ_EINT(0),
+		.chip	= {
+			.base	= EXYNOS5_GPX0(0),
+			.ngpio	= EXYNOS5_GPIO_X0_NR,
+			.label	= "GPX0",
+			.to_irq	= samsung_gpiolib_to_irq,
+		},
+	}, {
+		.base	= (S5P_VA_GPIO1 + 0xC20),
+		.config	= &samsung_gpio_cfgs[9],
+		.irq_base = IRQ_EINT(8),
+		.chip	= {
+			.base	= EXYNOS5_GPX1(0),
+			.ngpio	= EXYNOS5_GPIO_X1_NR,
+			.label	= "GPX1",
+			.to_irq	= samsung_gpiolib_to_irq,
+		},
+	}, {
+		.base	= (S5P_VA_GPIO1 + 0xC40),
+		.config	= &samsung_gpio_cfgs[9],
+		.irq_base = IRQ_EINT(16),
+		.chip	= {
+			.base	= EXYNOS5_GPX2(0),
+			.ngpio	= EXYNOS5_GPIO_X2_NR,
+			.label	= "GPX2",
+			.to_irq	= samsung_gpiolib_to_irq,
+		},
+	}, {
+		.base	= (S5P_VA_GPIO1 + 0xC60),
+		.config	= &samsung_gpio_cfgs[9],
+		.irq_base = IRQ_EINT(24),
+		.chip	= {
+			.base	= EXYNOS5_GPX3(0),
+			.ngpio	= EXYNOS5_GPIO_X3_NR,
+			.label	= "GPX3",
+			.to_irq	= samsung_gpiolib_to_irq,
+		},
+	},
+#endif
+};
+
+static struct samsung_gpio_chip exynos5_gpios_2[] = {
+#ifdef CONFIG_ARCH_EXYNOS5
+	{
+		.chip	= {
+			.base	= EXYNOS5_GPE0(0),
+			.ngpio	= EXYNOS5_GPIO_E0_NR,
+			.label	= "GPE0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPE1(0),
+			.ngpio	= EXYNOS5_GPIO_E1_NR,
+			.label	= "GPE1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPF0(0),
+			.ngpio	= EXYNOS5_GPIO_F0_NR,
+			.label	= "GPF0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPF1(0),
+			.ngpio	= EXYNOS5_GPIO_F1_NR,
+			.label	= "GPF1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPG0(0),
+			.ngpio	= EXYNOS5_GPIO_G0_NR,
+			.label	= "GPG0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPG1(0),
+			.ngpio	= EXYNOS5_GPIO_G1_NR,
+			.label	= "GPG1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPG2(0),
+			.ngpio	= EXYNOS5_GPIO_G2_NR,
+			.label	= "GPG2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPH0(0),
+			.ngpio	= EXYNOS5_GPIO_H0_NR,
+			.label	= "GPH0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPH1(0),
+			.ngpio	= EXYNOS5_GPIO_H1_NR,
+			.label	= "GPH1",
+
+		},
+	},
+#endif
+};
+
+static struct samsung_gpio_chip exynos5_gpios_3[] = {
+#ifdef CONFIG_ARCH_EXYNOS5
+	{
+		.chip	= {
+			.base	= EXYNOS5_GPV0(0),
+			.ngpio	= EXYNOS5_GPIO_V0_NR,
+			.label	= "GPV0",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPV1(0),
+			.ngpio	= EXYNOS5_GPIO_V1_NR,
+			.label	= "GPV1",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPV2(0),
+			.ngpio	= EXYNOS5_GPIO_V2_NR,
+			.label	= "GPV2",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPV3(0),
+			.ngpio	= EXYNOS5_GPIO_V3_NR,
+			.label	= "GPV3",
+		},
+	}, {
+		.chip	= {
+			.base	= EXYNOS5_GPV4(0),
+			.ngpio	= EXYNOS5_GPIO_V4_NR,
+			.label	= "GPV4",
+		},
+	},
+#endif
+};
+
+static struct samsung_gpio_chip exynos5_gpios_4[] = {
+#ifdef CONFIG_ARCH_EXYNOS5
+	{
+		.chip	= {
+			.base	= EXYNOS5_GPZ(0),
+			.ngpio	= EXYNOS5_GPIO_Z_NR,
+			.label	= "GPZ",
+		},
+	},
+#endif
+};
+
+
 #if defined(CONFIG_ARCH_EXYNOS4) && defined(CONFIG_OF)
 static int exynos4_gpio_xlate(struct gpio_chip *gc, struct device_node *np,
 			      const void *gpio_spec, u32 *flags)

@@ -2521,7 +2797,7 @@ static __init int samsung_gpiolib_init(void)
 
 		for (i = 0; i < nr_chips; i++, chip++) {
 			if (!chip->config) {
-				chip->config = &exynos4_gpio_cfg;
+				chip->config = &exynos_gpio_cfg;
 				chip->group = group++;
 			}
 #ifdef CONFIG_CPU_EXYNOS4210

@@ -2537,7 +2813,7 @@ static __init int samsung_gpiolib_init(void)
 
 		for (i = 0; i < nr_chips; i++, chip++) {
 			if (!chip->config) {
-				chip->config = &exynos4_gpio_cfg;
+				chip->config = &exynos_gpio_cfg;
 				chip->group = group++;
 			}
 #ifdef CONFIG_CPU_EXYNOS4210

@@ -2553,7 +2829,7 @@ static __init int samsung_gpiolib_init(void)
 
 		for (i = 0; i < nr_chips; i++, chip++) {
 			if (!chip->config) {
-				chip->config = &exynos4_gpio_cfg;
+				chip->config = &exynos_gpio_cfg;
 				chip->group = group++;
 			}
 #ifdef CONFIG_CPU_EXYNOS4210

@@ -2567,6 +2843,60 @@ static __init int samsung_gpiolib_init(void)
 		s5p_register_gpioint_bank(IRQ_GPIO_XA, 0, IRQ_GPIO1_NR_GROUPS);
 		s5p_register_gpioint_bank(IRQ_GPIO_XB, IRQ_GPIO1_NR_GROUPS, IRQ_GPIO2_NR_GROUPS);
 #endif
+	} else if (soc_is_exynos5250()) {
+		group = 0;
+
+		/* gpio part1 */
+		chip = exynos5_gpios_1;
+		nr_chips = ARRAY_SIZE(exynos5_gpios_1);
+
+		for (i = 0; i < nr_chips; i++, chip++) {
+			if (!chip->config) {
+				chip->config = &exynos_gpio_cfg;
+				chip->group = group++;
+			}
+		}
+		samsung_gpiolib_add_4bit_chips(exynos5_gpios_1,
+						nr_chips, S5P_VA_GPIO1);
+
+		/* gpio part2 */
+		chip = exynos5_gpios_2;
+		nr_chips = ARRAY_SIZE(exynos5_gpios_2);
+
+		for (i = 0; i < nr_chips; i++, chip++) {
+			if (!chip->config) {
+				chip->config = &exynos_gpio_cfg;
+				chip->group = group++;
+			}
+		}
+		samsung_gpiolib_add_4bit_chips(exynos5_gpios_2,
+						nr_chips, S5P_VA_GPIO2);
+
+		/* gpio part3 */
+		chip = exynos5_gpios_3;
+		nr_chips = ARRAY_SIZE(exynos5_gpios_3);
+
+		for (i = 0; i < nr_chips; i++, chip++) {
+			if (!chip->config) {
+				chip->config = &exynos_gpio_cfg;
+				chip->group = group++;
+			}
+		}
+		samsung_gpiolib_add_4bit_chips(exynos5_gpios_3,
+						nr_chips, S5P_VA_GPIO3);
+
+		/* gpio part4 */
+		chip = exynos5_gpios_4;
+		nr_chips = ARRAY_SIZE(exynos5_gpios_4);
+
+		for (i = 0; i < nr_chips; i++, chip++) {
+			if (!chip->config) {
+				chip->config = &exynos_gpio_cfg;
+				chip->group = group++;
+			}
+		}
+		samsung_gpiolib_add_4bit_chips(exynos5_gpios_4,
+						nr_chips, S5P_VA_GPIO4);
 	} else {
 		WARN(1, "Unknown SoC in gpio-samsung, no GPIOs added\n");
 		return -ENODEV;
Signature · 1 line
1.7.4.4