[PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Changhwan Youn]

This patch adds implementation External GIC on EXYNOS4 SoC.

Note: need to update timer codes for supporting old type of
EXYNOS4 SoCs.

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
[PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
[PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1
[PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
[PATCH 5/7] ARM: EXYNOS4: Add support external GIC
[PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers
[PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler

[PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Changhwan Youn]

This patch adds implementation External GIC on EXYNOS4 SoC.

Note: need to update timer codes for supporting old type of
EXYNOS4 SoCs.

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
[PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
[PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1
[PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
[PATCH 5/7] ARM: EXYNOS4: Add support external GIC
[PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers
[PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[Changhwan Youn]

This patch adds external GIC io memory mapping
to support external GIC on EXYNOS4.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c              |   10 ++++++++++
 arch/arm/mach-exynos4/include/mach/map.h |    5 +++--
 arch/arm/plat-s5p/include/plat/map-s5p.h |    5 +++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 1196f39..6a1ed74 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -107,6 +107,16 @@ static struct map_desc exynos4_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(EXYNOS4_PA_AUDSS),
 		.length		= SZ_1K,
 		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GIC_CPU,
+		.pfn		= __phys_to_pfn(EXYNOS4_PA_GIC_CPU),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GIC_DIST,
+		.pfn		= __phys_to_pfn(EXYNOS4_PA_GIC_DIST),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE,
 	},
 };
 
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 57d8074..9d5d797 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -61,10 +61,11 @@
 
 #define EXYNOS4_PA_COMBINER		0x10448000
 
+#define EXYNOS4_PA_GIC_CPU		0x10480000
+#define EXYNOS4_PA_GIC_DIST		0x10490000
+
 #define EXYNOS4_PA_COREPERI		0x10500000
-#define EXYNOS4_PA_GIC_CPU		0x10500100
 #define EXYNOS4_PA_TWD			0x10500600
-#define EXYNOS4_PA_GIC_DIST		0x10501000
 #define EXYNOS4_PA_L2CC			0x10502000
 
 #define EXYNOS4_PA_MDMA			0x10810000
diff --git a/arch/arm/plat-s5p/include/plat/map-s5p.h b/arch/arm/plat-s5p/include/plat/map-s5p.h
index daa4a44..264df2d 100644
--- a/arch/arm/plat-s5p/include/plat/map-s5p.h
+++ b/arch/arm/plat-s5p/include/plat/map-s5p.h
@@ -35,9 +35,10 @@
 #define S5P_VA_COREPERI_BASE	S3C_ADDR(0x02800000)
 #define S5P_VA_COREPERI(x)	(S5P_VA_COREPERI_BASE + (x))
 #define S5P_VA_SCU		S5P_VA_COREPERI(0x0)
-#define S5P_VA_GIC_CPU		S5P_VA_COREPERI(0x100)
 #define S5P_VA_TWD		S5P_VA_COREPERI(0x600)
-#define S5P_VA_GIC_DIST		S5P_VA_COREPERI(0x1000)
+
+#define S5P_VA_GIC_CPU		S3C_ADDR(0x02810000)
+#define S5P_VA_GIC_DIST		S3C_ADDR(0x02820000)
 
 #define S3C_VA_USB_HSPHY	S3C_ADDR(0x02900000)
 #define S5P_VA_AUDSS		S3C_ADDR(0x02A00000)
-- 
1.7.1

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[Changhwan Youn]

This patch adds external GIC io memory mapping
to support external GIC on EXYNOS4.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c              |   10 ++++++++++
 arch/arm/mach-exynos4/include/mach/map.h |    5 +++--
 arch/arm/plat-s5p/include/plat/map-s5p.h |    5 +++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 1196f39..6a1ed74 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -107,6 +107,16 @@ static struct map_desc exynos4_iodesc[] __initdata = {
 		.pfn		= __phys_to_pfn(EXYNOS4_PA_AUDSS),
 		.length		= SZ_1K,
 		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GIC_CPU,
+		.pfn		= __phys_to_pfn(EXYNOS4_PA_GIC_CPU),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE,
+	}, {
+		.virtual	= (unsigned long)S5P_VA_GIC_DIST,
+		.pfn		= __phys_to_pfn(EXYNOS4_PA_GIC_DIST),
+		.length		= SZ_64K,
+		.type		= MT_DEVICE,
 	},
 };
 
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 57d8074..9d5d797 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -61,10 +61,11 @@
 
 #define EXYNOS4_PA_COMBINER		0x10448000
 
+#define EXYNOS4_PA_GIC_CPU		0x10480000
+#define EXYNOS4_PA_GIC_DIST		0x10490000
+
 #define EXYNOS4_PA_COREPERI		0x10500000
-#define EXYNOS4_PA_GIC_CPU		0x10500100
 #define EXYNOS4_PA_TWD			0x10500600
-#define EXYNOS4_PA_GIC_DIST		0x10501000
 #define EXYNOS4_PA_L2CC			0x10502000
 
 #define EXYNOS4_PA_MDMA			0x10810000
diff --git a/arch/arm/plat-s5p/include/plat/map-s5p.h b/arch/arm/plat-s5p/include/plat/map-s5p.h
index daa4a44..264df2d 100644
--- a/arch/arm/plat-s5p/include/plat/map-s5p.h
+++ b/arch/arm/plat-s5p/include/plat/map-s5p.h
@@ -35,9 +35,10 @@
 #define S5P_VA_COREPERI_BASE	S3C_ADDR(0x02800000)
 #define S5P_VA_COREPERI(x)	(S5P_VA_COREPERI_BASE + (x))
 #define S5P_VA_SCU		S5P_VA_COREPERI(0x0)
-#define S5P_VA_GIC_CPU		S5P_VA_COREPERI(0x100)
 #define S5P_VA_TWD		S5P_VA_COREPERI(0x600)
-#define S5P_VA_GIC_DIST		S5P_VA_COREPERI(0x1000)
+
+#define S5P_VA_GIC_CPU		S3C_ADDR(0x02810000)
+#define S5P_VA_GIC_DIST		S3C_ADDR(0x02820000)
 
 #define S3C_VA_USB_HSPHY	S3C_ADDR(0x02900000)
 #define S5P_VA_AUDSS		S3C_ADDR(0x02A00000)
-- 
1.7.1

[PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC

[Changhwan Youn]

To support external GIC needs to update mapping of interrupt number.
This patch modifies it for external GIC and accordingly removes
the unused code.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c               |    8 --
 arch/arm/mach-exynos4/include/mach/irqs.h |  188 ++++++++++++++---------------
 2 files changed, 93 insertions(+), 103 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 6a1ed74..fa33294 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -167,14 +167,6 @@ void __init exynos4_init_irq(void)
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
 
-		/*
-		 * From SPI(0) to SPI(39) and SPI(51), SPI(53) are
-		 * connected to the interrupt combiner. These irqs
-		 * should be initialized to support cascade interrupt.
-		 */
-		if ((irq >= 40) && !(irq == 51) && !(irq == 53))
-			continue;
-
 		combiner_init(irq, (void __iomem *)S5P_VA_COMBINER(irq),
 				COMBINER_IRQ(irq, 0));
 		combiner_cascade_irq(irq, IRQ_SPI(irq));
diff --git a/arch/arm/mach-exynos4/include/mach/irqs.h b/arch/arm/mach-exynos4/include/mach/irqs.h
index 5d03730..e497ea2 100644
--- a/arch/arm/mach-exynos4/include/mach/irqs.h
+++ b/arch/arm/mach-exynos4/include/mach/irqs.h
@@ -25,34 +25,100 @@
 
 #define IRQ_SPI(x)		S5P_IRQ(x+32)
 
-#define IRQ_MCT1		IRQ_SPI(35)
-
-#define IRQ_EINT0		IRQ_SPI(40)
-#define IRQ_EINT1		IRQ_SPI(41)
-#define IRQ_EINT2		IRQ_SPI(42)
-#define IRQ_EINT3		IRQ_SPI(43)
-#define IRQ_USB_HSOTG		IRQ_SPI(44)
-#define IRQ_USB_HOST		IRQ_SPI(45)
-#define IRQ_MODEM_IF		IRQ_SPI(46)
-#define IRQ_ROTATOR		IRQ_SPI(47)
-#define IRQ_JPEG		IRQ_SPI(48)
-#define IRQ_2D			IRQ_SPI(49)
-#define IRQ_PCIE		IRQ_SPI(50)
-#define IRQ_MCT0		IRQ_SPI(51)
-#define IRQ_MFC			IRQ_SPI(52)
-#define IRQ_AUDIO_SS		IRQ_SPI(54)
-#define IRQ_AC97		IRQ_SPI(55)
-#define IRQ_SPDIF		IRQ_SPI(56)
-#define IRQ_KEYPAD		IRQ_SPI(57)
-#define IRQ_INTFEEDCTRL_SSS	IRQ_SPI(58)
-#define IRQ_SLIMBUS		IRQ_SPI(59)
-#define IRQ_PMU			IRQ_SPI(60)
-#define IRQ_TSI			IRQ_SPI(61)
-#define IRQ_SATA		IRQ_SPI(62)
-#define IRQ_GPS			IRQ_SPI(63)
+#define IRQ_EINT0		IRQ_SPI(16)
+#define IRQ_EINT1		IRQ_SPI(17)
+#define IRQ_EINT2		IRQ_SPI(18)
+#define IRQ_EINT3		IRQ_SPI(19)
+#define IRQ_EINT4		IRQ_SPI(20)
+#define IRQ_EINT5		IRQ_SPI(21)
+#define IRQ_EINT6		IRQ_SPI(22)
+#define IRQ_EINT7		IRQ_SPI(23)
+#define IRQ_EINT8		IRQ_SPI(24)
+#define IRQ_EINT9		IRQ_SPI(25)
+#define IRQ_EINT10		IRQ_SPI(26)
+#define IRQ_EINT11		IRQ_SPI(27)
+#define IRQ_EINT12		IRQ_SPI(28)
+#define IRQ_EINT13		IRQ_SPI(29)
+#define IRQ_EINT14		IRQ_SPI(30)
+#define IRQ_EINT15		IRQ_SPI(31)
+#define IRQ_EINT16_31		IRQ_SPI(32)
+
+#define IRQ_PDMA0		IRQ_SPI(35)
+#define IRQ_PDMA1		IRQ_SPI(36)
+#define IRQ_TIMER0_VIC		IRQ_SPI(37)
+#define IRQ_TIMER1_VIC		IRQ_SPI(38)
+#define IRQ_TIMER2_VIC		IRQ_SPI(39)
+#define IRQ_TIMER3_VIC		IRQ_SPI(40)
+#define IRQ_TIMER4_VIC		IRQ_SPI(41)
+#define IRQ_MCT_L0		IRQ_SPI(42)
+#define IRQ_WDT			IRQ_SPI(43)
+#define IRQ_RTC_ALARM		IRQ_SPI(44)
+#define IRQ_RTC_TIC		IRQ_SPI(45)
+#define IRQ_GPIO_XB		IRQ_SPI(46)
+#define IRQ_GPIO_XA		IRQ_SPI(47)
+#define IRQ_MCT_L1		IRQ_SPI(48)
+
+#define IRQ_UART0		IRQ_SPI(52)
+#define IRQ_UART1		IRQ_SPI(53)
+#define IRQ_UART2		IRQ_SPI(54)
+#define IRQ_UART3		IRQ_SPI(55)
+#define IRQ_UART4		IRQ_SPI(56)
+#define IRQ_MCT_G0		IRQ_SPI(57)
+#define IRQ_IIC			IRQ_SPI(58)
+#define IRQ_IIC1		IRQ_SPI(59)
+#define IRQ_IIC2		IRQ_SPI(60)
+#define IRQ_IIC3		IRQ_SPI(61)
+#define IRQ_IIC4		IRQ_SPI(62)
+#define IRQ_IIC5		IRQ_SPI(63)
+#define IRQ_IIC6		IRQ_SPI(64)
+#define IRQ_IIC7		IRQ_SPI(65)
+
+#define IRQ_USB_HOST		IRQ_SPI(70)
+#define IRQ_USB_HSOTG		IRQ_SPI(71)
+#define IRQ_MODEM_IF		IRQ_SPI(72)
+#define IRQ_HSMMC0		IRQ_SPI(73)
+#define IRQ_HSMMC1		IRQ_SPI(74)
+#define IRQ_HSMMC2		IRQ_SPI(75)
+#define IRQ_HSMMC3		IRQ_SPI(76)
+
+#define IRQ_MIPICSI0		IRQ_SPI(78)
+
+#define IRQ_MIPICSI1		IRQ_SPI(80)
+
+#define IRQ_ONENAND_AUDI	IRQ_SPI(82)
+#define IRQ_ROTATOR		IRQ_SPI(83)
+#define IRQ_FIMC0		IRQ_SPI(84)
+#define IRQ_FIMC1		IRQ_SPI(85)
+#define IRQ_FIMC2		IRQ_SPI(86)
+#define IRQ_FIMC3		IRQ_SPI(87)
+#define IRQ_JPEG		IRQ_SPI(88)
+#define IRQ_2D			IRQ_SPI(89)
+#define IRQ_PCIE		IRQ_SPI(90)
+
+#define IRQ_MFC			IRQ_SPI(94)
+
+#define IRQ_AUDIO_SS		IRQ_SPI(96)
+#define IRQ_I2S0		IRQ_SPI(97)
+#define IRQ_I2S1		IRQ_SPI(98)
+#define IRQ_I2S2		IRQ_SPI(99)
+#define IRQ_AC97		IRQ_SPI(100)
+
+#define IRQ_SPDIF		IRQ_SPI(104)
+#define IRQ_ADC0		IRQ_SPI(105)
+#define IRQ_PEN0		IRQ_SPI(106)
+#define IRQ_ADC1		IRQ_SPI(107)
+#define IRQ_PEN1		IRQ_SPI(108)
+#define IRQ_KEYPAD		IRQ_SPI(109)
+#define IRQ_PMU			IRQ_SPI(110)
+#define IRQ_GPS			IRQ_SPI(111)
+#define IRQ_INTFEEDCTRL_SSS	IRQ_SPI(112)
+#define IRQ_SLIMBUS		IRQ_SPI(113)
+
+#define IRQ_TSI			IRQ_SPI(115)
+#define IRQ_SATA		IRQ_SPI(116)
 
 #define MAX_IRQ_IN_COMBINER	8
-#define COMBINER_GROUP(x)	((x) * MAX_IRQ_IN_COMBINER + IRQ_SPI(64))
+#define COMBINER_GROUP(x)	((x) * MAX_IRQ_IN_COMBINER + IRQ_SPI(128))
 #define COMBINER_IRQ(x, y)	(COMBINER_GROUP(x) + y)
 
 #define IRQ_SYSMMU_MDMA0_0	COMBINER_IRQ(4, 0)
@@ -73,75 +139,7 @@
 #define IRQ_SYSMMU_MFC_M1_0	COMBINER_IRQ(5, 6)
 #define IRQ_SYSMMU_PCIE_0	COMBINER_IRQ(5, 7)
 
-#define IRQ_PDMA0		COMBINER_IRQ(21, 0)
-#define IRQ_PDMA1		COMBINER_IRQ(21, 1)
-
-#define IRQ_TIMER0_VIC		COMBINER_IRQ(22, 0)
-#define IRQ_TIMER1_VIC		COMBINER_IRQ(22, 1)
-#define IRQ_TIMER2_VIC		COMBINER_IRQ(22, 2)
-#define IRQ_TIMER3_VIC		COMBINER_IRQ(22, 3)
-#define IRQ_TIMER4_VIC		COMBINER_IRQ(22, 4)
-
-#define IRQ_RTC_ALARM		COMBINER_IRQ(23, 0)
-#define IRQ_RTC_TIC		COMBINER_IRQ(23, 1)
-
-#define IRQ_GPIO_XB		COMBINER_IRQ(24, 0)
-#define IRQ_GPIO_XA		COMBINER_IRQ(24, 1)
-
-#define IRQ_UART0		COMBINER_IRQ(26, 0)
-#define IRQ_UART1		COMBINER_IRQ(26, 1)
-#define IRQ_UART2		COMBINER_IRQ(26, 2)
-#define IRQ_UART3		COMBINER_IRQ(26, 3)
-#define IRQ_UART4		COMBINER_IRQ(26, 4)
-
-#define IRQ_IIC			COMBINER_IRQ(27, 0)
-#define IRQ_IIC1		COMBINER_IRQ(27, 1)
-#define IRQ_IIC2		COMBINER_IRQ(27, 2)
-#define IRQ_IIC3		COMBINER_IRQ(27, 3)
-#define IRQ_IIC4		COMBINER_IRQ(27, 4)
-#define IRQ_IIC5		COMBINER_IRQ(27, 5)
-#define IRQ_IIC6		COMBINER_IRQ(27, 6)
-#define IRQ_IIC7		COMBINER_IRQ(27, 7)
-
-#define IRQ_HSMMC0		COMBINER_IRQ(29, 0)
-#define IRQ_HSMMC1		COMBINER_IRQ(29, 1)
-#define IRQ_HSMMC2		COMBINER_IRQ(29, 2)
-#define IRQ_HSMMC3		COMBINER_IRQ(29, 3)
-
-#define IRQ_MIPI_CSIS0		COMBINER_IRQ(30, 0)
-#define IRQ_MIPI_CSIS1		COMBINER_IRQ(30, 1)
-
-#define IRQ_FIMC0		COMBINER_IRQ(32, 0)
-#define IRQ_FIMC1		COMBINER_IRQ(32, 1)
-#define IRQ_FIMC2		COMBINER_IRQ(33, 0)
-#define IRQ_FIMC3		COMBINER_IRQ(33, 1)
-
-#define IRQ_ONENAND_AUDI	COMBINER_IRQ(34, 0)
-
-#define IRQ_MCT_L1		COMBINER_IRQ(35, 3)
-
-#define IRQ_EINT4		COMBINER_IRQ(37, 0)
-#define IRQ_EINT5		COMBINER_IRQ(37, 1)
-#define IRQ_EINT6		COMBINER_IRQ(37, 2)
-#define IRQ_EINT7		COMBINER_IRQ(37, 3)
-#define IRQ_EINT8		COMBINER_IRQ(38, 0)
-
-#define IRQ_EINT9		COMBINER_IRQ(38, 1)
-#define IRQ_EINT10		COMBINER_IRQ(38, 2)
-#define IRQ_EINT11		COMBINER_IRQ(38, 3)
-#define IRQ_EINT12		COMBINER_IRQ(38, 4)
-#define IRQ_EINT13		COMBINER_IRQ(38, 5)
-#define IRQ_EINT14		COMBINER_IRQ(38, 6)
-#define IRQ_EINT15		COMBINER_IRQ(38, 7)
-
-#define IRQ_EINT16_31		COMBINER_IRQ(39, 0)
-
-#define IRQ_MCT_L0		COMBINER_IRQ(51, 0)
-
-#define IRQ_WDT			COMBINER_IRQ(53, 0)
-#define IRQ_MCT_G0		COMBINER_IRQ(53, 4)
-
-#define MAX_COMBINER_NR		54
+#define MAX_COMBINER_NR		16
 
 #define S5P_IRQ_EINT_BASE	COMBINER_IRQ(MAX_COMBINER_NR, 0)
 
-- 
1.7.1

[PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC

[Changhwan Youn]

To support external GIC needs to update mapping of interrupt number.
This patch modifies it for external GIC and accordingly removes
the unused code.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c               |    8 --
 arch/arm/mach-exynos4/include/mach/irqs.h |  188 ++++++++++++++---------------
 2 files changed, 93 insertions(+), 103 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 6a1ed74..fa33294 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -167,14 +167,6 @@ void __init exynos4_init_irq(void)
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
 
-		/*
-		 * From SPI(0) to SPI(39) and SPI(51), SPI(53) are
-		 * connected to the interrupt combiner. These irqs
-		 * should be initialized to support cascade interrupt.
-		 */
-		if ((irq >= 40) && !(irq == 51) && !(irq == 53))
-			continue;
-
 		combiner_init(irq, (void __iomem *)S5P_VA_COMBINER(irq),
 				COMBINER_IRQ(irq, 0));
 		combiner_cascade_irq(irq, IRQ_SPI(irq));
diff --git a/arch/arm/mach-exynos4/include/mach/irqs.h b/arch/arm/mach-exynos4/include/mach/irqs.h
index 5d03730..e497ea2 100644
--- a/arch/arm/mach-exynos4/include/mach/irqs.h
+++ b/arch/arm/mach-exynos4/include/mach/irqs.h
@@ -25,34 +25,100 @@
 
 #define IRQ_SPI(x)		S5P_IRQ(x+32)
 
-#define IRQ_MCT1		IRQ_SPI(35)
-
-#define IRQ_EINT0		IRQ_SPI(40)
-#define IRQ_EINT1		IRQ_SPI(41)
-#define IRQ_EINT2		IRQ_SPI(42)
-#define IRQ_EINT3		IRQ_SPI(43)
-#define IRQ_USB_HSOTG		IRQ_SPI(44)
-#define IRQ_USB_HOST		IRQ_SPI(45)
-#define IRQ_MODEM_IF		IRQ_SPI(46)
-#define IRQ_ROTATOR		IRQ_SPI(47)
-#define IRQ_JPEG		IRQ_SPI(48)
-#define IRQ_2D			IRQ_SPI(49)
-#define IRQ_PCIE		IRQ_SPI(50)
-#define IRQ_MCT0		IRQ_SPI(51)
-#define IRQ_MFC			IRQ_SPI(52)
-#define IRQ_AUDIO_SS		IRQ_SPI(54)
-#define IRQ_AC97		IRQ_SPI(55)
-#define IRQ_SPDIF		IRQ_SPI(56)
-#define IRQ_KEYPAD		IRQ_SPI(57)
-#define IRQ_INTFEEDCTRL_SSS	IRQ_SPI(58)
-#define IRQ_SLIMBUS		IRQ_SPI(59)
-#define IRQ_PMU			IRQ_SPI(60)
-#define IRQ_TSI			IRQ_SPI(61)
-#define IRQ_SATA		IRQ_SPI(62)
-#define IRQ_GPS			IRQ_SPI(63)
+#define IRQ_EINT0		IRQ_SPI(16)
+#define IRQ_EINT1		IRQ_SPI(17)
+#define IRQ_EINT2		IRQ_SPI(18)
+#define IRQ_EINT3		IRQ_SPI(19)
+#define IRQ_EINT4		IRQ_SPI(20)
+#define IRQ_EINT5		IRQ_SPI(21)
+#define IRQ_EINT6		IRQ_SPI(22)
+#define IRQ_EINT7		IRQ_SPI(23)
+#define IRQ_EINT8		IRQ_SPI(24)
+#define IRQ_EINT9		IRQ_SPI(25)
+#define IRQ_EINT10		IRQ_SPI(26)
+#define IRQ_EINT11		IRQ_SPI(27)
+#define IRQ_EINT12		IRQ_SPI(28)
+#define IRQ_EINT13		IRQ_SPI(29)
+#define IRQ_EINT14		IRQ_SPI(30)
+#define IRQ_EINT15		IRQ_SPI(31)
+#define IRQ_EINT16_31		IRQ_SPI(32)
+
+#define IRQ_PDMA0		IRQ_SPI(35)
+#define IRQ_PDMA1		IRQ_SPI(36)
+#define IRQ_TIMER0_VIC		IRQ_SPI(37)
+#define IRQ_TIMER1_VIC		IRQ_SPI(38)
+#define IRQ_TIMER2_VIC		IRQ_SPI(39)
+#define IRQ_TIMER3_VIC		IRQ_SPI(40)
+#define IRQ_TIMER4_VIC		IRQ_SPI(41)
+#define IRQ_MCT_L0		IRQ_SPI(42)
+#define IRQ_WDT			IRQ_SPI(43)
+#define IRQ_RTC_ALARM		IRQ_SPI(44)
+#define IRQ_RTC_TIC		IRQ_SPI(45)
+#define IRQ_GPIO_XB		IRQ_SPI(46)
+#define IRQ_GPIO_XA		IRQ_SPI(47)
+#define IRQ_MCT_L1		IRQ_SPI(48)
+
+#define IRQ_UART0		IRQ_SPI(52)
+#define IRQ_UART1		IRQ_SPI(53)
+#define IRQ_UART2		IRQ_SPI(54)
+#define IRQ_UART3		IRQ_SPI(55)
+#define IRQ_UART4		IRQ_SPI(56)
+#define IRQ_MCT_G0		IRQ_SPI(57)
+#define IRQ_IIC			IRQ_SPI(58)
+#define IRQ_IIC1		IRQ_SPI(59)
+#define IRQ_IIC2		IRQ_SPI(60)
+#define IRQ_IIC3		IRQ_SPI(61)
+#define IRQ_IIC4		IRQ_SPI(62)
+#define IRQ_IIC5		IRQ_SPI(63)
+#define IRQ_IIC6		IRQ_SPI(64)
+#define IRQ_IIC7		IRQ_SPI(65)
+
+#define IRQ_USB_HOST		IRQ_SPI(70)
+#define IRQ_USB_HSOTG		IRQ_SPI(71)
+#define IRQ_MODEM_IF		IRQ_SPI(72)
+#define IRQ_HSMMC0		IRQ_SPI(73)
+#define IRQ_HSMMC1		IRQ_SPI(74)
+#define IRQ_HSMMC2		IRQ_SPI(75)
+#define IRQ_HSMMC3		IRQ_SPI(76)
+
+#define IRQ_MIPICSI0		IRQ_SPI(78)
+
+#define IRQ_MIPICSI1		IRQ_SPI(80)
+
+#define IRQ_ONENAND_AUDI	IRQ_SPI(82)
+#define IRQ_ROTATOR		IRQ_SPI(83)
+#define IRQ_FIMC0		IRQ_SPI(84)
+#define IRQ_FIMC1		IRQ_SPI(85)
+#define IRQ_FIMC2		IRQ_SPI(86)
+#define IRQ_FIMC3		IRQ_SPI(87)
+#define IRQ_JPEG		IRQ_SPI(88)
+#define IRQ_2D			IRQ_SPI(89)
+#define IRQ_PCIE		IRQ_SPI(90)
+
+#define IRQ_MFC			IRQ_SPI(94)
+
+#define IRQ_AUDIO_SS		IRQ_SPI(96)
+#define IRQ_I2S0		IRQ_SPI(97)
+#define IRQ_I2S1		IRQ_SPI(98)
+#define IRQ_I2S2		IRQ_SPI(99)
+#define IRQ_AC97		IRQ_SPI(100)
+
+#define IRQ_SPDIF		IRQ_SPI(104)
+#define IRQ_ADC0		IRQ_SPI(105)
+#define IRQ_PEN0		IRQ_SPI(106)
+#define IRQ_ADC1		IRQ_SPI(107)
+#define IRQ_PEN1		IRQ_SPI(108)
+#define IRQ_KEYPAD		IRQ_SPI(109)
+#define IRQ_PMU			IRQ_SPI(110)
+#define IRQ_GPS			IRQ_SPI(111)
+#define IRQ_INTFEEDCTRL_SSS	IRQ_SPI(112)
+#define IRQ_SLIMBUS		IRQ_SPI(113)
+
+#define IRQ_TSI			IRQ_SPI(115)
+#define IRQ_SATA		IRQ_SPI(116)
 
 #define MAX_IRQ_IN_COMBINER	8
-#define COMBINER_GROUP(x)	((x) * MAX_IRQ_IN_COMBINER + IRQ_SPI(64))
+#define COMBINER_GROUP(x)	((x) * MAX_IRQ_IN_COMBINER + IRQ_SPI(128))
 #define COMBINER_IRQ(x, y)	(COMBINER_GROUP(x) + y)
 
 #define IRQ_SYSMMU_MDMA0_0	COMBINER_IRQ(4, 0)
@@ -73,75 +139,7 @@
 #define IRQ_SYSMMU_MFC_M1_0	COMBINER_IRQ(5, 6)
 #define IRQ_SYSMMU_PCIE_0	COMBINER_IRQ(5, 7)
 
-#define IRQ_PDMA0		COMBINER_IRQ(21, 0)
-#define IRQ_PDMA1		COMBINER_IRQ(21, 1)
-
-#define IRQ_TIMER0_VIC		COMBINER_IRQ(22, 0)
-#define IRQ_TIMER1_VIC		COMBINER_IRQ(22, 1)
-#define IRQ_TIMER2_VIC		COMBINER_IRQ(22, 2)
-#define IRQ_TIMER3_VIC		COMBINER_IRQ(22, 3)
-#define IRQ_TIMER4_VIC		COMBINER_IRQ(22, 4)
-
-#define IRQ_RTC_ALARM		COMBINER_IRQ(23, 0)
-#define IRQ_RTC_TIC		COMBINER_IRQ(23, 1)
-
-#define IRQ_GPIO_XB		COMBINER_IRQ(24, 0)
-#define IRQ_GPIO_XA		COMBINER_IRQ(24, 1)
-
-#define IRQ_UART0		COMBINER_IRQ(26, 0)
-#define IRQ_UART1		COMBINER_IRQ(26, 1)
-#define IRQ_UART2		COMBINER_IRQ(26, 2)
-#define IRQ_UART3		COMBINER_IRQ(26, 3)
-#define IRQ_UART4		COMBINER_IRQ(26, 4)
-
-#define IRQ_IIC			COMBINER_IRQ(27, 0)
-#define IRQ_IIC1		COMBINER_IRQ(27, 1)
-#define IRQ_IIC2		COMBINER_IRQ(27, 2)
-#define IRQ_IIC3		COMBINER_IRQ(27, 3)
-#define IRQ_IIC4		COMBINER_IRQ(27, 4)
-#define IRQ_IIC5		COMBINER_IRQ(27, 5)
-#define IRQ_IIC6		COMBINER_IRQ(27, 6)
-#define IRQ_IIC7		COMBINER_IRQ(27, 7)
-
-#define IRQ_HSMMC0		COMBINER_IRQ(29, 0)
-#define IRQ_HSMMC1		COMBINER_IRQ(29, 1)
-#define IRQ_HSMMC2		COMBINER_IRQ(29, 2)
-#define IRQ_HSMMC3		COMBINER_IRQ(29, 3)
-
-#define IRQ_MIPI_CSIS0		COMBINER_IRQ(30, 0)
-#define IRQ_MIPI_CSIS1		COMBINER_IRQ(30, 1)
-
-#define IRQ_FIMC0		COMBINER_IRQ(32, 0)
-#define IRQ_FIMC1		COMBINER_IRQ(32, 1)
-#define IRQ_FIMC2		COMBINER_IRQ(33, 0)
-#define IRQ_FIMC3		COMBINER_IRQ(33, 1)
-
-#define IRQ_ONENAND_AUDI	COMBINER_IRQ(34, 0)
-
-#define IRQ_MCT_L1		COMBINER_IRQ(35, 3)
-
-#define IRQ_EINT4		COMBINER_IRQ(37, 0)
-#define IRQ_EINT5		COMBINER_IRQ(37, 1)
-#define IRQ_EINT6		COMBINER_IRQ(37, 2)
-#define IRQ_EINT7		COMBINER_IRQ(37, 3)
-#define IRQ_EINT8		COMBINER_IRQ(38, 0)
-
-#define IRQ_EINT9		COMBINER_IRQ(38, 1)
-#define IRQ_EINT10		COMBINER_IRQ(38, 2)
-#define IRQ_EINT11		COMBINER_IRQ(38, 3)
-#define IRQ_EINT12		COMBINER_IRQ(38, 4)
-#define IRQ_EINT13		COMBINER_IRQ(38, 5)
-#define IRQ_EINT14		COMBINER_IRQ(38, 6)
-#define IRQ_EINT15		COMBINER_IRQ(38, 7)
-
-#define IRQ_EINT16_31		COMBINER_IRQ(39, 0)
-
-#define IRQ_MCT_L0		COMBINER_IRQ(51, 0)
-
-#define IRQ_WDT			COMBINER_IRQ(53, 0)
-#define IRQ_MCT_G0		COMBINER_IRQ(53, 4)
-
-#define MAX_COMBINER_NR		54
+#define MAX_COMBINER_NR		16
 
 #define S5P_IRQ_EINT_BASE	COMBINER_IRQ(MAX_COMBINER_NR, 0)
 
-- 
1.7.1

[PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1

[Changhwan Youn]

IRQ_MCT_L1 is connected directly to GIC in external GIC mapping,
while in internal GIC mapping, it is connected to GIC through
interrupt combiner. Therfore the affinity for mct1 event timer
interrupt should be changed through IRQ_MCT_L1.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/mct.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index 14ac10b..1ae059b 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -383,8 +383,8 @@ static void exynos4_mct_tick_init(struct clock_event_device *evt)
 		setup_irq(IRQ_MCT_L0, &mct_tick0_event_irq);
 	} else {
 		mct_tick1_event_irq.dev_id = &mct_tick[cpu];
-		irq_set_affinity(IRQ_MCT1, cpumask_of(1));
 		setup_irq(IRQ_MCT_L1, &mct_tick1_event_irq);
+		irq_set_affinity(IRQ_MCT_L1, cpumask_of(1));
 	}
 }
 
-- 
1.7.1

[PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1

[Changhwan Youn]

IRQ_MCT_L1 is connected directly to GIC in external GIC mapping,
while in internal GIC mapping, it is connected to GIC through
interrupt combiner. Therfore the affinity for mct1 event timer
interrupt should be changed through IRQ_MCT_L1.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/mct.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/mct.c b/arch/arm/mach-exynos4/mct.c
index 14ac10b..1ae059b 100644
--- a/arch/arm/mach-exynos4/mct.c
+++ b/arch/arm/mach-exynos4/mct.c
@@ -383,8 +383,8 @@ static void exynos4_mct_tick_init(struct clock_event_device *evt)
 		setup_irq(IRQ_MCT_L0, &mct_tick0_event_irq);
 	} else {
 		mct_tick1_event_irq.dev_id = &mct_tick[cpu];
-		irq_set_affinity(IRQ_MCT1, cpumask_of(1));
 		setup_irq(IRQ_MCT_L1, &mct_tick1_event_irq);
+		irq_set_affinity(IRQ_MCT_L1, cpumask_of(1));
 	}
 }
 
-- 
1.7.1

[PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header

[Changhwan Youn]

Since Samsung EXYNOS4210 cannot support register banking in GIC,
so needs to update CPU interface base address.
The 'gic_chip_data' is used for it, this patch moves gic_chip_data
structure declaraton to arch/arm/include/asm/hardware/gic.h to use
it.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/common/gic.c               |    6 ------
 arch/arm/include/asm/hardware/gic.h |    6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4ddd0a6..23564ed 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -38,12 +38,6 @@ static DEFINE_SPINLOCK(irq_controller_lock);
 /* Address of GIC 0 CPU interface */
 void __iomem *gic_cpu_base_addr __read_mostly;
 
-struct gic_chip_data {
-	unsigned int irq_offset;
-	void __iomem *dist_base;
-	void __iomem *cpu_base;
-};
-
 /*
  * Supported arch specific GIC irq extension.
  * Default make them NULL.
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 0691f9d..435d3f8 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -41,6 +41,12 @@ void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
 void gic_enable_ppi(unsigned int);
+
+struct gic_chip_data {
+	unsigned int irq_offset;
+	void __iomem *dist_base;
+	void __iomem *cpu_base;
+};
 #endif
 
 #endif
-- 
1.7.1

[PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header

[Changhwan Youn]

Since Samsung EXYNOS4210 cannot support register banking in GIC,
so needs to update CPU interface base address.
The 'gic_chip_data' is used for it, this patch moves gic_chip_data
structure declaraton to arch/arm/include/asm/hardware/gic.h to use
it.

Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/common/gic.c               |    6 ------
 arch/arm/include/asm/hardware/gic.h |    6 ++++++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 4ddd0a6..23564ed 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -38,12 +38,6 @@ static DEFINE_SPINLOCK(irq_controller_lock);
 /* Address of GIC 0 CPU interface */
 void __iomem *gic_cpu_base_addr __read_mostly;
 
-struct gic_chip_data {
-	unsigned int irq_offset;
-	void __iomem *dist_base;
-	void __iomem *cpu_base;
-};
-
 /*
  * Supported arch specific GIC irq extension.
  * Default make them NULL.
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 0691f9d..435d3f8 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -41,6 +41,12 @@ void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
 void gic_enable_ppi(unsigned int);
+
+struct gic_chip_data {
+	unsigned int irq_offset;
+	void __iomem *dist_base;
+	void __iomem *cpu_base;
+};
 #endif
 
 #endif
-- 
1.7.1

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Changhwan Youn]

For full support of power modes, this patch adds implementation
external GIC on EXYNOS4.

External GIC of Exynos4 cannot support register banking so
several interrupt related code for CPU1 should be different
from that of CPU0.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
 arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
 arch/arm/mach-exynos4/include/mach/map.h         |    1 +
 arch/arm/mach-exynos4/platsmp.c                  |   27 +++++++++++++++++++++-
 4 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index fa33294..40a866c 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -16,6 +16,7 @@
 
 #include <asm/proc-fns.h>
 #include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/gic.h>
 
 #include <plat/cpu.h>
 #include <plat/clock.h>
@@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
 	exynos4_setup_clocks();
 }
 
+static void exynos4_gic_irq_eoi(struct irq_data *d)
+{
+	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+
+	gic_data->cpu_base = S5P_VA_GIC_CPU +
+			    (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+}
+
 void __init exynos4_init_irq(void)
 {
 	int irq;
 
 	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
+	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
 
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d8f38c2..4fad076 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -10,6 +10,7 @@
 */
 
 #include <mach/hardware.h>
+#include <mach/map.h>
 #include <asm/hardware/gic.h>
 
 		.macro	disable_fiq
@@ -18,6 +19,10 @@
 		.macro  get_irqnr_preamble, base, tmp
 		ldr	\base, =gic_cpu_base_addr
 		ldr	\base, [\base]
+		mrc     p15, 0, \tmp, c0, c0, 5
+		and     \tmp, \tmp, #3
+		cmp     \tmp, #1
+		addeq   \base, \base, #EXYNOS4_GIC_BANK_OFFSET
 		.endm
 
 		.macro  arch_ret_to_user, tmp1, tmp2
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 9d5d797..2339a70 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -63,6 +63,7 @@
 
 #define EXYNOS4_PA_GIC_CPU		0x10480000
 #define EXYNOS4_PA_GIC_DIST		0x10490000
+#define EXYNOS4_GIC_BANK_OFFSET		0x8000
 
 #define EXYNOS4_PA_COREPERI		0x10500000
 #define EXYNOS4_PA_TWD			0x10500600
diff --git a/arch/arm/mach-exynos4/platsmp.c b/arch/arm/mach-exynos4/platsmp.c
index c5e65a0..decf528 100644
--- a/arch/arm/mach-exynos4/platsmp.c
+++ b/arch/arm/mach-exynos4/platsmp.c
@@ -58,6 +58,31 @@ static void __iomem *scu_base_addr(void)
 
 static DEFINE_SPINLOCK(boot_lock);
 
+static void __cpuinit exynos4_gic_secondary_init(void)
+{
+	void __iomem *dist_base = S5P_VA_GIC_DIST +
+				 (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+	void __iomem *cpu_base = S5P_VA_GIC_CPU +
+				(EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+	int i;
+
+	/*
+	 * Deal with the banked PPI and SGI interrupts - disable all
+	 * PPI interrupts, ensure all SGI interrupts are enabled.
+	 */
+	__raw_writel(0xffff0000, dist_base + GIC_DIST_ENABLE_CLEAR);
+	__raw_writel(0x0000ffff, dist_base + GIC_DIST_ENABLE_SET);
+
+	/*
+	 * Set priority on PPI and SGI interrupts
+	 */
+	for (i = 0; i < 32; i += 4)
+		__raw_writel(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
+
+	__raw_writel(0xf0, cpu_base + GIC_CPU_PRIMASK);
+	__raw_writel(1, cpu_base + GIC_CPU_CTRL);
+}
+
 void __cpuinit platform_secondary_init(unsigned int cpu)
 {
 	/*
@@ -65,7 +90,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu)
 	 * core (e.g. timer irq), then they will not have been enabled
 	 * for us: do so
 	 */
-	gic_secondary_init(0);
+	exynos4_gic_secondary_init();
 
 	/*
 	 * let the primary processor know we're out of the
-- 
1.7.1

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Changhwan Youn]

For full support of power modes, this patch adds implementation
external GIC on EXYNOS4.

External GIC of Exynos4 cannot support register banking so
several interrupt related code for CPU1 should be different
from that of CPU0.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
 arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
 arch/arm/mach-exynos4/include/mach/map.h         |    1 +
 arch/arm/mach-exynos4/platsmp.c                  |   27 +++++++++++++++++++++-
 4 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index fa33294..40a866c 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -16,6 +16,7 @@
 
 #include <asm/proc-fns.h>
 #include <asm/hardware/cache-l2x0.h>
+#include <asm/hardware/gic.h>
 
 #include <plat/cpu.h>
 #include <plat/clock.h>
@@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
 	exynos4_setup_clocks();
 }
 
+static void exynos4_gic_irq_eoi(struct irq_data *d)
+{
+	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
+
+	gic_data->cpu_base = S5P_VA_GIC_CPU +
+			    (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+}
+
 void __init exynos4_init_irq(void)
 {
 	int irq;
 
 	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
+	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
 
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index d8f38c2..4fad076 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -10,6 +10,7 @@
 */
 
 #include <mach/hardware.h>
+#include <mach/map.h>
 #include <asm/hardware/gic.h>
 
 		.macro	disable_fiq
@@ -18,6 +19,10 @@
 		.macro  get_irqnr_preamble, base, tmp
 		ldr	\base, =gic_cpu_base_addr
 		ldr	\base, [\base]
+		mrc     p15, 0, \tmp, c0, c0, 5
+		and     \tmp, \tmp, #3
+		cmp     \tmp, #1
+		addeq   \base, \base, #EXYNOS4_GIC_BANK_OFFSET
 		.endm
 
 		.macro  arch_ret_to_user, tmp1, tmp2
diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
index 9d5d797..2339a70 100644
--- a/arch/arm/mach-exynos4/include/mach/map.h
+++ b/arch/arm/mach-exynos4/include/mach/map.h
@@ -63,6 +63,7 @@
 
 #define EXYNOS4_PA_GIC_CPU		0x10480000
 #define EXYNOS4_PA_GIC_DIST		0x10490000
+#define EXYNOS4_GIC_BANK_OFFSET		0x8000
 
 #define EXYNOS4_PA_COREPERI		0x10500000
 #define EXYNOS4_PA_TWD			0x10500600
diff --git a/arch/arm/mach-exynos4/platsmp.c b/arch/arm/mach-exynos4/platsmp.c
index c5e65a0..decf528 100644
--- a/arch/arm/mach-exynos4/platsmp.c
+++ b/arch/arm/mach-exynos4/platsmp.c
@@ -58,6 +58,31 @@ static void __iomem *scu_base_addr(void)
 
 static DEFINE_SPINLOCK(boot_lock);
 
+static void __cpuinit exynos4_gic_secondary_init(void)
+{
+	void __iomem *dist_base = S5P_VA_GIC_DIST +
+				 (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+	void __iomem *cpu_base = S5P_VA_GIC_CPU +
+				(EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());
+	int i;
+
+	/*
+	 * Deal with the banked PPI and SGI interrupts - disable all
+	 * PPI interrupts, ensure all SGI interrupts are enabled.
+	 */
+	__raw_writel(0xffff0000, dist_base + GIC_DIST_ENABLE_CLEAR);
+	__raw_writel(0x0000ffff, dist_base + GIC_DIST_ENABLE_SET);
+
+	/*
+	 * Set priority on PPI and SGI interrupts
+	 */
+	for (i = 0; i < 32; i += 4)
+		__raw_writel(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
+
+	__raw_writel(0xf0, cpu_base + GIC_CPU_PRIMASK);
+	__raw_writel(1, cpu_base + GIC_CPU_CTRL);
+}
+
 void __cpuinit platform_secondary_init(unsigned int cpu)
 {
 	/*
@@ -65,7 +90,7 @@ void __cpuinit platform_secondary_init(unsigned int cpu)
 	 * core (e.g. timer irq), then they will not have been enabled
 	 * for us: do so
 	 */
-	gic_secondary_init(0);
+	exynos4_gic_secondary_init();
 
 	/*
 	 * let the primary processor know we're out of the
-- 
1.7.1

[PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers

[Changhwan Youn]

External GIC cannot support PPI (Private Peripheral Interrupt) for
ARM private timers. Thus MCT should be selected as clock event timers
by default.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/Kconfig                    |    3 +-
 arch/arm/mach-exynos4/Makefile                   |    7 +-
 arch/arm/mach-exynos4/cpu.c                      |    2 +-
 arch/arm/mach-exynos4/include/mach/entry-macro.S |    6 -
 arch/arm/mach-exynos4/include/mach/irqs.h        |    2 -
 arch/arm/mach-exynos4/localtimer.c               |   26 --
 arch/arm/mach-exynos4/time.c                     |  303 ----------------------
 7 files changed, 4 insertions(+), 345 deletions(-)
 delete mode 100644 arch/arm/mach-exynos4/localtimer.c
 delete mode 100644 arch/arm/mach-exynos4/time.c

diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index 1435fc3..0aca083 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -16,7 +16,8 @@ config CPU_EXYNOS4210
 	  Enable EXYNOS4210 CPU support
 
 config EXYNOS4_MCT
-	bool "Kernel timer support by MCT"
+	bool
+	default y
 	help
 	  Use MCT (Multi Core Timer) as kernel timers
 
diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 60fe5ec..c3c70ab 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -20,12 +20,7 @@ obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 
 obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
 
-ifeq ($(CONFIG_EXYNOS4_MCT),y)
-obj-y				+= mct.o
-else
-obj-y				+= time.o
-obj-$(CONFIG_LOCAL_TIMERS)	+= localtimer.o
-endif
+obj-$(CONFIG_EXYNOS4_MCT)	+= mct.o
 
 obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 
diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 40a866c..d153309 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -172,7 +172,7 @@ void __init exynos4_init_irq(void)
 {
 	int irq;
 
-	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
+	gic_init(0, IRQ_SPI(0), S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
 	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index 4fad076..d7a1e28 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -80,10 +80,4 @@
 		/* As above, this assumes that irqstat and base are preserved.. */
 
 		.macro test_for_ltirq, irqnr, irqstat, base, tmp
-		bic	\irqnr, \irqstat, #0x1c00
-		mov	\tmp, #0
-		cmp	\irqnr, #29
-		moveq	\tmp, #1
-		streq	\irqstat, [\base, #GIC_CPU_EOI]
-		cmp	\tmp, #0
 		.endm
diff --git a/arch/arm/mach-exynos4/include/mach/irqs.h b/arch/arm/mach-exynos4/include/mach/irqs.h
index e497ea2..250427f 100644
--- a/arch/arm/mach-exynos4/include/mach/irqs.h
+++ b/arch/arm/mach-exynos4/include/mach/irqs.h
@@ -19,8 +19,6 @@
 
 #define IRQ_PPI(x)		S5P_IRQ(x+16)
 
-#define IRQ_LOCALTIMER		IRQ_PPI(13)
-
 /* SPI: Shared Peripheral Interrupt */
 
 #define IRQ_SPI(x)		S5P_IRQ(x+32)
diff --git a/arch/arm/mach-exynos4/localtimer.c b/arch/arm/mach-exynos4/localtimer.c
deleted file mode 100644
index 6bf3d0a..0000000
--- a/arch/arm/mach-exynos4/localtimer.c
+++ /dev/null
@@ -1,26 +0,0 @@
-/* linux/arch/arm/mach-exynos4/localtimer.c
- *
- * Cloned from linux/arch/arm/mach-realview/localtimer.c
- *
- *  Copyright (C) 2002 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
-*/
-
-#include <linux/clockchips.h>
-
-#include <asm/irq.h>
-#include <asm/localtimer.h>
-
-/*
- * Setup the local clock events for a CPU.
- */
-int __cpuinit local_timer_setup(struct clock_event_device *evt)
-{
-	evt->irq = IRQ_LOCALTIMER;
-	twd_timer_setup(evt);
-	return 0;
-}
diff --git a/arch/arm/mach-exynos4/time.c b/arch/arm/mach-exynos4/time.c
deleted file mode 100644
index 9a74294..0000000
--- a/arch/arm/mach-exynos4/time.c
+++ /dev/null
@@ -1,303 +0,0 @@
-/* linux/arch/arm/mach-exynos4/time.c
- *
- * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
- *		http://www.samsung.com
- *
- * EXYNOS4 (and compatible) HRT support
- * PWM 2/4 is used for this feature
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
-*/
-
-#include <linux/sched.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/clockchips.h>
-#include <linux/platform_device.h>
-
-#include <asm/smp_twd.h>
-
-#include <mach/map.h>
-#include <plat/regs-timer.h>
-#include <asm/mach/time.h>
-
-static unsigned long clock_count_per_tick;
-
-static struct clk *tin2;
-static struct clk *tin4;
-static struct clk *tdiv2;
-static struct clk *tdiv4;
-static struct clk *timerclk;
-
-static void exynos4_pwm_stop(unsigned int pwm_id)
-{
-	unsigned long tcon;
-
-	tcon = __raw_readl(S3C2410_TCON);
-
-	switch (pwm_id) {
-	case 2:
-		tcon &= ~S3C2410_TCON_T2START;
-		break;
-	case 4:
-		tcon &= ~S3C2410_TCON_T4START;
-		break;
-	default:
-		break;
-	}
-	__raw_writel(tcon, S3C2410_TCON);
-}
-
-static void exynos4_pwm_init(unsigned int pwm_id, unsigned long tcnt)
-{
-	unsigned long tcon;
-
-	tcon = __raw_readl(S3C2410_TCON);
-
-	/* timers reload after counting zero, so reduce the count by 1 */
-	tcnt--;
-
-	/* ensure timer is stopped... */
-	switch (pwm_id) {
-	case 2:
-		tcon &= ~(0xf<<12);
-		tcon |= S3C2410_TCON_T2MANUALUPD;
-
-		__raw_writel(tcnt, S3C2410_TCNTB(2));
-		__raw_writel(tcnt, S3C2410_TCMPB(2));
-		__raw_writel(tcon, S3C2410_TCON);
-
-		break;
-	case 4:
-		tcon &= ~(7<<20);
-		tcon |= S3C2410_TCON_T4MANUALUPD;
-
-		__raw_writel(tcnt, S3C2410_TCNTB(4));
-		__raw_writel(tcnt, S3C2410_TCMPB(4));
-		__raw_writel(tcon, S3C2410_TCON);
-
-		break;
-	default:
-		break;
-	}
-}
-
-static inline void exynos4_pwm_start(unsigned int pwm_id, bool periodic)
-{
-	unsigned long tcon;
-
-	tcon  = __raw_readl(S3C2410_TCON);
-
-	switch (pwm_id) {
-	case 2:
-		tcon |= S3C2410_TCON_T2START;
-		tcon &= ~S3C2410_TCON_T2MANUALUPD;
-
-		if (periodic)
-			tcon |= S3C2410_TCON_T2RELOAD;
-		else
-			tcon &= ~S3C2410_TCON_T2RELOAD;
-		break;
-	case 4:
-		tcon |= S3C2410_TCON_T4START;
-		tcon &= ~S3C2410_TCON_T4MANUALUPD;
-
-		if (periodic)
-			tcon |= S3C2410_TCON_T4RELOAD;
-		else
-			tcon &= ~S3C2410_TCON_T4RELOAD;
-		break;
-	default:
-		break;
-	}
-	__raw_writel(tcon, S3C2410_TCON);
-}
-
-static int exynos4_pwm_set_next_event(unsigned long cycles,
-					struct clock_event_device *evt)
-{
-	exynos4_pwm_init(2, cycles);
-	exynos4_pwm_start(2, 0);
-	return 0;
-}
-
-static void exynos4_pwm_set_mode(enum clock_event_mode mode,
-				struct clock_event_device *evt)
-{
-	exynos4_pwm_stop(2);
-
-	switch (mode) {
-	case CLOCK_EVT_MODE_PERIODIC:
-		exynos4_pwm_init(2, clock_count_per_tick);
-		exynos4_pwm_start(2, 1);
-		break;
-	case CLOCK_EVT_MODE_ONESHOT:
-		break;
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_RESUME:
-		break;
-	}
-}
-
-static struct clock_event_device pwm_event_device = {
-	.name		= "pwm_timer2",
-	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
-	.rating		= 200,
-	.shift		= 32,
-	.set_next_event	= exynos4_pwm_set_next_event,
-	.set_mode	= exynos4_pwm_set_mode,
-};
-
-irqreturn_t exynos4_clock_event_isr(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = &pwm_event_device;
-
-	evt->event_handler(evt);
-
-	return IRQ_HANDLED;
-}
-
-static struct irqaction exynos4_clock_event_irq = {
-	.name		= "pwm_timer2_irq",
-	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
-	.handler	= exynos4_clock_event_isr,
-};
-
-static void __init exynos4_clockevent_init(void)
-{
-	unsigned long pclk;
-	unsigned long clock_rate;
-	struct clk *tscaler;
-
-	pclk = clk_get_rate(timerclk);
-
-	/* configure clock tick */
-
-	tscaler = clk_get_parent(tdiv2);
-
-	clk_set_rate(tscaler, pclk / 2);
-	clk_set_rate(tdiv2, pclk / 2);
-	clk_set_parent(tin2, tdiv2);
-
-	clock_rate = clk_get_rate(tin2);
-
-	clock_count_per_tick = clock_rate / HZ;
-
-	pwm_event_device.mult =
-		div_sc(clock_rate, NSEC_PER_SEC, pwm_event_device.shift);
-	pwm_event_device.max_delta_ns =
-		clockevent_delta2ns(-1, &pwm_event_device);
-	pwm_event_device.min_delta_ns =
-		clockevent_delta2ns(1, &pwm_event_device);
-
-	pwm_event_device.cpumask = cpumask_of(0);
-	clockevents_register_device(&pwm_event_device);
-
-	setup_irq(IRQ_TIMER2, &exynos4_clock_event_irq);
-}
-
-static cycle_t exynos4_pwm4_read(struct clocksource *cs)
-{
-	return (cycle_t) ~__raw_readl(S3C_TIMERREG(0x40));
-}
-
-#ifdef CONFIG_PM
-static void exynos4_pwm4_resume(struct clocksource *cs)
-{
-	unsigned long pclk;
-
-	pclk = clk_get_rate(timerclk);
-
-	clk_set_rate(tdiv4, pclk / 2);
-	clk_set_parent(tin4, tdiv4);
-
-	exynos4_pwm_init(4, ~0);
-	exynos4_pwm_start(4, 1);
-}
-#endif
-
-struct clocksource pwm_clocksource = {
-	.name		= "pwm_timer4",
-	.rating		= 250,
-	.read		= exynos4_pwm4_read,
-	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS ,
-#ifdef CONFIG_PM
-	.resume		= exynos4_pwm4_resume,
-#endif
-};
-
-static void __init exynos4_clocksource_init(void)
-{
-	unsigned long pclk;
-	unsigned long clock_rate;
-
-	pclk = clk_get_rate(timerclk);
-
-	clk_set_rate(tdiv4, pclk / 2);
-	clk_set_parent(tin4, tdiv4);
-
-	clock_rate = clk_get_rate(tin4);
-
-	exynos4_pwm_init(4, ~0);
-	exynos4_pwm_start(4, 1);
-
-	if (clocksource_register_hz(&pwm_clocksource, clock_rate))
-		panic("%s: can't register clocksource\n", pwm_clocksource.name);
-}
-
-static void __init exynos4_timer_resources(void)
-{
-	struct platform_device tmpdev;
-
-	tmpdev.dev.bus = &platform_bus_type;
-
-	timerclk = clk_get(NULL, "timers");
-	if (IS_ERR(timerclk))
-		panic("failed to get timers clock for system timer");
-
-	clk_enable(timerclk);
-
-	tmpdev.id = 2;
-	tmpdev.dev.init_name = "s3c24xx-pwm.2";
-	tin2 = clk_get(&tmpdev.dev, "pwm-tin");
-	if (IS_ERR(tin2))
-		panic("failed to get pwm-tin2 clock for system timer");
-
-	tdiv2 = clk_get(&tmpdev.dev, "pwm-tdiv");
-	if (IS_ERR(tdiv2))
-		panic("failed to get pwm-tdiv2 clock for system timer");
-	clk_enable(tin2);
-
-	tmpdev.id = 4;
-	tmpdev.dev.init_name = "s3c24xx-pwm.4";
-	tin4 = clk_get(&tmpdev.dev, "pwm-tin");
-	if (IS_ERR(tin4))
-		panic("failed to get pwm-tin4 clock for system timer");
-
-	tdiv4 = clk_get(&tmpdev.dev, "pwm-tdiv");
-	if (IS_ERR(tdiv4))
-		panic("failed to get pwm-tdiv4 clock for system timer");
-
-	clk_enable(tin4);
-}
-
-static void __init exynos4_timer_init(void)
-{
-#ifdef CONFIG_LOCAL_TIMERS
-	twd_base = S5P_VA_TWD;
-#endif
-
-	exynos4_timer_resources();
-	exynos4_clockevent_init();
-	exynos4_clocksource_init();
-}
-
-struct sys_timer exynos4_timer = {
-	.init		= exynos4_timer_init,
-};
-- 
1.7.1

[PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers

[Changhwan Youn]

External GIC cannot support PPI (Private Peripheral Interrupt) for
ARM private timers. Thus MCT should be selected as clock event timers
by default.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/mach-exynos4/Kconfig                    |    3 +-
 arch/arm/mach-exynos4/Makefile                   |    7 +-
 arch/arm/mach-exynos4/cpu.c                      |    2 +-
 arch/arm/mach-exynos4/include/mach/entry-macro.S |    6 -
 arch/arm/mach-exynos4/include/mach/irqs.h        |    2 -
 arch/arm/mach-exynos4/localtimer.c               |   26 --
 arch/arm/mach-exynos4/time.c                     |  303 ----------------------
 7 files changed, 4 insertions(+), 345 deletions(-)
 delete mode 100644 arch/arm/mach-exynos4/localtimer.c
 delete mode 100644 arch/arm/mach-exynos4/time.c

diff --git a/arch/arm/mach-exynos4/Kconfig b/arch/arm/mach-exynos4/Kconfig
index 1435fc3..0aca083 100644
--- a/arch/arm/mach-exynos4/Kconfig
+++ b/arch/arm/mach-exynos4/Kconfig
@@ -16,7 +16,8 @@ config CPU_EXYNOS4210
 	  Enable EXYNOS4210 CPU support
 
 config EXYNOS4_MCT
-	bool "Kernel timer support by MCT"
+	bool
+	default y
 	help
 	  Use MCT (Multi Core Timer) as kernel timers
 
diff --git a/arch/arm/mach-exynos4/Makefile b/arch/arm/mach-exynos4/Makefile
index 60fe5ec..c3c70ab 100644
--- a/arch/arm/mach-exynos4/Makefile
+++ b/arch/arm/mach-exynos4/Makefile
@@ -20,12 +20,7 @@ obj-$(CONFIG_CPU_IDLE)		+= cpuidle.o
 
 obj-$(CONFIG_SMP)		+= platsmp.o headsmp.o
 
-ifeq ($(CONFIG_EXYNOS4_MCT),y)
-obj-y				+= mct.o
-else
-obj-y				+= time.o
-obj-$(CONFIG_LOCAL_TIMERS)	+= localtimer.o
-endif
+obj-$(CONFIG_EXYNOS4_MCT)	+= mct.o
 
 obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug.o
 
diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index 40a866c..d153309 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -172,7 +172,7 @@ void __init exynos4_init_irq(void)
 {
 	int irq;
 
-	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
+	gic_init(0, IRQ_SPI(0), S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
 	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
diff --git a/arch/arm/mach-exynos4/include/mach/entry-macro.S b/arch/arm/mach-exynos4/include/mach/entry-macro.S
index 4fad076..d7a1e28 100644
--- a/arch/arm/mach-exynos4/include/mach/entry-macro.S
+++ b/arch/arm/mach-exynos4/include/mach/entry-macro.S
@@ -80,10 +80,4 @@
 		/* As above, this assumes that irqstat and base are preserved.. */
 
 		.macro test_for_ltirq, irqnr, irqstat, base, tmp
-		bic	\irqnr, \irqstat, #0x1c00
-		mov	\tmp, #0
-		cmp	\irqnr, #29
-		moveq	\tmp, #1
-		streq	\irqstat, [\base, #GIC_CPU_EOI]
-		cmp	\tmp, #0
 		.endm
diff --git a/arch/arm/mach-exynos4/include/mach/irqs.h b/arch/arm/mach-exynos4/include/mach/irqs.h
index e497ea2..250427f 100644
--- a/arch/arm/mach-exynos4/include/mach/irqs.h
+++ b/arch/arm/mach-exynos4/include/mach/irqs.h
@@ -19,8 +19,6 @@
 
 #define IRQ_PPI(x)		S5P_IRQ(x+16)
 
-#define IRQ_LOCALTIMER		IRQ_PPI(13)
-
 /* SPI: Shared Peripheral Interrupt */
 
 #define IRQ_SPI(x)		S5P_IRQ(x+32)
diff --git a/arch/arm/mach-exynos4/localtimer.c b/arch/arm/mach-exynos4/localtimer.c
deleted file mode 100644
index 6bf3d0a..0000000
--- a/arch/arm/mach-exynos4/localtimer.c
+++ /dev/null
@@ -1,26 +0,0 @@
-/* linux/arch/arm/mach-exynos4/localtimer.c
- *
- * Cloned from linux/arch/arm/mach-realview/localtimer.c
- *
- *  Copyright (C) 2002 ARM Ltd.
- *  All Rights Reserved
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
-*/
-
-#include <linux/clockchips.h>
-
-#include <asm/irq.h>
-#include <asm/localtimer.h>
-
-/*
- * Setup the local clock events for a CPU.
- */
-int __cpuinit local_timer_setup(struct clock_event_device *evt)
-{
-	evt->irq = IRQ_LOCALTIMER;
-	twd_timer_setup(evt);
-	return 0;
-}
diff --git a/arch/arm/mach-exynos4/time.c b/arch/arm/mach-exynos4/time.c
deleted file mode 100644
index 9a74294..0000000
--- a/arch/arm/mach-exynos4/time.c
+++ /dev/null
@@ -1,303 +0,0 @@
-/* linux/arch/arm/mach-exynos4/time.c
- *
- * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
- *		http://www.samsung.com
- *
- * EXYNOS4 (and compatible) HRT support
- * PWM 2/4 is used for this feature
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
-*/
-
-#include <linux/sched.h>
-#include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/err.h>
-#include <linux/clk.h>
-#include <linux/clockchips.h>
-#include <linux/platform_device.h>
-
-#include <asm/smp_twd.h>
-
-#include <mach/map.h>
-#include <plat/regs-timer.h>
-#include <asm/mach/time.h>
-
-static unsigned long clock_count_per_tick;
-
-static struct clk *tin2;
-static struct clk *tin4;
-static struct clk *tdiv2;
-static struct clk *tdiv4;
-static struct clk *timerclk;
-
-static void exynos4_pwm_stop(unsigned int pwm_id)
-{
-	unsigned long tcon;
-
-	tcon = __raw_readl(S3C2410_TCON);
-
-	switch (pwm_id) {
-	case 2:
-		tcon &= ~S3C2410_TCON_T2START;
-		break;
-	case 4:
-		tcon &= ~S3C2410_TCON_T4START;
-		break;
-	default:
-		break;
-	}
-	__raw_writel(tcon, S3C2410_TCON);
-}
-
-static void exynos4_pwm_init(unsigned int pwm_id, unsigned long tcnt)
-{
-	unsigned long tcon;
-
-	tcon = __raw_readl(S3C2410_TCON);
-
-	/* timers reload after counting zero, so reduce the count by 1 */
-	tcnt--;
-
-	/* ensure timer is stopped... */
-	switch (pwm_id) {
-	case 2:
-		tcon &= ~(0xf<<12);
-		tcon |= S3C2410_TCON_T2MANUALUPD;
-
-		__raw_writel(tcnt, S3C2410_TCNTB(2));
-		__raw_writel(tcnt, S3C2410_TCMPB(2));
-		__raw_writel(tcon, S3C2410_TCON);
-
-		break;
-	case 4:
-		tcon &= ~(7<<20);
-		tcon |= S3C2410_TCON_T4MANUALUPD;
-
-		__raw_writel(tcnt, S3C2410_TCNTB(4));
-		__raw_writel(tcnt, S3C2410_TCMPB(4));
-		__raw_writel(tcon, S3C2410_TCON);
-
-		break;
-	default:
-		break;
-	}
-}
-
-static inline void exynos4_pwm_start(unsigned int pwm_id, bool periodic)
-{
-	unsigned long tcon;
-
-	tcon  = __raw_readl(S3C2410_TCON);
-
-	switch (pwm_id) {
-	case 2:
-		tcon |= S3C2410_TCON_T2START;
-		tcon &= ~S3C2410_TCON_T2MANUALUPD;
-
-		if (periodic)
-			tcon |= S3C2410_TCON_T2RELOAD;
-		else
-			tcon &= ~S3C2410_TCON_T2RELOAD;
-		break;
-	case 4:
-		tcon |= S3C2410_TCON_T4START;
-		tcon &= ~S3C2410_TCON_T4MANUALUPD;
-
-		if (periodic)
-			tcon |= S3C2410_TCON_T4RELOAD;
-		else
-			tcon &= ~S3C2410_TCON_T4RELOAD;
-		break;
-	default:
-		break;
-	}
-	__raw_writel(tcon, S3C2410_TCON);
-}
-
-static int exynos4_pwm_set_next_event(unsigned long cycles,
-					struct clock_event_device *evt)
-{
-	exynos4_pwm_init(2, cycles);
-	exynos4_pwm_start(2, 0);
-	return 0;
-}
-
-static void exynos4_pwm_set_mode(enum clock_event_mode mode,
-				struct clock_event_device *evt)
-{
-	exynos4_pwm_stop(2);
-
-	switch (mode) {
-	case CLOCK_EVT_MODE_PERIODIC:
-		exynos4_pwm_init(2, clock_count_per_tick);
-		exynos4_pwm_start(2, 1);
-		break;
-	case CLOCK_EVT_MODE_ONESHOT:
-		break;
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_RESUME:
-		break;
-	}
-}
-
-static struct clock_event_device pwm_event_device = {
-	.name		= "pwm_timer2",
-	.features       = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
-	.rating		= 200,
-	.shift		= 32,
-	.set_next_event	= exynos4_pwm_set_next_event,
-	.set_mode	= exynos4_pwm_set_mode,
-};
-
-irqreturn_t exynos4_clock_event_isr(int irq, void *dev_id)
-{
-	struct clock_event_device *evt = &pwm_event_device;
-
-	evt->event_handler(evt);
-
-	return IRQ_HANDLED;
-}
-
-static struct irqaction exynos4_clock_event_irq = {
-	.name		= "pwm_timer2_irq",
-	.flags		= IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
-	.handler	= exynos4_clock_event_isr,
-};
-
-static void __init exynos4_clockevent_init(void)
-{
-	unsigned long pclk;
-	unsigned long clock_rate;
-	struct clk *tscaler;
-
-	pclk = clk_get_rate(timerclk);
-
-	/* configure clock tick */
-
-	tscaler = clk_get_parent(tdiv2);
-
-	clk_set_rate(tscaler, pclk / 2);
-	clk_set_rate(tdiv2, pclk / 2);
-	clk_set_parent(tin2, tdiv2);
-
-	clock_rate = clk_get_rate(tin2);
-
-	clock_count_per_tick = clock_rate / HZ;
-
-	pwm_event_device.mult =
-		div_sc(clock_rate, NSEC_PER_SEC, pwm_event_device.shift);
-	pwm_event_device.max_delta_ns =
-		clockevent_delta2ns(-1, &pwm_event_device);
-	pwm_event_device.min_delta_ns =
-		clockevent_delta2ns(1, &pwm_event_device);
-
-	pwm_event_device.cpumask = cpumask_of(0);
-	clockevents_register_device(&pwm_event_device);
-
-	setup_irq(IRQ_TIMER2, &exynos4_clock_event_irq);
-}
-
-static cycle_t exynos4_pwm4_read(struct clocksource *cs)
-{
-	return (cycle_t) ~__raw_readl(S3C_TIMERREG(0x40));
-}
-
-#ifdef CONFIG_PM
-static void exynos4_pwm4_resume(struct clocksource *cs)
-{
-	unsigned long pclk;
-
-	pclk = clk_get_rate(timerclk);
-
-	clk_set_rate(tdiv4, pclk / 2);
-	clk_set_parent(tin4, tdiv4);
-
-	exynos4_pwm_init(4, ~0);
-	exynos4_pwm_start(4, 1);
-}
-#endif
-
-struct clocksource pwm_clocksource = {
-	.name		= "pwm_timer4",
-	.rating		= 250,
-	.read		= exynos4_pwm4_read,
-	.mask		= CLOCKSOURCE_MASK(32),
-	.flags		= CLOCK_SOURCE_IS_CONTINUOUS ,
-#ifdef CONFIG_PM
-	.resume		= exynos4_pwm4_resume,
-#endif
-};
-
-static void __init exynos4_clocksource_init(void)
-{
-	unsigned long pclk;
-	unsigned long clock_rate;
-
-	pclk = clk_get_rate(timerclk);
-
-	clk_set_rate(tdiv4, pclk / 2);
-	clk_set_parent(tin4, tdiv4);
-
-	clock_rate = clk_get_rate(tin4);
-
-	exynos4_pwm_init(4, ~0);
-	exynos4_pwm_start(4, 1);
-
-	if (clocksource_register_hz(&pwm_clocksource, clock_rate))
-		panic("%s: can't register clocksource\n", pwm_clocksource.name);
-}
-
-static void __init exynos4_timer_resources(void)
-{
-	struct platform_device tmpdev;
-
-	tmpdev.dev.bus = &platform_bus_type;
-
-	timerclk = clk_get(NULL, "timers");
-	if (IS_ERR(timerclk))
-		panic("failed to get timers clock for system timer");
-
-	clk_enable(timerclk);
-
-	tmpdev.id = 2;
-	tmpdev.dev.init_name = "s3c24xx-pwm.2";
-	tin2 = clk_get(&tmpdev.dev, "pwm-tin");
-	if (IS_ERR(tin2))
-		panic("failed to get pwm-tin2 clock for system timer");
-
-	tdiv2 = clk_get(&tmpdev.dev, "pwm-tdiv");
-	if (IS_ERR(tdiv2))
-		panic("failed to get pwm-tdiv2 clock for system timer");
-	clk_enable(tin2);
-
-	tmpdev.id = 4;
-	tmpdev.dev.init_name = "s3c24xx-pwm.4";
-	tin4 = clk_get(&tmpdev.dev, "pwm-tin");
-	if (IS_ERR(tin4))
-		panic("failed to get pwm-tin4 clock for system timer");
-
-	tdiv4 = clk_get(&tmpdev.dev, "pwm-tdiv");
-	if (IS_ERR(tdiv4))
-		panic("failed to get pwm-tdiv4 clock for system timer");
-
-	clk_enable(tin4);
-}
-
-static void __init exynos4_timer_init(void)
-{
-#ifdef CONFIG_LOCAL_TIMERS
-	twd_base = S5P_VA_TWD;
-#endif
-
-	exynos4_timer_resources();
-	exynos4_clockevent_init();
-	exynos4_clocksource_init();
-}
-
-struct sys_timer exynos4_timer = {
-	.init		= exynos4_timer_init,
-};
-- 
1.7.1

[PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler

[Changhwan Youn]

This patch adds chained IRQ enter/exit functions to uart
interrupt handler in order to function correctly on primary
controllers with different methods of flow control.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/plat-samsung/irq-uart.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/irq-uart.c b/arch/arm/plat-samsung/irq-uart.c
index 32582c0..8960eaf 100644
--- a/arch/arm/plat-samsung/irq-uart.c
+++ b/arch/arm/plat-samsung/irq-uart.c
@@ -19,6 +19,8 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 
+#include <asm/mach/irq.h>
+
 #include <mach/map.h>
 #include <plat/irq-uart.h>
 #include <plat/regs-serial.h>
@@ -30,9 +32,12 @@
 static void s3c_irq_demux_uart(unsigned int irq, struct irq_desc *desc)
 {
 	struct s3c_uart_irq *uirq = desc->irq_data.handler_data;
+	struct irq_chip *chip = irq_get_chip(irq);
 	u32 pend = __raw_readl(uirq->regs + S3C64XX_UINTP);
 	int base = uirq->base_irq;
 
+	chained_irq_enter(chip, desc);
+
 	if (pend & (1 << 0))
 		generic_handle_irq(base);
 	if (pend & (1 << 1))
@@ -41,6 +46,8 @@ static void s3c_irq_demux_uart(unsigned int irq, struct irq_desc *desc)
 		generic_handle_irq(base + 2);
 	if (pend & (1 << 3))
 		generic_handle_irq(base + 3);
+
+	chained_irq_exit(chip, desc);
 }
 
 static void __init s3c_init_uart_irq(struct s3c_uart_irq *uirq)
-- 
1.7.1

[PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler

[Changhwan Youn]

This patch adds chained IRQ enter/exit functions to uart
interrupt handler in order to function correctly on primary
controllers with different methods of flow control.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
---
 arch/arm/plat-samsung/irq-uart.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-samsung/irq-uart.c b/arch/arm/plat-samsung/irq-uart.c
index 32582c0..8960eaf 100644
--- a/arch/arm/plat-samsung/irq-uart.c
+++ b/arch/arm/plat-samsung/irq-uart.c
@@ -19,6 +19,8 @@
 #include <linux/irq.h>
 #include <linux/io.h>
 
+#include <asm/mach/irq.h>
+
 #include <mach/map.h>
 #include <plat/irq-uart.h>
 #include <plat/regs-serial.h>
@@ -30,9 +32,12 @@
 static void s3c_irq_demux_uart(unsigned int irq, struct irq_desc *desc)
 {
 	struct s3c_uart_irq *uirq = desc->irq_data.handler_data;
+	struct irq_chip *chip = irq_get_chip(irq);
 	u32 pend = __raw_readl(uirq->regs + S3C64XX_UINTP);
 	int base = uirq->base_irq;
 
+	chained_irq_enter(chip, desc);
+
 	if (pend & (1 << 0))
 		generic_handle_irq(base);
 	if (pend & (1 << 1))
@@ -41,6 +46,8 @@ static void s3c_irq_demux_uart(unsigned int irq, struct irq_desc *desc)
 		generic_handle_irq(base + 2);
 	if (pend & (1 << 3))
 		generic_handle_irq(base + 3);
+
+	chained_irq_exit(chip, desc);
 }
 
 static void __init s3c_init_uart_irq(struct s3c_uart_irq *uirq)
-- 
1.7.1

Re: [PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Kyungmin Park]

Hi,

There are some boards which use the EVT0 chip. some SMDKV310 and
Universal-C210.
With this changes, it can't use these boards.

Do you want to remove EVT0 based boards?

Thank you,
Kyungmin Park

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds implementation External GIC on EXYNOS4 SoC.
>
> Note: need to update timer codes for supporting old type of
> EXYNOS4 SoCs.
>
> [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
> [PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
> [PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1
> [PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
> [PATCH 5/7] ARM: EXYNOS4: Add support external GIC
> [PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers
> [PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Kyungmin Park]

Hi,

There are some boards which use the EVT0 chip. some SMDKV310 and
Universal-C210.
With this changes, it can't use these boards.

Do you want to remove EVT0 based boards?

Thank you,
Kyungmin Park

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds implementation External GIC on EXYNOS4 SoC.
>
> Note: need to update timer codes for supporting old type of
> EXYNOS4 SoCs.
>
> [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
> [PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
> [PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using IRQ_MCT_L1
> [PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
> [PATCH 5/7] ARM: EXYNOS4: Add support external GIC
> [PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private timers
> [PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart interrupt handler
> --
> 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
>

Re: [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[MyungJoo Ham]

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds external GIC io memory mapping
> to support external GIC on EXYNOS4.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/mach-exynos4/cpu.c              |   10 ++++++++++
>  arch/arm/mach-exynos4/include/mach/map.h |    5 +++--
>  arch/arm/plat-s5p/include/plat/map-s5p.h |    5 +++--
>  3 files changed, 16 insertions(+), 4 deletions(-)
>

Hello,

It appears that something is missing in the patch series.
A patch related with "AUDSS" is required before applying this patch.
I get patch fail errors at cpu.c and map-s5p.h.


Cheers!

- MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[MyungJoo Ham]

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds external GIC io memory mapping
> to support external GIC on EXYNOS4.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
> ?arch/arm/mach-exynos4/cpu.c ? ? ? ? ? ? ?| ? 10 ++++++++++
> ?arch/arm/mach-exynos4/include/mach/map.h | ? ?5 +++--
> ?arch/arm/plat-s5p/include/plat/map-s5p.h | ? ?5 +++--
> ?3 files changed, 16 insertions(+), 4 deletions(-)
>

Hello,

It appears that something is missing in the patch series.
A patch related with "AUDSS" is required before applying this patch.
I get patch fail errors at cpu.c and map-s5p.h.


Cheers!

- MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

Re: [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[MyungJoo Ham]

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds external GIC io memory mapping
> to support external GIC on EXYNOS4.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/mach-exynos4/cpu.c              |   10 ++++++++++
>  arch/arm/mach-exynos4/include/mach/map.h |    5 +++--
>  arch/arm/plat-s5p/include/plat/map-s5p.h |    5 +++--
>  3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> index 1196f39..6a1ed74 100644
> --- a/arch/arm/mach-exynos4/cpu.c
> +++ b/arch/arm/mach-exynos4/cpu.c
> @@ -107,6 +107,16 @@ static struct map_desc exynos4_iodesc[] __initdata = {
>                .pfn            = __phys_to_pfn(EXYNOS4_PA_AUDSS),
>                .length         = SZ_1K,
>                .type           = MT_DEVICE,
> +       }, {
> +               .virtual        = (unsigned long)S5P_VA_GIC_CPU,
> +               .pfn            = __phys_to_pfn(EXYNOS4_PA_GIC_CPU),
> +               .length         = SZ_64K,
> +               .type           = MT_DEVICE,
> +       }, {
> +               .virtual        = (unsigned long)S5P_VA_GIC_DIST,
> +               .pfn            = __phys_to_pfn(EXYNOS4_PA_GIC_DIST),
> +               .length         = SZ_64K,
> +               .type           = MT_DEVICE,
>        },
>  };
>
> diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
> index 57d8074..9d5d797 100644
> --- a/arch/arm/mach-exynos4/include/mach/map.h
> +++ b/arch/arm/mach-exynos4/include/mach/map.h
> @@ -61,10 +61,11 @@
>
>  #define EXYNOS4_PA_COMBINER            0x10448000

Isn't this 0x10440000 if we are going to use External GIC?

The user manual says it's 0x10440000 for Ext-GIC and 0x10448000 for
Internal-GIC.


-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[MyungJoo Ham]

On Mon, Jun 20, 2011 at 4:34 PM, Changhwan Youn <chaos.youn@samsung.com> wrote:
> This patch adds external GIC io memory mapping
> to support external GIC on EXYNOS4.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
> ?arch/arm/mach-exynos4/cpu.c ? ? ? ? ? ? ?| ? 10 ++++++++++
> ?arch/arm/mach-exynos4/include/mach/map.h | ? ?5 +++--
> ?arch/arm/plat-s5p/include/plat/map-s5p.h | ? ?5 +++--
> ?3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> index 1196f39..6a1ed74 100644
> --- a/arch/arm/mach-exynos4/cpu.c
> +++ b/arch/arm/mach-exynos4/cpu.c
> @@ -107,6 +107,16 @@ static struct map_desc exynos4_iodesc[] __initdata = {
> ? ? ? ? ? ? ? ?.pfn ? ? ? ? ? ?= __phys_to_pfn(EXYNOS4_PA_AUDSS),
> ? ? ? ? ? ? ? ?.length ? ? ? ? = SZ_1K,
> ? ? ? ? ? ? ? ?.type ? ? ? ? ? = MT_DEVICE,
> + ? ? ? }, {
> + ? ? ? ? ? ? ? .virtual ? ? ? ?= (unsigned long)S5P_VA_GIC_CPU,
> + ? ? ? ? ? ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(EXYNOS4_PA_GIC_CPU),
> + ? ? ? ? ? ? ? .length ? ? ? ? = SZ_64K,
> + ? ? ? ? ? ? ? .type ? ? ? ? ? = MT_DEVICE,
> + ? ? ? }, {
> + ? ? ? ? ? ? ? .virtual ? ? ? ?= (unsigned long)S5P_VA_GIC_DIST,
> + ? ? ? ? ? ? ? .pfn ? ? ? ? ? ?= __phys_to_pfn(EXYNOS4_PA_GIC_DIST),
> + ? ? ? ? ? ? ? .length ? ? ? ? = SZ_64K,
> + ? ? ? ? ? ? ? .type ? ? ? ? ? = MT_DEVICE,
> ? ? ? ?},
> ?};
>
> diff --git a/arch/arm/mach-exynos4/include/mach/map.h b/arch/arm/mach-exynos4/include/mach/map.h
> index 57d8074..9d5d797 100644
> --- a/arch/arm/mach-exynos4/include/mach/map.h
> +++ b/arch/arm/mach-exynos4/include/mach/map.h
> @@ -61,10 +61,11 @@
>
> ?#define EXYNOS4_PA_COMBINER ? ? ? ? ? ?0x10448000

Isn't this 0x10440000 if we are going to use External GIC?

The user manual says it's 0x10440000 for Ext-GIC and 0x10448000 for
Internal-GIC.


-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

RE: [PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header

[Kukjin Kim]

Changhwan Youn wrote:
> 
> Since Samsung EXYNOS4210 cannot support register banking in GIC,
> so needs to update CPU interface base address.
> The 'gic_chip_data' is used for it, this patch moves gic_chip_data
> structure declaraton to arch/arm/include/asm/hardware/gic.h to use
> it.
> 
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>

Russell, how do you think this?

As I know, this is needed on Samsung SoC, EXYNOS4210.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/common/gic.c               |    6 ------
>  arch/arm/include/asm/hardware/gic.h |    6 ++++++
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 4ddd0a6..23564ed 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -38,12 +38,6 @@ static DEFINE_SPINLOCK(irq_controller_lock);
>  /* Address of GIC 0 CPU interface */
>  void __iomem *gic_cpu_base_addr __read_mostly;
> 
> -struct gic_chip_data {
> -	unsigned int irq_offset;
> -	void __iomem *dist_base;
> -	void __iomem *cpu_base;
> -};
> -
>  /*
>   * Supported arch specific GIC irq extension.
>   * Default make them NULL.
> diff --git a/arch/arm/include/asm/hardware/gic.h
> b/arch/arm/include/asm/hardware/gic.h
> index 0691f9d..435d3f8 100644
> --- a/arch/arm/include/asm/hardware/gic.h
> +++ b/arch/arm/include/asm/hardware/gic.h
> @@ -41,6 +41,12 @@ void gic_secondary_init(unsigned int);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
>  void gic_enable_ppi(unsigned int);
> +
> +struct gic_chip_data {
> +	unsigned int irq_offset;
> +	void __iomem *dist_base;
> +	void __iomem *cpu_base;
> +};
>  #endif
> 
>  #endif
> --
> 1.7.1

[PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header

[Kukjin Kim]

Changhwan Youn wrote:
> 
> Since Samsung EXYNOS4210 cannot support register banking in GIC,
> so needs to update CPU interface base address.
> The 'gic_chip_data' is used for it, this patch moves gic_chip_data
> structure declaraton to arch/arm/include/asm/hardware/gic.h to use
> it.
> 
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>

Russell, how do you think this?

As I know, this is needed on Samsung SoC, EXYNOS4210.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/common/gic.c               |    6 ------
>  arch/arm/include/asm/hardware/gic.h |    6 ++++++
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 4ddd0a6..23564ed 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -38,12 +38,6 @@ static DEFINE_SPINLOCK(irq_controller_lock);
>  /* Address of GIC 0 CPU interface */
>  void __iomem *gic_cpu_base_addr __read_mostly;
> 
> -struct gic_chip_data {
> -	unsigned int irq_offset;
> -	void __iomem *dist_base;
> -	void __iomem *cpu_base;
> -};
> -
>  /*
>   * Supported arch specific GIC irq extension.
>   * Default make them NULL.
> diff --git a/arch/arm/include/asm/hardware/gic.h
> b/arch/arm/include/asm/hardware/gic.h
> index 0691f9d..435d3f8 100644
> --- a/arch/arm/include/asm/hardware/gic.h
> +++ b/arch/arm/include/asm/hardware/gic.h
> @@ -41,6 +41,12 @@ void gic_secondary_init(unsigned int);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
>  void gic_enable_ppi(unsigned int);
> +
> +struct gic_chip_data {
> +	unsigned int irq_offset;
> +	void __iomem *dist_base;
> +	void __iomem *cpu_base;
> +};
>  #endif
> 
>  #endif
> --
> 1.7.1

RE: [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> >  #define EXYNOS4_PA_COMBINER            0x10448000
> 
> Isn't this 0x10440000 if we are going to use External GIC?
> 
Oops, you're right, thanks for your pointing out.
Yes, should be 0x10440000.

When I apply this, I will fix it.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping

[Kukjin Kim]

MyungJoo Ham wrote:
> 
> > ?#define EXYNOS4_PA_COMBINER ? ? ? ? ? ?0x10448000
> 
> Isn't this 0x10440000 if we are going to use External GIC?
> 
Oops, you're right, thanks for your pointing out.
Yes, should be 0x10440000.

When I apply this, I will fix it.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

RE: [PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Kukjin Kim]

Changhwan Youn wrote:
> 
> This patch adds implementation External GIC on EXYNOS4 SoC.
> 
> Note: need to update timer codes for supporting old type of
> EXYNOS4 SoCs.
> 
> [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
> [PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
> [PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using
IRQ_MCT_L1
> [PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
> [PATCH 5/7] ARM: EXYNOS4: Add support external GIC
> [PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private
timers
> [PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart
interrupt
> handler

OK, applied.
Thanks.

As a note, this is needed for to support upcoming other EXYNOS4 SoCs with
current exynos4 mainline.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 0/7] ARM: EXYNOS4: Adds External GIC

[Kukjin Kim]

Changhwan Youn wrote:
> 
> This patch adds implementation External GIC on EXYNOS4 SoC.
> 
> Note: need to update timer codes for supporting old type of
> EXYNOS4 SoCs.
> 
> [PATCH 1/7] ARM: EXYNOS4: Add external GIC io memory mapping
> [PATCH 2/7] ARM: EXYNOS4: modify interrupt mappings for external GIC
> [PATCH 3/7] ARM: EXYNOS4: set the affinity of mct1 interrupt using
IRQ_MCT_L1
> [PATCH 4/7] ARM: GIC: move gic_chip_data structure declaration to header
> [PATCH 5/7] ARM: EXYNOS4: Add support external GIC
> [PATCH 6/7] ARM: EXYNOS4: Remove clock event timers using ARM private
timers
> [PATCH 7/7] ARM: EXYNOS4: Add chained enrty/exit function to uart
interrupt
> handler

OK, applied.
Thanks.

As a note, this is needed for to support upcoming other EXYNOS4 SoCs with
current exynos4 mainline.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

Hi Changhwan,

On 20/06/11 08:34, Changhwan Youn wrote:
> For full support of power modes, this patch adds implementation
> external GIC on EXYNOS4.
> 
> External GIC of Exynos4 cannot support register banking so
> several interrupt related code for CPU1 should be different
> from that of CPU0.

I just realized that patch has made it to mainline... Unfortunately, it
seems quite broken to me:

> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>  arch/arm/mach-exynos4/platsmp.c                  |   27 +++++++++++++++++++++-
>  4 files changed, 42 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> index fa33294..40a866c 100644
> --- a/arch/arm/mach-exynos4/cpu.c
> +++ b/arch/arm/mach-exynos4/cpu.c
> @@ -16,6 +16,7 @@
>  
>  #include <asm/proc-fns.h>
>  #include <asm/hardware/cache-l2x0.h>
> +#include <asm/hardware/gic.h>
>  
>  #include <plat/cpu.h>
>  #include <plat/clock.h>
> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>  	exynos4_setup_clocks();
>  }
>  
> +static void exynos4_gic_irq_eoi(struct irq_data *d)
> +{
> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> +
> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> +			    (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());

Here, you're overwriting a field that is shared among *all* the
interrupts in the system.  What if an interrupt comes up on another CPU?
If you look at the implementation of gic_eoi_irq(), you'll definitely
see the race.

> +}
> +
>  void __init exynos4_init_irq(void)
>  {
>  	int irq;
>  
>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;

And here you're abusing the GIC extension feature.

I've also had a look at -next, and this has been extended further to
support 4412. The problem with that is without banking, you're painfully
working around the GIC driver. At that stage, I wonder if you wouldn't
be better off with a separate driver instead of abusing the existing one...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

Hi Changhwan,

On 20/06/11 08:34, Changhwan Youn wrote:
> For full support of power modes, this patch adds implementation
> external GIC on EXYNOS4.
> 
> External GIC of Exynos4 cannot support register banking so
> several interrupt related code for CPU1 should be different
> from that of CPU0.

I just realized that patch has made it to mainline... Unfortunately, it
seems quite broken to me:

> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> ---
>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>  arch/arm/mach-exynos4/platsmp.c                  |   27 +++++++++++++++++++++-
>  4 files changed, 42 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> index fa33294..40a866c 100644
> --- a/arch/arm/mach-exynos4/cpu.c
> +++ b/arch/arm/mach-exynos4/cpu.c
> @@ -16,6 +16,7 @@
>  
>  #include <asm/proc-fns.h>
>  #include <asm/hardware/cache-l2x0.h>
> +#include <asm/hardware/gic.h>
>  
>  #include <plat/cpu.h>
>  #include <plat/clock.h>
> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>  	exynos4_setup_clocks();
>  }
>  
> +static void exynos4_gic_irq_eoi(struct irq_data *d)
> +{
> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> +
> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> +			    (EXYNOS4_GIC_BANK_OFFSET * smp_processor_id());

Here, you're overwriting a field that is shared among *all* the
interrupts in the system.  What if an interrupt comes up on another CPU?
If you look at the implementation of gic_eoi_irq(), you'll definitely
see the race.

> +}
> +
>  void __init exynos4_init_irq(void)
>  {
>  	int irq;
>  
>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;

And here you're abusing the GIC extension feature.

I've also had a look at -next, and this has been extended further to
support 4412. The problem with that is without banking, you're painfully
working around the GIC driver. At that stage, I wonder if you wouldn't
be better off with a separate driver instead of abusing the existing one...

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> Hi Changhwan,
> 
Hi Marc,

(Cc'ed Will Deacon and Russell King)

> On 20/06/11 08:34, Changhwan Youn wrote:
> > For full support of power modes, this patch adds implementation
> > external GIC on EXYNOS4.
> >
> > External GIC of Exynos4 cannot support register banking so
> > several interrupt related code for CPU1 should be different
> > from that of CPU0.
> 
> I just realized that patch has made it to mainline... Unfortunately, it
> seems quite broken to me:
> 
> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> > ---
> >  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
> >  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
> >  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
> >  arch/arm/mach-exynos4/platsmp.c                  |   27
> +++++++++++++++++++++-
> >  4 files changed, 42 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> > index fa33294..40a866c 100644
> > --- a/arch/arm/mach-exynos4/cpu.c
> > +++ b/arch/arm/mach-exynos4/cpu.c
> > @@ -16,6 +16,7 @@
> >
> >  #include <asm/proc-fns.h>
> >  #include <asm/hardware/cache-l2x0.h>
> > +#include <asm/hardware/gic.h>
> >
> >  #include <plat/cpu.h>
> >  #include <plat/clock.h>
> > @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
> >  	exynos4_setup_clocks();
> >  }
> >
> > +static void exynos4_gic_irq_eoi(struct irq_data *d)
> > +{
> > +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> > +
> > +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> > +			    (EXYNOS4_GIC_BANK_OFFSET *
> smp_processor_id());
> 
> Here, you're overwriting a field that is shared among *all* the
> interrupts in the system.  What if an interrupt comes up on another CPU?
> If you look at the implementation of gic_eoi_irq(), you'll definitely
> see the race.
> 
Hmm...as you can see in git log, the EXYNOS4210 cannot support register
banking in GIC so this is needed.

> > +}
> > +
> >  void __init exynos4_init_irq(void)
> >  {
> >  	int irq;
> >
> >  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
> > +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
> 
> And here you're abusing the GIC extension feature.
> 
I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
support own specific extensions like in the EXYNOS4 case.

> I've also had a look at -next, and this has been extended further to
> support 4412. The problem with that is without banking, you're painfully
> working around the GIC driver. At that stage, I wonder if you wouldn't
> be better off with a separate driver instead of abusing the existing
one...
> 
Well, in this case, you mean separate driver is better to us even though
there is a gic driver in arch/arm/common? I don't think so because separate
driver will probably have many duplicated codes and if common gic driver can
support every silicons which have different version's gic it's better to us
and should do.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> Hi Changhwan,
> 
Hi Marc,

(Cc'ed Will Deacon and Russell King)

> On 20/06/11 08:34, Changhwan Youn wrote:
> > For full support of power modes, this patch adds implementation
> > external GIC on EXYNOS4.
> >
> > External GIC of Exynos4 cannot support register banking so
> > several interrupt related code for CPU1 should be different
> > from that of CPU0.
> 
> I just realized that patch has made it to mainline... Unfortunately, it
> seems quite broken to me:
> 
> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> > ---
> >  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
> >  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
> >  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
> >  arch/arm/mach-exynos4/platsmp.c                  |   27
> +++++++++++++++++++++-
> >  4 files changed, 42 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
> > index fa33294..40a866c 100644
> > --- a/arch/arm/mach-exynos4/cpu.c
> > +++ b/arch/arm/mach-exynos4/cpu.c
> > @@ -16,6 +16,7 @@
> >
> >  #include <asm/proc-fns.h>
> >  #include <asm/hardware/cache-l2x0.h>
> > +#include <asm/hardware/gic.h>
> >
> >  #include <plat/cpu.h>
> >  #include <plat/clock.h>
> > @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
> >  	exynos4_setup_clocks();
> >  }
> >
> > +static void exynos4_gic_irq_eoi(struct irq_data *d)
> > +{
> > +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
> > +
> > +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> > +			    (EXYNOS4_GIC_BANK_OFFSET *
> smp_processor_id());
> 
> Here, you're overwriting a field that is shared among *all* the
> interrupts in the system.  What if an interrupt comes up on another CPU?
> If you look at the implementation of gic_eoi_irq(), you'll definitely
> see the race.
> 
Hmm...as you can see in git log, the EXYNOS4210 cannot support register
banking in GIC so this is needed.

> > +}
> > +
> >  void __init exynos4_init_irq(void)
> >  {
> >  	int irq;
> >
> >  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
> > +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
> 
> And here you're abusing the GIC extension feature.
> 
I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
support own specific extensions like in the EXYNOS4 case.

> I've also had a look at -next, and this has been extended further to
> support 4412. The problem with that is without banking, you're painfully
> working around the GIC driver. At that stage, I wonder if you wouldn't
> be better off with a separate driver instead of abusing the existing
one...
> 
Well, in this case, you mean separate driver is better to us even though
there is a gic driver in arch/arm/common? I don't think so because separate
driver will probably have many duplicated codes and if common gic driver can
support every silicons which have different version's gic it's better to us
and should do.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 06/10/11 07:30, Kukjin Kim wrote:
> Marc Zyngier wrote:
>>
>> Hi Changhwan,
>>
> Hi Marc,
> 
> (Cc'ed Will Deacon and Russell King)
> 
>> On 20/06/11 08:34, Changhwan Youn wrote:
>>> For full support of power modes, this patch adds implementation
>>> external GIC on EXYNOS4.
>>>
>>> External GIC of Exynos4 cannot support register banking so
>>> several interrupt related code for CPU1 should be different
>>> from that of CPU0.
>>
>> I just realized that patch has made it to mainline... Unfortunately, it
>> seems quite broken to me:
>>
>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
>>> ---
>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
>> +++++++++++++++++++++-
>>>  4 files changed, 42 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
>>> index fa33294..40a866c 100644
>>> --- a/arch/arm/mach-exynos4/cpu.c
>>> +++ b/arch/arm/mach-exynos4/cpu.c
>>> @@ -16,6 +16,7 @@
>>>
>>>  #include <asm/proc-fns.h>
>>>  #include <asm/hardware/cache-l2x0.h>
>>> +#include <asm/hardware/gic.h>
>>>
>>>  #include <plat/cpu.h>
>>>  #include <plat/clock.h>
>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>>>  	exynos4_setup_clocks();
>>>  }
>>>
>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
>>> +{
>>> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
>>> +
>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
>> smp_processor_id());
>>
>> Here, you're overwriting a field that is shared among *all* the
>> interrupts in the system.  What if an interrupt comes up on another CPU?
>> If you look at the implementation of gic_eoi_irq(), you'll definitely
>> see the race.
>>
> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
> banking in GIC so this is needed.

I don't dispute the need. I claim that the implementation is wrong, and
will fail given the right timings.

>>> +}
>>> +
>>>  void __init exynos4_init_irq(void)
>>>  {
>>>  	int irq;
>>>
>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
>>
>> And here you're abusing the GIC extension feature.
>>
> I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
> support own specific extensions like in the EXYNOS4 case.

Sure. My point is you are diverting the GIC extension from its purpose,
which is mostly to be able to control wake-up sources (as for example in
the Tegra case). Here, you use this hooks to work around the fact that
the GIC driver is written with banking in mind, which is quite a
different thing.

>> I've also had a look at -next, and this has been extended further to
>> support 4412. The problem with that is without banking, you're painfully
>> working around the GIC driver. At that stage, I wonder if you wouldn't
>> be better off with a separate driver instead of abusing the existing
> one...
>>
> Well, in this case, you mean separate driver is better to us even though
> there is a gic driver in arch/arm/common? I don't think so because separate
> driver will probably have many duplicated codes and if common gic driver can
> support every silicons which have different version's gic it's better to us
> and should do.

If you really insist on using the GIC common code, then I'd suggest to
adapt it to your needs instead of working around the problem.
What about making cpu_base a percpu field inside struct gic_chip_data?
No hook abuse, and no race conditions. You could also do that for
dist_base, as it looks to be required for the 4412.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 06/10/11 07:30, Kukjin Kim wrote:
> Marc Zyngier wrote:
>>
>> Hi Changhwan,
>>
> Hi Marc,
> 
> (Cc'ed Will Deacon and Russell King)
> 
>> On 20/06/11 08:34, Changhwan Youn wrote:
>>> For full support of power modes, this patch adds implementation
>>> external GIC on EXYNOS4.
>>>
>>> External GIC of Exynos4 cannot support register banking so
>>> several interrupt related code for CPU1 should be different
>>> from that of CPU0.
>>
>> I just realized that patch has made it to mainline... Unfortunately, it
>> seems quite broken to me:
>>
>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
>>> ---
>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
>> +++++++++++++++++++++-
>>>  4 files changed, 42 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
>>> index fa33294..40a866c 100644
>>> --- a/arch/arm/mach-exynos4/cpu.c
>>> +++ b/arch/arm/mach-exynos4/cpu.c
>>> @@ -16,6 +16,7 @@
>>>
>>>  #include <asm/proc-fns.h>
>>>  #include <asm/hardware/cache-l2x0.h>
>>> +#include <asm/hardware/gic.h>
>>>
>>>  #include <plat/cpu.h>
>>>  #include <plat/clock.h>
>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>>>  	exynos4_setup_clocks();
>>>  }
>>>
>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
>>> +{
>>> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
>>> +
>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
>> smp_processor_id());
>>
>> Here, you're overwriting a field that is shared among *all* the
>> interrupts in the system.  What if an interrupt comes up on another CPU?
>> If you look at the implementation of gic_eoi_irq(), you'll definitely
>> see the race.
>>
> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
> banking in GIC so this is needed.

I don't dispute the need. I claim that the implementation is wrong, and
will fail given the right timings.

>>> +}
>>> +
>>>  void __init exynos4_init_irq(void)
>>>  {
>>>  	int irq;
>>>
>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
>>
>> And here you're abusing the GIC extension feature.
>>
> I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
> support own specific extensions like in the EXYNOS4 case.

Sure. My point is you are diverting the GIC extension from its purpose,
which is mostly to be able to control wake-up sources (as for example in
the Tegra case). Here, you use this hooks to work around the fact that
the GIC driver is written with banking in mind, which is quite a
different thing.

>> I've also had a look at -next, and this has been extended further to
>> support 4412. The problem with that is without banking, you're painfully
>> working around the GIC driver. At that stage, I wonder if you wouldn't
>> be better off with a separate driver instead of abusing the existing
> one...
>>
> Well, in this case, you mean separate driver is better to us even though
> there is a gic driver in arch/arm/common? I don't think so because separate
> driver will probably have many duplicated codes and if common gic driver can
> support every silicons which have different version's gic it's better to us
> and should do.

If you really insist on using the GIC common code, then I'd suggest to
adapt it to your needs instead of working around the problem.
What about making cpu_base a percpu field inside struct gic_chip_data?
No hook abuse, and no race conditions. You could also do that for
dist_base, as it looks to be required for the 4412.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

[-- Attachment #1: Type: text/plain, Size: 4469 bytes --]

On 06/10/11 09:18, Marc Zyngier wrote:
> On 06/10/11 07:30, Kukjin Kim wrote:
>> Marc Zyngier wrote:
>>>
>>> Hi Changhwan,
>>>
>> Hi Marc,
>>
>> (Cc'ed Will Deacon and Russell King)
>>
>>> On 20/06/11 08:34, Changhwan Youn wrote:
>>>> For full support of power modes, this patch adds implementation
>>>> external GIC on EXYNOS4.
>>>>
>>>> External GIC of Exynos4 cannot support register banking so
>>>> several interrupt related code for CPU1 should be different
>>>> from that of CPU0.
>>>
>>> I just realized that patch has made it to mainline... Unfortunately, it
>>> seems quite broken to me:
>>>
>>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
>>>> ---
>>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
>>> +++++++++++++++++++++-
>>>>  4 files changed, 42 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
>>>> index fa33294..40a866c 100644
>>>> --- a/arch/arm/mach-exynos4/cpu.c
>>>> +++ b/arch/arm/mach-exynos4/cpu.c
>>>> @@ -16,6 +16,7 @@
>>>>
>>>>  #include <asm/proc-fns.h>
>>>>  #include <asm/hardware/cache-l2x0.h>
>>>> +#include <asm/hardware/gic.h>
>>>>
>>>>  #include <plat/cpu.h>
>>>>  #include <plat/clock.h>
>>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>>>>  	exynos4_setup_clocks();
>>>>  }
>>>>
>>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
>>>> +{
>>>> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
>>>> +
>>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
>>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
>>> smp_processor_id());
>>>
>>> Here, you're overwriting a field that is shared among *all* the
>>> interrupts in the system.  What if an interrupt comes up on another CPU?
>>> If you look at the implementation of gic_eoi_irq(), you'll definitely
>>> see the race.
>>>
>> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
>> banking in GIC so this is needed.
> 
> I don't dispute the need. I claim that the implementation is wrong, and
> will fail given the right timings.
> 
>>>> +}
>>>> +
>>>>  void __init exynos4_init_irq(void)
>>>>  {
>>>>  	int irq;
>>>>
>>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
>>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
>>>
>>> And here you're abusing the GIC extension feature.
>>>
>> I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
>> support own specific extensions like in the EXYNOS4 case.
> 
> Sure. My point is you are diverting the GIC extension from its purpose,
> which is mostly to be able to control wake-up sources (as for example in
> the Tegra case). Here, you use this hooks to work around the fact that
> the GIC driver is written with banking in mind, which is quite a
> different thing.
> 
>>> I've also had a look at -next, and this has been extended further to
>>> support 4412. The problem with that is without banking, you're painfully
>>> working around the GIC driver. At that stage, I wonder if you wouldn't
>>> be better off with a separate driver instead of abusing the existing
>> one...
>>>
>> Well, in this case, you mean separate driver is better to us even though
>> there is a gic driver in arch/arm/common? I don't think so because separate
>> driver will probably have many duplicated codes and if common gic driver can
>> support every silicons which have different version's gic it's better to us
>> and should do.
> 
> If you really insist on using the GIC common code, then I'd suggest to
> adapt it to your needs instead of working around the problem.
> What about making cpu_base a percpu field inside struct gic_chip_data?
> No hook abuse, and no race conditions. You could also do that for
> dist_base, as it looks to be required for the 4412.

So to make my suggestion completely clear, here's a patch I'm now
carrying in my tree. It's only been test compiled on EXYNOS4, but works
nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
variables, removes these callbacks, removes your private copy of
gic_cpu_init, and makes struct gic_chip_data private again.

What do you think?

	M.
-- 
Jazz is not dead. It just smells funny...

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ARM-gic-allow-GIC-to-support-non-banked-setups.patch --]
[-- Type: text/x-patch; name=0001-ARM-gic-allow-GIC-to-support-non-banked-setups.patch, Size: 10440 bytes --]

>From da57c3979543fcef47f1ae22b5224a1d7a96aa2d Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Fri, 7 Oct 2011 10:23:31 +0100
Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups

The GIC support code is heavily using the fact that hardware
implementations are exposing banked registers. Unfortunately, it
looks like at least one GIC implementation (EXYNOS4) offers both
the distributor and the CPU interfaces at different addresses,
depending on the CPU.

This problem is solved by turning the distributor and CPU interface
addresses into per-cpu variables. The EXYNOS4 code is updated not
to mess with the GIC internals while handling interrupts, and
struct gic_chip_data is back to being private.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
 arch/arm/include/asm/hardware/gic.h |   17 +------
 arch/arm/mach-exynos4/cpu.c         |   14 ------
 arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
 4 files changed, 66 insertions(+), 69 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index b574931..c7521dd 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -37,6 +37,20 @@
 #include <asm/mach/irq.h>
 #include <asm/hardware/gic.h>
 
+struct gic_chip_data {
+	unsigned int irq_offset;
+	void __percpu __iomem **dist_base;
+	void __percpu __iomem **cpu_base;
+#ifdef CONFIG_CPU_PM
+	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
+	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
+	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
+	u32 __percpu *saved_ppi_enable;
+	u32 __percpu *saved_ppi_conf;
+#endif
+	unsigned int gic_irqs;
+};
+
 static DEFINE_RAW_SPINLOCK(irq_controller_lock);
 
 /* Address of GIC 0 CPU interface */
@@ -61,16 +75,26 @@ struct irq_chip gic_arch_extn = {
 
 static struct gic_chip_data gic_data[MAX_GIC_NR] __read_mostly;
 
+static inline void __iomem *gic_data_dist_base(struct gic_chip_data *data)
+{
+	return *__this_cpu_ptr(data->dist_base);
+}
+
 static inline void __iomem *gic_dist_base(struct irq_data *d)
 {
 	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
-	return gic_data->dist_base;
+	return gic_data_dist_base(gic_data);
+}
+
+static inline void __iomem *gic_data_cpu_base(struct gic_chip_data *data)
+{
+	return *__this_cpu_ptr(data->cpu_base);
 }
 
 static inline void __iomem *gic_cpu_base(struct irq_data *d)
 {
 	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
-	return gic_data->cpu_base;
+	return gic_data_cpu_base(gic_data);
 }
 
 static inline unsigned int gic_irq(struct irq_data *d)
@@ -243,7 +267,7 @@ static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
 	chained_irq_enter(chip, desc);
 
 	raw_spin_lock(&irq_controller_lock);
-	status = readl_relaxed(chip_data->cpu_base + GIC_CPU_INTACK);
+	status = readl_relaxed(gic_data_cpu_base(chip_data) + GIC_CPU_INTACK);
 	raw_spin_unlock(&irq_controller_lock);
 
 	gic_irq = (status & 0x3ff);
@@ -287,7 +311,7 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 {
 	unsigned int gic_irqs, irq_limit, i;
 	u32 cpumask;
-	void __iomem *base = gic->dist_base;
+	void __iomem *base = gic_data_dist_base(gic);
 	u32 cpu = 0;
 	u32 nrppis = 0, ppi_base = 0;
 
@@ -380,8 +404,8 @@ static void __init gic_dist_init(struct gic_chip_data *gic,
 
 static void __cpuinit gic_cpu_init(struct gic_chip_data *gic)
 {
-	void __iomem *dist_base = gic->dist_base;
-	void __iomem *base = gic->cpu_base;
+	void __iomem *dist_base = gic_data_dist_base(gic);
+	void __iomem *base = gic_data_cpu_base(gic);
 	int i;
 
 	/*
@@ -418,7 +442,7 @@ static void gic_dist_save(unsigned int gic_nr)
 		BUG();
 
 	gic_irqs = gic_data[gic_nr].gic_irqs;
-	dist_base = gic_data[gic_nr].dist_base;
+	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
 
 	if (!dist_base)
 		return;
@@ -453,7 +477,7 @@ static void gic_dist_restore(unsigned int gic_nr)
 		BUG();
 
 	gic_irqs = gic_data[gic_nr].gic_irqs;
-	dist_base = gic_data[gic_nr].dist_base;
+	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
 
 	if (!dist_base)
 		return;
@@ -489,8 +513,8 @@ static void gic_cpu_save(unsigned int gic_nr)
 	if (gic_nr >= MAX_GIC_NR)
 		BUG();
 
-	dist_base = gic_data[gic_nr].dist_base;
-	cpu_base = gic_data[gic_nr].cpu_base;
+	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 
 	if (!dist_base || !cpu_base)
 		return;
@@ -515,8 +539,8 @@ static void gic_cpu_restore(unsigned int gic_nr)
 	if (gic_nr >= MAX_GIC_NR)
 		BUG();
 
-	dist_base = gic_data[gic_nr].dist_base;
-	cpu_base = gic_data[gic_nr].cpu_base;
+	dist_base = gic_data_dist_base(&gic_data[gic_nr]);
+	cpu_base = gic_data_cpu_base(&gic_data[gic_nr]);
 
 	if (!dist_base || !cpu_base)
 		return;
@@ -588,12 +612,24 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 	void __iomem *dist_base, void __iomem *cpu_base)
 {
 	struct gic_chip_data *gic;
+	int cpu;
 
 	BUG_ON(gic_nr >= MAX_GIC_NR);
 
 	gic = &gic_data[gic_nr];
-	gic->dist_base = dist_base;
-	gic->cpu_base = cpu_base;
+	gic->dist_base = alloc_percpu(void __iomem *);
+	gic->cpu_base = alloc_percpu(void __iomem *);
+	if (WARN_ON(!gic->dist_base || !gic->cpu_base)) {
+		free_percpu(gic->dist_base);
+		free_percpu(gic->cpu_base);
+		return;
+	}
+
+	for_each_possible_cpu(cpu) {
+		*per_cpu_ptr(gic->dist_base, cpu) = dist_base;
+		*per_cpu_ptr(gic->cpu_base, cpu) = cpu_base;
+	}
+
 	gic->irq_offset = (irq_start - 1) & ~31;
 
 	if (gic_nr == 0)
@@ -605,11 +641,17 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 	gic_pm_init(gic);
 }
 
-void __cpuinit gic_secondary_init(unsigned int gic_nr)
+void __cpuinit gic_secondary_init_base(unsigned int gic_nr,
+				       void __iomem *dist_base,
+				       void __iomem *cpu_base)
 {
 	BUG_ON(gic_nr >= MAX_GIC_NR);
 
-	gic_cpu_init(&gic_data[gic_nr]);
+	if (dist_base)
+		*__this_cpu_ptr(gic_data[gic_nr].dist_base) = dist_base;
+	if (cpu_base)
+		*__this_cpu_ptr(gic_data[gic_nr].cpu_base) = cpu_base;
+	gic_cpu_init(&gic_data[gic_nr]);	
 }
 
 #ifdef CONFIG_SMP
@@ -629,6 +671,6 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	dsb();
 
 	/* this always happens on GIC0 */
-	writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
+	writel_relaxed(map << 16 | irq, gic_data_dist_base(&gic_data[0]) + GIC_DIST_SOFTINT);
 }
 #endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 2b7ec2b..d45d78b 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -36,24 +36,13 @@
 extern struct irq_chip gic_arch_extn;
 
 void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
-void gic_secondary_init(unsigned int);
+void gic_secondary_init_base(unsigned int, void __iomem *, void __iomem *);
 void gic_handle_irq(struct pt_regs *regs);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
 
-struct gic_chip_data {
-	unsigned int irq_offset;
-	void __iomem *dist_base;
-	void __iomem *cpu_base;
-#ifdef CONFIG_CPU_PM
-	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
-	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
-	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
-	u32 __percpu *saved_ppi_enable;
-	u32 __percpu *saved_ppi_conf;
-#endif
-	unsigned int gic_irqs;
-};
+#define gic_secondary_init(n)	gic_secondary_init_base((n), NULL, NULL)
+
 #endif
 
 #endif
diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
index c682887..09e2760 100644
--- a/arch/arm/mach-exynos4/cpu.c
+++ b/arch/arm/mach-exynos4/cpu.c
@@ -204,17 +204,6 @@ void __init exynos4_init_clocks(int xtal)
 	exynos4_setup_clocks();
 }
 
-static void exynos4_gic_irq_fix_base(struct irq_data *d)
-{
-	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
-
-	gic_data->cpu_base = S5P_VA_GIC_CPU +
-			    (gic_bank_offset * smp_processor_id());
-
-	gic_data->dist_base = S5P_VA_GIC_DIST +
-			    (gic_bank_offset * smp_processor_id());
-}
-
 void __init exynos4_init_irq(void)
 {
 	int irq;
@@ -222,9 +211,6 @@ void __init exynos4_init_irq(void)
 	gic_bank_offset = soc_is_exynos4412() ? 0x4000 : 0x8000;
 
 	gic_init(0, IRQ_PPI(0), S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
-	gic_arch_extn.irq_eoi = exynos4_gic_irq_fix_base;
-	gic_arch_extn.irq_unmask = exynos4_gic_irq_fix_base;
-	gic_arch_extn.irq_mask = exynos4_gic_irq_fix_base;
 
 	for (irq = 0; irq < MAX_COMBINER_NR; irq++) {
 
diff --git a/arch/arm/mach-exynos4/platsmp.c b/arch/arm/mach-exynos4/platsmp.c
index 56377d2..76cb2ba 100644
--- a/arch/arm/mach-exynos4/platsmp.c
+++ b/arch/arm/mach-exynos4/platsmp.c
@@ -67,39 +67,19 @@ static void __iomem *scu_base_addr(void)
 
 static DEFINE_SPINLOCK(boot_lock);
 
-static void __cpuinit exynos4_gic_secondary_init(void)
+static void __cpuinit exynos4_secondary_init(unsigned int cpu)
 {
 	void __iomem *dist_base = S5P_VA_GIC_DIST +
-				(gic_bank_offset * smp_processor_id());
+				(gic_bank_offset * cpu_logical_map(cpu));
 	void __iomem *cpu_base = S5P_VA_GIC_CPU +
-				(gic_bank_offset * smp_processor_id());
-	int i;
-
-	/*
-	 * Deal with the banked PPI and SGI interrupts - disable all
-	 * PPI interrupts, ensure all SGI interrupts are enabled.
-	 */
-	__raw_writel(0xffff0000, dist_base + GIC_DIST_ENABLE_CLEAR);
-	__raw_writel(0x0000ffff, dist_base + GIC_DIST_ENABLE_SET);
+				(gic_bank_offset * cpu_logical_map(cpu));
 
 	/*
-	 * Set priority on PPI and SGI interrupts
-	 */
-	for (i = 0; i < 32; i += 4)
-		__raw_writel(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
-
-	__raw_writel(0xf0, cpu_base + GIC_CPU_PRIMASK);
-	__raw_writel(1, cpu_base + GIC_CPU_CTRL);
-}
-
-static void __cpuinit exynos4_secondary_init(unsigned int cpu)
-{
-	/*
 	 * if any interrupts are already enabled for the primary
 	 * core (e.g. timer irq), then they will not have been enabled
 	 * for us: do so
 	 */
-	exynos4_gic_secondary_init();
+	gic_secondary_init_base(0, dist_base, cpu_base);
 
 	/*
 	 * let the primary processor know we're out of the
-- 
1.7.0.4

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 06/10/11 09:18, Marc Zyngier wrote:
> On 06/10/11 07:30, Kukjin Kim wrote:
>> Marc Zyngier wrote:
>>>
>>> Hi Changhwan,
>>>
>> Hi Marc,
>>
>> (Cc'ed Will Deacon and Russell King)
>>
>>> On 20/06/11 08:34, Changhwan Youn wrote:
>>>> For full support of power modes, this patch adds implementation
>>>> external GIC on EXYNOS4.
>>>>
>>>> External GIC of Exynos4 cannot support register banking so
>>>> several interrupt related code for CPU1 should be different
>>>> from that of CPU0.
>>>
>>> I just realized that patch has made it to mainline... Unfortunately, it
>>> seems quite broken to me:
>>>
>>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
>>>> ---
>>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
>>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
>>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
>>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
>>> +++++++++++++++++++++-
>>>>  4 files changed, 42 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos4/cpu.c b/arch/arm/mach-exynos4/cpu.c
>>>> index fa33294..40a866c 100644
>>>> --- a/arch/arm/mach-exynos4/cpu.c
>>>> +++ b/arch/arm/mach-exynos4/cpu.c
>>>> @@ -16,6 +16,7 @@
>>>>
>>>>  #include <asm/proc-fns.h>
>>>>  #include <asm/hardware/cache-l2x0.h>
>>>> +#include <asm/hardware/gic.h>
>>>>
>>>>  #include <plat/cpu.h>
>>>>  #include <plat/clock.h>
>>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
>>>>  	exynos4_setup_clocks();
>>>>  }
>>>>
>>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
>>>> +{
>>>> +	struct gic_chip_data *gic_data = irq_data_get_irq_chip_data(d);
>>>> +
>>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
>>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
>>> smp_processor_id());
>>>
>>> Here, you're overwriting a field that is shared among *all* the
>>> interrupts in the system.  What if an interrupt comes up on another CPU?
>>> If you look at the implementation of gic_eoi_irq(), you'll definitely
>>> see the race.
>>>
>> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
>> banking in GIC so this is needed.
> 
> I don't dispute the need. I claim that the implementation is wrong, and
> will fail given the right timings.
> 
>>>> +}
>>>> +
>>>>  void __init exynos4_init_irq(void)
>>>>  {
>>>>  	int irq;
>>>>
>>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST, S5P_VA_GIC_CPU);
>>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
>>>
>>> And here you're abusing the GIC extension feature.
>>>
>> I think gic_arch_extn.irq_eoi can be overwritten in each architecture to
>> support own specific extensions like in the EXYNOS4 case.
> 
> Sure. My point is you are diverting the GIC extension from its purpose,
> which is mostly to be able to control wake-up sources (as for example in
> the Tegra case). Here, you use this hooks to work around the fact that
> the GIC driver is written with banking in mind, which is quite a
> different thing.
> 
>>> I've also had a look at -next, and this has been extended further to
>>> support 4412. The problem with that is without banking, you're painfully
>>> working around the GIC driver. At that stage, I wonder if you wouldn't
>>> be better off with a separate driver instead of abusing the existing
>> one...
>>>
>> Well, in this case, you mean separate driver is better to us even though
>> there is a gic driver in arch/arm/common? I don't think so because separate
>> driver will probably have many duplicated codes and if common gic driver can
>> support every silicons which have different version's gic it's better to us
>> and should do.
> 
> If you really insist on using the GIC common code, then I'd suggest to
> adapt it to your needs instead of working around the problem.
> What about making cpu_base a percpu field inside struct gic_chip_data?
> No hook abuse, and no race conditions. You could also do that for
> dist_base, as it looks to be required for the 4412.

So to make my suggestion completely clear, here's a patch I'm now
carrying in my tree. It's only been test compiled on EXYNOS4, but works
nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
variables, removes these callbacks, removes your private copy of
gic_cpu_init, and makes struct gic_chip_data private again.

What do you think?

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> On 06/10/11 09:18, Marc Zyngier wrote:
> > On 06/10/11 07:30, Kukjin Kim wrote:
> >> Marc Zyngier wrote:
> >>>
> >>> Hi Changhwan,
> >>>
> >> Hi Marc,
> >>
> >> (Cc'ed Will Deacon and Russell King)
> >>
> >>> On 20/06/11 08:34, Changhwan Youn wrote:
> >>>> For full support of power modes, this patch adds implementation
> >>>> external GIC on EXYNOS4.
> >>>>
> >>>> External GIC of Exynos4 cannot support register banking so
> >>>> several interrupt related code for CPU1 should be different
> >>>> from that of CPU0.
> >>>
> >>> I just realized that patch has made it to mainline... Unfortunately,
it
> >>> seems quite broken to me:
> >>>
> >>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> >>>> ---
> >>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
> >>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
> >>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
> >>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
> >>> +++++++++++++++++++++-
> >>>>  4 files changed, 42 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/mach-exynos4/cpu.c
b/arch/arm/mach-exynos4/cpu.c
> >>>> index fa33294..40a866c 100644
> >>>> --- a/arch/arm/mach-exynos4/cpu.c
> >>>> +++ b/arch/arm/mach-exynos4/cpu.c
> >>>> @@ -16,6 +16,7 @@
> >>>>
> >>>>  #include <asm/proc-fns.h>
> >>>>  #include <asm/hardware/cache-l2x0.h>
> >>>> +#include <asm/hardware/gic.h>
> >>>>
> >>>>  #include <plat/cpu.h>
> >>>>  #include <plat/clock.h>
> >>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
> >>>>  	exynos4_setup_clocks();
> >>>>  }
> >>>>
> >>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
> >>>> +{
> >>>> +	struct gic_chip_data *gic_data =
irq_data_get_irq_chip_data(d);
> >>>> +
> >>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> >>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
> >>> smp_processor_id());
> >>>
> >>> Here, you're overwriting a field that is shared among *all* the
> >>> interrupts in the system.  What if an interrupt comes up on another
CPU?
> >>> If you look at the implementation of gic_eoi_irq(), you'll definitely
> >>> see the race.
> >>>
> >> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
> >> banking in GIC so this is needed.
> >
> > I don't dispute the need. I claim that the implementation is wrong, and
> > will fail given the right timings.
> >
> >>>> +}
> >>>> +
> >>>>  void __init exynos4_init_irq(void)
> >>>>  {
> >>>>  	int irq;
> >>>>
> >>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST,
S5P_VA_GIC_CPU);
> >>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
> >>>
> >>> And here you're abusing the GIC extension feature.
> >>>
> >> I think gic_arch_extn.irq_eoi can be overwritten in each architecture
to
> >> support own specific extensions like in the EXYNOS4 case.
> >
> > Sure. My point is you are diverting the GIC extension from its purpose,
> > which is mostly to be able to control wake-up sources (as for example in
> > the Tegra case). Here, you use this hooks to work around the fact that
> > the GIC driver is written with banking in mind, which is quite a
> > different thing.
> >
> >>> I've also had a look at -next, and this has been extended further to
> >>> support 4412. The problem with that is without banking, you're
painfully
> >>> working around the GIC driver. At that stage, I wonder if you wouldn't
> >>> be better off with a separate driver instead of abusing the existing
> >> one...
> >>>
> >> Well, in this case, you mean separate driver is better to us even
though
> >> there is a gic driver in arch/arm/common? I don't think so because
separate
> >> driver will probably have many duplicated codes and if common gic
driver can
> >> support every silicons which have different version's gic it's better
to us
> >> and should do.
> >
> > If you really insist on using the GIC common code, then I'd suggest to
> > adapt it to your needs instead of working around the problem.
> > What about making cpu_base a percpu field inside struct gic_chip_data?
> > No hook abuse, and no race conditions. You could also do that for
> > dist_base, as it looks to be required for the 4412.
> 
> So to make my suggestion completely clear, here's a patch I'm now
> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> variables, removes these callbacks, removes your private copy of
> gic_cpu_init, and makes struct gic_chip_data private again.
> 
> What do you think?

Let me check this soon. Actually I need to sort this out to test on my board
with my git.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> On 06/10/11 09:18, Marc Zyngier wrote:
> > On 06/10/11 07:30, Kukjin Kim wrote:
> >> Marc Zyngier wrote:
> >>>
> >>> Hi Changhwan,
> >>>
> >> Hi Marc,
> >>
> >> (Cc'ed Will Deacon and Russell King)
> >>
> >>> On 20/06/11 08:34, Changhwan Youn wrote:
> >>>> For full support of power modes, this patch adds implementation
> >>>> external GIC on EXYNOS4.
> >>>>
> >>>> External GIC of Exynos4 cannot support register banking so
> >>>> several interrupt related code for CPU1 should be different
> >>>> from that of CPU0.
> >>>
> >>> I just realized that patch has made it to mainline... Unfortunately,
it
> >>> seems quite broken to me:
> >>>
> >>>> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> >>>> ---
> >>>>  arch/arm/mach-exynos4/cpu.c                      |   10 ++++++++
> >>>>  arch/arm/mach-exynos4/include/mach/entry-macro.S |    5 ++++
> >>>>  arch/arm/mach-exynos4/include/mach/map.h         |    1 +
> >>>>  arch/arm/mach-exynos4/platsmp.c                  |   27
> >>> +++++++++++++++++++++-
> >>>>  4 files changed, 42 insertions(+), 1 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm/mach-exynos4/cpu.c
b/arch/arm/mach-exynos4/cpu.c
> >>>> index fa33294..40a866c 100644
> >>>> --- a/arch/arm/mach-exynos4/cpu.c
> >>>> +++ b/arch/arm/mach-exynos4/cpu.c
> >>>> @@ -16,6 +16,7 @@
> >>>>
> >>>>  #include <asm/proc-fns.h>
> >>>>  #include <asm/hardware/cache-l2x0.h>
> >>>> +#include <asm/hardware/gic.h>
> >>>>
> >>>>  #include <plat/cpu.h>
> >>>>  #include <plat/clock.h>
> >>>> @@ -159,11 +160,20 @@ void __init exynos4_init_clocks(int xtal)
> >>>>  	exynos4_setup_clocks();
> >>>>  }
> >>>>
> >>>> +static void exynos4_gic_irq_eoi(struct irq_data *d)
> >>>> +{
> >>>> +	struct gic_chip_data *gic_data =
irq_data_get_irq_chip_data(d);
> >>>> +
> >>>> +	gic_data->cpu_base = S5P_VA_GIC_CPU +
> >>>> +			    (EXYNOS4_GIC_BANK_OFFSET *
> >>> smp_processor_id());
> >>>
> >>> Here, you're overwriting a field that is shared among *all* the
> >>> interrupts in the system.  What if an interrupt comes up on another
CPU?
> >>> If you look at the implementation of gic_eoi_irq(), you'll definitely
> >>> see the race.
> >>>
> >> Hmm...as you can see in git log, the EXYNOS4210 cannot support register
> >> banking in GIC so this is needed.
> >
> > I don't dispute the need. I claim that the implementation is wrong, and
> > will fail given the right timings.
> >
> >>>> +}
> >>>> +
> >>>>  void __init exynos4_init_irq(void)
> >>>>  {
> >>>>  	int irq;
> >>>>
> >>>>  	gic_init(0, IRQ_LOCALTIMER, S5P_VA_GIC_DIST,
S5P_VA_GIC_CPU);
> >>>> +	gic_arch_extn.irq_eoi = exynos4_gic_irq_eoi;
> >>>
> >>> And here you're abusing the GIC extension feature.
> >>>
> >> I think gic_arch_extn.irq_eoi can be overwritten in each architecture
to
> >> support own specific extensions like in the EXYNOS4 case.
> >
> > Sure. My point is you are diverting the GIC extension from its purpose,
> > which is mostly to be able to control wake-up sources (as for example in
> > the Tegra case). Here, you use this hooks to work around the fact that
> > the GIC driver is written with banking in mind, which is quite a
> > different thing.
> >
> >>> I've also had a look at -next, and this has been extended further to
> >>> support 4412. The problem with that is without banking, you're
painfully
> >>> working around the GIC driver. At that stage, I wonder if you wouldn't
> >>> be better off with a separate driver instead of abusing the existing
> >> one...
> >>>
> >> Well, in this case, you mean separate driver is better to us even
though
> >> there is a gic driver in arch/arm/common? I don't think so because
separate
> >> driver will probably have many duplicated codes and if common gic
driver can
> >> support every silicons which have different version's gic it's better
to us
> >> and should do.
> >
> > If you really insist on using the GIC common code, then I'd suggest to
> > adapt it to your needs instead of working around the problem.
> > What about making cpu_base a percpu field inside struct gic_chip_data?
> > No hook abuse, and no race conditions. You could also do that for
> > dist_base, as it looks to be required for the 4412.
> 
> So to make my suggestion completely clear, here's a patch I'm now
> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> variables, removes these callbacks, removes your private copy of
> gic_cpu_init, and makes struct gic_chip_data private again.
> 
> What do you think?

Let me check this soon. Actually I need to sort this out to test on my board
with my git.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Will Deacon]

Hi Marc,

On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> So to make my suggestion completely clear, here's a patch I'm now
> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> variables, removes these callbacks, removes your private copy of
> gic_cpu_init, and makes struct gic_chip_data private again.
> 
> What do you think?

This looks like the right sort of idea, although I'm deeply suspicious about
per-cpu base addresses for the GIC distributor. It would be nice if the
Samsung guys can elaborate on what's going on here. Few comments inline.

> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
> 
> The GIC support code is heavily using the fact that hardware
> implementations are exposing banked registers. Unfortunately, it
> looks like at least one GIC implementation (EXYNOS4) offers both
> the distributor and the CPU interfaces at different addresses,
> depending on the CPU.
> 
> This problem is solved by turning the distributor and CPU interface
> addresses into per-cpu variables. The EXYNOS4 code is updated not
> to mess with the GIC internals while handling interrupts, and
> struct gic_chip_data is back to being private.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
>  arch/arm/include/asm/hardware/gic.h |   17 +------
>  arch/arm/mach-exynos4/cpu.c         |   14 ------
>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
>  4 files changed, 66 insertions(+), 69 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index b574931..c7521dd 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -37,6 +37,20 @@
>  #include <asm/mach/irq.h>
>  #include <asm/hardware/gic.h>
>  
> +struct gic_chip_data {
> +	unsigned int irq_offset;
> +	void __percpu __iomem **dist_base;
> +	void __percpu __iomem **cpu_base;
> +#ifdef CONFIG_CPU_PM
> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
> +	u32 __percpu *saved_ppi_enable;
> +	u32 __percpu *saved_ppi_conf;
> +#endif
> +	unsigned int gic_irqs;
> +};

I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.

> +static inline void __iomem *gic_data_dist_base(struct gic_chip_data *data)
> +{
> +	return *__this_cpu_ptr(data->dist_base);
> +}

Then you can use __get_cpu_var here (same applies throughout).

> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
>  {
>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> -				(gic_bank_offset * smp_processor_id());
> +				(gic_bank_offset * cpu_logical_map(cpu));

Again, I'm deeply suspicious of this code :) Is there not a common memory
alias for the distributor across all of the CPUs?

Will

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Will Deacon]

Hi Marc,

On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> So to make my suggestion completely clear, here's a patch I'm now
> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> variables, removes these callbacks, removes your private copy of
> gic_cpu_init, and makes struct gic_chip_data private again.
> 
> What do you think?

This looks like the right sort of idea, although I'm deeply suspicious about
per-cpu base addresses for the GIC distributor. It would be nice if the
Samsung guys can elaborate on what's going on here. Few comments inline.

> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
> 
> The GIC support code is heavily using the fact that hardware
> implementations are exposing banked registers. Unfortunately, it
> looks like at least one GIC implementation (EXYNOS4) offers both
> the distributor and the CPU interfaces at different addresses,
> depending on the CPU.
> 
> This problem is solved by turning the distributor and CPU interface
> addresses into per-cpu variables. The EXYNOS4 code is updated not
> to mess with the GIC internals while handling interrupts, and
> struct gic_chip_data is back to being private.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
>  arch/arm/include/asm/hardware/gic.h |   17 +------
>  arch/arm/mach-exynos4/cpu.c         |   14 ------
>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
>  4 files changed, 66 insertions(+), 69 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index b574931..c7521dd 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -37,6 +37,20 @@
>  #include <asm/mach/irq.h>
>  #include <asm/hardware/gic.h>
>  
> +struct gic_chip_data {
> +	unsigned int irq_offset;
> +	void __percpu __iomem **dist_base;
> +	void __percpu __iomem **cpu_base;
> +#ifdef CONFIG_CPU_PM
> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
> +	u32 __percpu *saved_ppi_enable;
> +	u32 __percpu *saved_ppi_conf;
> +#endif
> +	unsigned int gic_irqs;
> +};

I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.

> +static inline void __iomem *gic_data_dist_base(struct gic_chip_data *data)
> +{
> +	return *__this_cpu_ptr(data->dist_base);
> +}

Then you can use __get_cpu_var here (same applies throughout).

> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
>  {
>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> -				(gic_bank_offset * smp_processor_id());
> +				(gic_bank_offset * cpu_logical_map(cpu));

Again, I'm deeply suspicious of this code :) Is there not a common memory
alias for the distributor across all of the CPUs?

Will

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 07/10/11 16:16, Will Deacon wrote:
> Hi Marc,
> 
> On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
>> So to make my suggestion completely clear, here's a patch I'm now
>> carrying in my tree. It's only been test compiled on EXYNOS4, but works
>> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
>> variables, removes these callbacks, removes your private copy of
>> gic_cpu_init, and makes struct gic_chip_data private again.
>>
>> What do you think?
> 
> This looks like the right sort of idea, although I'm deeply suspicious about
> per-cpu base addresses for the GIC distributor. It would be nice if the
> Samsung guys can elaborate on what's going on here. Few comments inline.

Yeah, this seem odd, specially as what's currently in mainline doesn't
try to play tricks with the distributor. It looks like having a per-core
distributor address is required for PPI usage on these SoCs, but the
commit message is uninformative at best:

https://github.com/sfrothwell/linux-next/commit/637c2afa57ec9cd0ddc8879ea0cda4d8835ba71d#arch/arm/mach-exynos4/cpu.c

Again, comments from Samsung people would be much appreciated.

>> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
>>
>> The GIC support code is heavily using the fact that hardware
>> implementations are exposing banked registers. Unfortunately, it
>> looks like at least one GIC implementation (EXYNOS4) offers both
>> the distributor and the CPU interfaces at different addresses,
>> depending on the CPU.
>>
>> This problem is solved by turning the distributor and CPU interface
>> addresses into per-cpu variables. The EXYNOS4 code is updated not
>> to mess with the GIC internals while handling interrupts, and
>> struct gic_chip_data is back to being private.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
>>  arch/arm/include/asm/hardware/gic.h |   17 +------
>>  arch/arm/mach-exynos4/cpu.c         |   14 ------
>>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
>>  4 files changed, 66 insertions(+), 69 deletions(-)
>>
>> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
>> index b574931..c7521dd 100644
>> --- a/arch/arm/common/gic.c
>> +++ b/arch/arm/common/gic.c
>> @@ -37,6 +37,20 @@
>>  #include <asm/mach/irq.h>
>>  #include <asm/hardware/gic.h>
>>  
>> +struct gic_chip_data {
>> +	unsigned int irq_offset;
>> +	void __percpu __iomem **dist_base;
>> +	void __percpu __iomem **cpu_base;
>> +#ifdef CONFIG_CPU_PM
>> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
>> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
>> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
>> +	u32 __percpu *saved_ppi_enable;
>> +	u32 __percpu *saved_ppi_conf;
>> +#endif
>> +	unsigned int gic_irqs;
>> +};
> 
> I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.

Unfortunately, you can't. That would be nice though... ;-)

>> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
>>  {
>>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
>> -				(gic_bank_offset * smp_processor_id());
>> +				(gic_bank_offset * cpu_logical_map(cpu));
> 
> Again, I'm deeply suspicious of this code :) Is there not a common memory
> alias for the distributor across all of the CPUs?

Kukjin, could you please comment on the presence of a common memory
region for the distributor? This seem quite odd...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 07/10/11 16:16, Will Deacon wrote:
> Hi Marc,
> 
> On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
>> So to make my suggestion completely clear, here's a patch I'm now
>> carrying in my tree. It's only been test compiled on EXYNOS4, but works
>> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
>> variables, removes these callbacks, removes your private copy of
>> gic_cpu_init, and makes struct gic_chip_data private again.
>>
>> What do you think?
> 
> This looks like the right sort of idea, although I'm deeply suspicious about
> per-cpu base addresses for the GIC distributor. It would be nice if the
> Samsung guys can elaborate on what's going on here. Few comments inline.

Yeah, this seem odd, specially as what's currently in mainline doesn't
try to play tricks with the distributor. It looks like having a per-core
distributor address is required for PPI usage on these SoCs, but the
commit message is uninformative at best:

https://github.com/sfrothwell/linux-next/commit/637c2afa57ec9cd0ddc8879ea0cda4d8835ba71d#arch/arm/mach-exynos4/cpu.c

Again, comments from Samsung people would be much appreciated.

>> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
>>
>> The GIC support code is heavily using the fact that hardware
>> implementations are exposing banked registers. Unfortunately, it
>> looks like at least one GIC implementation (EXYNOS4) offers both
>> the distributor and the CPU interfaces at different addresses,
>> depending on the CPU.
>>
>> This problem is solved by turning the distributor and CPU interface
>> addresses into per-cpu variables. The EXYNOS4 code is updated not
>> to mess with the GIC internals while handling interrupts, and
>> struct gic_chip_data is back to being private.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
>>  arch/arm/include/asm/hardware/gic.h |   17 +------
>>  arch/arm/mach-exynos4/cpu.c         |   14 ------
>>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
>>  4 files changed, 66 insertions(+), 69 deletions(-)
>>
>> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
>> index b574931..c7521dd 100644
>> --- a/arch/arm/common/gic.c
>> +++ b/arch/arm/common/gic.c
>> @@ -37,6 +37,20 @@
>>  #include <asm/mach/irq.h>
>>  #include <asm/hardware/gic.h>
>>  
>> +struct gic_chip_data {
>> +	unsigned int irq_offset;
>> +	void __percpu __iomem **dist_base;
>> +	void __percpu __iomem **cpu_base;
>> +#ifdef CONFIG_CPU_PM
>> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
>> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
>> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
>> +	u32 __percpu *saved_ppi_enable;
>> +	u32 __percpu *saved_ppi_conf;
>> +#endif
>> +	unsigned int gic_irqs;
>> +};
> 
> I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.

Unfortunately, you can't. That would be nice though... ;-)

>> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
>>  {
>>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
>> -				(gic_bank_offset * smp_processor_id());
>> +				(gic_bank_offset * cpu_logical_map(cpu));
> 
> Again, I'm deeply suspicious of this code :) Is there not a common memory
> alias for the distributor across all of the CPUs?

Kukjin, could you please comment on the presence of a common memory
region for the distributor? This seem quite odd...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Changhwan Youn]

Dear Marc and Will,

On Monday, October 10, 2011 10:02 PM, Marc Zyngier wrote:
> 
> On 07/10/11 16:16, Will Deacon wrote:
> > Hi Marc,
> >
> > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> >> So to make my suggestion completely clear, here's a patch I'm now
> >> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> >> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> >> variables, removes these callbacks, removes your private copy of
> >> gic_cpu_init, and makes struct gic_chip_data private again.
> >>
> >> What do you think?
> >
> > This looks like the right sort of idea, although I'm deeply suspicious about
> > per-cpu base addresses for the GIC distributor. It would be nice if the
> > Samsung guys can elaborate on what's going on here. Few comments inline.
> 
> Yeah, this seem odd, specially as what's currently in mainline doesn't
> try to play tricks with the distributor. It looks like having a per-core
> distributor address is required for PPI usage on these SoCs, but the
> commit message is uninformative at best:
> 
> https://github.com/sfrothwell/linux-
> next/commit/637c2afa57ec9cd0ddc8879ea0cda4d8835ba71d#arch/arm/mach-exynos4/cpu.c
> 
> Again, comments from Samsung people would be much appreciated.

Your guess is right.
Exynos4 cannot support register banking, which means every CPU has
its own CPU base and Distributor base address.
Without the workaround I implemented, every CPU accesses the base address
for CPU0 and thus the interrupts for other CPUs cannot be handled properly.
 
> >> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
> >>
> >> The GIC support code is heavily using the fact that hardware
> >> implementations are exposing banked registers. Unfortunately, it
> >> looks like at least one GIC implementation (EXYNOS4) offers both
> >> the distributor and the CPU interfaces at different addresses,
> >> depending on the CPU.
> >>
> >> This problem is solved by turning the distributor and CPU interface
> >> addresses into per-cpu variables. The EXYNOS4 code is updated not
> >> to mess with the GIC internals while handling interrupts, and
> >> struct gic_chip_data is back to being private.
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
> >>  arch/arm/include/asm/hardware/gic.h |   17 +------
> >>  arch/arm/mach-exynos4/cpu.c         |   14 ------
> >>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
> >>  4 files changed, 66 insertions(+), 69 deletions(-)
> >>
> >> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> >> index b574931..c7521dd 100644
> >> --- a/arch/arm/common/gic.c
> >> +++ b/arch/arm/common/gic.c
> >> @@ -37,6 +37,20 @@
> >>  #include <asm/mach/irq.h>
> >>  #include <asm/hardware/gic.h>
> >>
> >> +struct gic_chip_data {
> >> +	unsigned int irq_offset;
> >> +	void __percpu __iomem **dist_base;
> >> +	void __percpu __iomem **cpu_base;
> >> +#ifdef CONFIG_CPU_PM
> >> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
> >> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
> >> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
> >> +	u32 __percpu *saved_ppi_enable;
> >> +	u32 __percpu *saved_ppi_conf;
> >> +#endif
> >> +	unsigned int gic_irqs;
> >> +};
> >
> > I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.
> 
> Unfortunately, you can't. That would be nice though... ;-)
> 
> >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> >>  {
> >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> >> -				(gic_bank_offset * smp_processor_id());
> >> +				(gic_bank_offset * cpu_logical_map(cpu));
> >
> > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > alias for the distributor across all of the CPUs?
> 
> Kukjin, could you please comment on the presence of a common memory
> region for the distributor? This seem quite odd...

Some registers in Distributor are banked for PPI and SGI support (banked interrupts).
The register for pending and enable status of these interrupts are 
banked.

Marc, I think the approach in your patch is much better than mine if it doesn't hurt
the performance of other platforms which use the common gic code.
I'll re-work the exynos4 interrupt code based on your patch though
I'm not sure that it's possible to be merged in merge window.

Kukjin, how do you think about this?

> Thanks,
> 
> 	M.
> --
> Jazz is not dead. It just smells funny...
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Changhwan Youn]

Dear Marc and Will,

On Monday, October 10, 2011 10:02 PM, Marc Zyngier wrote:
> 
> On 07/10/11 16:16, Will Deacon wrote:
> > Hi Marc,
> >
> > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> >> So to make my suggestion completely clear, here's a patch I'm now
> >> carrying in my tree. It's only been test compiled on EXYNOS4, but works
> >> nicely on my 11MP. It turns both dist_base and cpu_base into per-cpu
> >> variables, removes these callbacks, removes your private copy of
> >> gic_cpu_init, and makes struct gic_chip_data private again.
> >>
> >> What do you think?
> >
> > This looks like the right sort of idea, although I'm deeply suspicious about
> > per-cpu base addresses for the GIC distributor. It would be nice if the
> > Samsung guys can elaborate on what's going on here. Few comments inline.
> 
> Yeah, this seem odd, specially as what's currently in mainline doesn't
> try to play tricks with the distributor. It looks like having a per-core
> distributor address is required for PPI usage on these SoCs, but the
> commit message is uninformative at best:
> 
> https://github.com/sfrothwell/linux-
> next/commit/637c2afa57ec9cd0ddc8879ea0cda4d8835ba71d#arch/arm/mach-exynos4/cpu.c
> 
> Again, comments from Samsung people would be much appreciated.

Your guess is right.
Exynos4 cannot support register banking, which means every CPU has
its own CPU base and Distributor base address.
Without the workaround I implemented, every CPU accesses the base address
for CPU0 and thus the interrupts for other CPUs cannot be handled properly.
 
> >> Subject: [PATCH] ARM: gic: allow GIC to support non-banked setups
> >>
> >> The GIC support code is heavily using the fact that hardware
> >> implementations are exposing banked registers. Unfortunately, it
> >> looks like at least one GIC implementation (EXYNOS4) offers both
> >> the distributor and the CPU interfaces at different addresses,
> >> depending on the CPU.
> >>
> >> This problem is solved by turning the distributor and CPU interface
> >> addresses into per-cpu variables. The EXYNOS4 code is updated not
> >> to mess with the GIC internals while handling interrupts, and
> >> struct gic_chip_data is back to being private.
> >>
> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> >> ---
> >>  arch/arm/common/gic.c               |   76 +++++++++++++++++++++++++++--------
> >>  arch/arm/include/asm/hardware/gic.h |   17 +------
> >>  arch/arm/mach-exynos4/cpu.c         |   14 ------
> >>  arch/arm/mach-exynos4/platsmp.c     |   28 ++-----------
> >>  4 files changed, 66 insertions(+), 69 deletions(-)
> >>
> >> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> >> index b574931..c7521dd 100644
> >> --- a/arch/arm/common/gic.c
> >> +++ b/arch/arm/common/gic.c
> >> @@ -37,6 +37,20 @@
> >>  #include <asm/mach/irq.h>
> >>  #include <asm/hardware/gic.h>
> >>
> >> +struct gic_chip_data {
> >> +	unsigned int irq_offset;
> >> +	void __percpu __iomem **dist_base;
> >> +	void __percpu __iomem **cpu_base;
> >> +#ifdef CONFIG_CPU_PM
> >> +	u32 saved_spi_enable[DIV_ROUND_UP(1020, 32)];
> >> +	u32 saved_spi_conf[DIV_ROUND_UP(1020, 16)];
> >> +	u32 saved_spi_target[DIV_ROUND_UP(1020, 4)];
> >> +	u32 __percpu *saved_ppi_enable;
> >> +	u32 __percpu *saved_ppi_conf;
> >> +#endif
> >> +	unsigned int gic_irqs;
> >> +};
> >
> > I think you can use DECLARE_PER_CPU(void __iomem *dist_base) etc. instead.
> 
> Unfortunately, you can't. That would be nice though... ;-)
> 
> >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> >>  {
> >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> >> -				(gic_bank_offset * smp_processor_id());
> >> +				(gic_bank_offset * cpu_logical_map(cpu));
> >
> > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > alias for the distributor across all of the CPUs?
> 
> Kukjin, could you please comment on the presence of a common memory
> region for the distributor? This seem quite odd...

Some registers in Distributor are banked for PPI and SGI support (banked interrupts).
The register for pending and enable status of these interrupts are 
banked.

Marc, I think the approach in your patch is much better than mine if it doesn't hurt
the performance of other platforms which use the common gic code.
I'll re-work the exynos4 interrupt code based on your patch though
I'm not sure that it's possible to be merged in merge window.

Kukjin, how do you think about this?

> Thanks,
> 
> 	M.
> --
> Jazz is not dead. It just smells funny...
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

Hi Changwan,

On 11/10/11 13:22, Changhwan Youn wrote:
>> Kukjin, could you please comment on the presence of a common memory
>> region for the distributor? This seem quite odd...
> 
> Some registers in Distributor are banked for PPI and SGI support (banked interrupts).
> The register for pending and enable status of these interrupts are 
> banked.

Right, that explains it then.

> Marc, I think the approach in your patch is much better than mine if it doesn't hurt
> the performance of other platforms which use the common gic code.

It probably doesn't hurt the general case too much (I expect a bit more
pressure on the d-cache because of the per-cpu stuff, but nothing to be
too worried about).

> I'll re-work the exynos4 interrupt code based on your patch though
> I'm not sure that it's possible to be merged in merge window.

My main concern at the moment is that mainline is broken as far as
EXYNOS4 is concerned (there's a race with the EOI hook), so that should
get fixed first.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

Hi Changwan,

On 11/10/11 13:22, Changhwan Youn wrote:
>> Kukjin, could you please comment on the presence of a common memory
>> region for the distributor? This seem quite odd...
> 
> Some registers in Distributor are banked for PPI and SGI support (banked interrupts).
> The register for pending and enable status of these interrupts are 
> banked.

Right, that explains it then.

> Marc, I think the approach in your patch is much better than mine if it doesn't hurt
> the performance of other platforms which use the common gic code.

It probably doesn't hurt the general case too much (I expect a bit more
pressure on the d-cache because of the per-cpu stuff, but nothing to be
too worried about).

> I'll re-work the exynos4 interrupt code based on your patch though
> I'm not sure that it's possible to be merged in merge window.

My main concern at the moment is that mainline is broken as far as
EXYNOS4 is concerned (there's a race with the EOI hook), so that should
get fixed first.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> Hi Changwan,
> 
> On 11/10/11 13:22, Changhwan Youn wrote:
> >> Kukjin, could you please comment on the presence of a common memory
> >> region for the distributor? This seem quite odd...
> >
> > Some registers in Distributor are banked for PPI and SGI support (banked
> interrupts).
> > The register for pending and enable status of these interrupts are
> > banked.
> 
> Right, that explains it then.
> 
> > Marc, I think the approach in your patch is much better than mine if it
doesn't hurt
> > the performance of other platforms which use the common gic code.
> 
> It probably doesn't hurt the general case too much (I expect a bit more
> pressure on the d-cache because of the per-cpu stuff, but nothing to be
> too worried about).
> 
> > I'll re-work the exynos4 interrupt code based on your patch though
> > I'm not sure that it's possible to be merged in merge window.
> 
> My main concern at the moment is that mainline is broken as far as
> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
> get fixed first.
> 
Hi Marc,

OK. I agree with Will and your opinions and I think Changhwan can fix it as
per your suggestion, but he needs fixed/updated regarding gic codes to avoid
re-work and conflicts with others. So it would be better to us if he could
fix it after merging your patches even probably at the end of upcoming merge
window. I hope he can do it before v3.2-rc1.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> Hi Changwan,
> 
> On 11/10/11 13:22, Changhwan Youn wrote:
> >> Kukjin, could you please comment on the presence of a common memory
> >> region for the distributor? This seem quite odd...
> >
> > Some registers in Distributor are banked for PPI and SGI support (banked
> interrupts).
> > The register for pending and enable status of these interrupts are
> > banked.
> 
> Right, that explains it then.
> 
> > Marc, I think the approach in your patch is much better than mine if it
doesn't hurt
> > the performance of other platforms which use the common gic code.
> 
> It probably doesn't hurt the general case too much (I expect a bit more
> pressure on the d-cache because of the per-cpu stuff, but nothing to be
> too worried about).
> 
> > I'll re-work the exynos4 interrupt code based on your patch though
> > I'm not sure that it's possible to be merged in merge window.
> 
> My main concern at the moment is that mainline is broken as far as
> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
> get fixed first.
> 
Hi Marc,

OK. I agree with Will and your opinions and I think Changhwan can fix it as
per your suggestion, but he needs fixed/updated regarding gic codes to avoid
re-work and conflicts with others. So it would be better to us if he could
fix it after merging your patches even probably at the end of upcoming merge
window. I hope he can do it before v3.2-rc1.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Russell King - ARM Linux]

On Mon, Oct 10, 2011 at 02:02:09PM +0100, Marc Zyngier wrote:
> On 07/10/11 16:16, Will Deacon wrote:
> > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> >>  {
> >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> >> -				(gic_bank_offset * smp_processor_id());
> >> +				(gic_bank_offset * cpu_logical_map(cpu));
> > 
> > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > alias for the distributor across all of the CPUs?
> 
> Kukjin, could you please comment on the presence of a common memory
> region for the distributor? This seem quite odd...

It's not odd when you consider that there's per-CPU registers within the
GIC distributor as well (for the first 32 GIC IRQs) as the per-CPU GIC
CPU interfaces.

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Russell King - ARM Linux]

On Mon, Oct 10, 2011 at 02:02:09PM +0100, Marc Zyngier wrote:
> On 07/10/11 16:16, Will Deacon wrote:
> > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> >>  {
> >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> >> -				(gic_bank_offset * smp_processor_id());
> >> +				(gic_bank_offset * cpu_logical_map(cpu));
> > 
> > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > alias for the distributor across all of the CPUs?
> 
> Kukjin, could you please comment on the presence of a common memory
> region for the distributor? This seem quite odd...

It's not odd when you consider that there's per-CPU registers within the
GIC distributor as well (for the first 32 GIC IRQs) as the per-CPU GIC
CPU interfaces.

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Will Deacon]

On Thu, Oct 13, 2011 at 12:09:28PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 10, 2011 at 02:02:09PM +0100, Marc Zyngier wrote:
> > On 07/10/11 16:16, Will Deacon wrote:
> > > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> > >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> > >>  {
> > >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> > >> -				(gic_bank_offset * smp_processor_id());
> > >> +				(gic_bank_offset * cpu_logical_map(cpu));
> > > 
> > > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > > alias for the distributor across all of the CPUs?
> > 
> > Kukjin, could you please comment on the presence of a common memory
> > region for the distributor? This seem quite odd...
> 
> It's not odd when you consider that there's per-CPU registers within the
> GIC distributor as well (for the first 32 GIC IRQs) as the per-CPU GIC
> CPU interfaces.

Agreed, but it is odd for those registers not to be banked in hardware. Marc
and I spoke to some of the hardware chaps at ARM and they don't see this as
a common setup in the future, so rather than litter the GIC driver with
__percpu variables, we might be better off adding some extra platform callbacks
to the set that we already have.

Will

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Will Deacon]

On Thu, Oct 13, 2011 at 12:09:28PM +0100, Russell King - ARM Linux wrote:
> On Mon, Oct 10, 2011 at 02:02:09PM +0100, Marc Zyngier wrote:
> > On 07/10/11 16:16, Will Deacon wrote:
> > > On Fri, Oct 07, 2011 at 10:44:59AM +0100, Marc Zyngier wrote:
> > >> +static void __cpuinit exynos4_secondary_init(unsigned int cpu)
> > >>  {
> > >>  	void __iomem *dist_base = S5P_VA_GIC_DIST +
> > >> -				(gic_bank_offset * smp_processor_id());
> > >> +				(gic_bank_offset * cpu_logical_map(cpu));
> > > 
> > > Again, I'm deeply suspicious of this code :) Is there not a common memory
> > > alias for the distributor across all of the CPUs?
> > 
> > Kukjin, could you please comment on the presence of a common memory
> > region for the distributor? This seem quite odd...
> 
> It's not odd when you consider that there's per-CPU registers within the
> GIC distributor as well (for the first 32 GIC IRQs) as the per-CPU GIC
> CPU interfaces.

Agreed, but it is odd for those registers not to be banked in hardware. Marc
and I spoke to some of the hardware chaps at ARM and they don't see this as
a common setup in the future, so rather than litter the GIC driver with
__percpu variables, we might be better off adding some extra platform callbacks
to the set that we already have.

Will

Re: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 12/10/11 06:16, Kukjin Kim wrote:
> Marc Zyngier wrote:
>>
>> Hi Changwan,
>>
>> On 11/10/11 13:22, Changhwan Youn wrote:
>>>> Kukjin, could you please comment on the presence of a common memory
>>>> region for the distributor? This seem quite odd...
>>>
>>> Some registers in Distributor are banked for PPI and SGI support (banked
>> interrupts).
>>> The register for pending and enable status of these interrupts are
>>> banked.
>>
>> Right, that explains it then.
>>
>>> Marc, I think the approach in your patch is much better than mine if it
> doesn't hurt
>>> the performance of other platforms which use the common gic code.
>>
>> It probably doesn't hurt the general case too much (I expect a bit more
>> pressure on the d-cache because of the per-cpu stuff, but nothing to be
>> too worried about).
>>
>>> I'll re-work the exynos4 interrupt code based on your patch though
>>> I'm not sure that it's possible to be merged in merge window.
>>
>> My main concern at the moment is that mainline is broken as far as
>> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
>> get fixed first.
>>
> Hi Marc,
> 
> OK. I agree with Will and your opinions and I think Changhwan can fix it as
> per your suggestion, but he needs fixed/updated regarding gic codes to avoid
> re-work and conflicts with others. So it would be better to us if he could
> fix it after merging your patches even probably at the end of upcoming merge
> window. I hope he can do it before v3.2-rc1.

Right. So this damned thing has made it to mainline in its full glory.
Furthermore, the MCT code is also broken, as it uses the old PPI API
(doesn't even compile).

Can we please fix this as soon as possible? I posted patches for both a
while ago, with almost no reaction...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Marc Zyngier]

On 12/10/11 06:16, Kukjin Kim wrote:
> Marc Zyngier wrote:
>>
>> Hi Changwan,
>>
>> On 11/10/11 13:22, Changhwan Youn wrote:
>>>> Kukjin, could you please comment on the presence of a common memory
>>>> region for the distributor? This seem quite odd...
>>>
>>> Some registers in Distributor are banked for PPI and SGI support (banked
>> interrupts).
>>> The register for pending and enable status of these interrupts are
>>> banked.
>>
>> Right, that explains it then.
>>
>>> Marc, I think the approach in your patch is much better than mine if it
> doesn't hurt
>>> the performance of other platforms which use the common gic code.
>>
>> It probably doesn't hurt the general case too much (I expect a bit more
>> pressure on the d-cache because of the per-cpu stuff, but nothing to be
>> too worried about).
>>
>>> I'll re-work the exynos4 interrupt code based on your patch though
>>> I'm not sure that it's possible to be merged in merge window.
>>
>> My main concern at the moment is that mainline is broken as far as
>> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
>> get fixed first.
>>
> Hi Marc,
> 
> OK. I agree with Will and your opinions and I think Changhwan can fix it as
> per your suggestion, but he needs fixed/updated regarding gic codes to avoid
> re-work and conflicts with others. So it would be better to us if he could
> fix it after merging your patches even probably at the end of upcoming merge
> window. I hope he can do it before v3.2-rc1.

Right. So this damned thing has made it to mainline in its full glory.
Furthermore, the MCT code is also broken, as it uses the old PPI API
(doesn't even compile).

Can we please fix this as soon as possible? I posted patches for both a
while ago, with almost no reaction...

	M.
-- 
Jazz is not dead. It just smells funny...

RE: [PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> On 12/10/11 06:16, Kukjin Kim wrote:
> > Marc Zyngier wrote:
> >>
> >> Hi Changwan,
> >>
> >> On 11/10/11 13:22, Changhwan Youn wrote:
> >>>> Kukjin, could you please comment on the presence of a common memory
> >>>> region for the distributor? This seem quite odd...
> >>>
> >>> Some registers in Distributor are banked for PPI and SGI support
(banked
> >> interrupts).
> >>> The register for pending and enable status of these interrupts are
> >>> banked.
> >>
> >> Right, that explains it then.
> >>
> >>> Marc, I think the approach in your patch is much better than mine if
it
> > doesn't hurt
> >>> the performance of other platforms which use the common gic code.
> >>
> >> It probably doesn't hurt the general case too much (I expect a bit more
> >> pressure on the d-cache because of the per-cpu stuff, but nothing to be
> >> too worried about).
> >>
> >>> I'll re-work the exynos4 interrupt code based on your patch though
> >>> I'm not sure that it's possible to be merged in merge window.
> >>
> >> My main concern at the moment is that mainline is broken as far as
> >> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
> >> get fixed first.
> >>
> > Hi Marc,
> >
> > OK. I agree with Will and your opinions and I think Changhwan can fix it
as
> > per your suggestion, but he needs fixed/updated regarding gic codes to
avoid
> > re-work and conflicts with others. So it would be better to us if he
could
> > fix it after merging your patches even probably at the end of upcoming
merge
> > window. I hope he can do it before v3.2-rc1.
> 
> Right. So this damned thing has made it to mainline in its full glory.
> Furthermore, the MCT code is also broken, as it uses the old PPI API
> (doesn't even compile).
> 
> Can we please fix this as soon as possible? I posted patches for both a
> while ago, with almost no reaction...

Yeah, should be fixed. Let me check again and if any updates, let you know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 5/7] ARM: EXYNOS4: Add support external GIC

[Kukjin Kim]

Marc Zyngier wrote:
> 
> On 12/10/11 06:16, Kukjin Kim wrote:
> > Marc Zyngier wrote:
> >>
> >> Hi Changwan,
> >>
> >> On 11/10/11 13:22, Changhwan Youn wrote:
> >>>> Kukjin, could you please comment on the presence of a common memory
> >>>> region for the distributor? This seem quite odd...
> >>>
> >>> Some registers in Distributor are banked for PPI and SGI support
(banked
> >> interrupts).
> >>> The register for pending and enable status of these interrupts are
> >>> banked.
> >>
> >> Right, that explains it then.
> >>
> >>> Marc, I think the approach in your patch is much better than mine if
it
> > doesn't hurt
> >>> the performance of other platforms which use the common gic code.
> >>
> >> It probably doesn't hurt the general case too much (I expect a bit more
> >> pressure on the d-cache because of the per-cpu stuff, but nothing to be
> >> too worried about).
> >>
> >>> I'll re-work the exynos4 interrupt code based on your patch though
> >>> I'm not sure that it's possible to be merged in merge window.
> >>
> >> My main concern at the moment is that mainline is broken as far as
> >> EXYNOS4 is concerned (there's a race with the EOI hook), so that should
> >> get fixed first.
> >>
> > Hi Marc,
> >
> > OK. I agree with Will and your opinions and I think Changhwan can fix it
as
> > per your suggestion, but he needs fixed/updated regarding gic codes to
avoid
> > re-work and conflicts with others. So it would be better to us if he
could
> > fix it after merging your patches even probably at the end of upcoming
merge
> > window. I hope he can do it before v3.2-rc1.
> 
> Right. So this damned thing has made it to mainline in its full glory.
> Furthermore, the MCT code is also broken, as it uses the old PPI API
> (doesn't even compile).
> 
> Can we please fix this as soon as possible? I posted patches for both a
> while ago, with almost no reaction...

Yeah, should be fixed. Let me check again and if any updates, let you know.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.