[PATCH 0/7] exynos multiplatform support, part 2

[PATCH 0/7] exynos multiplatform support, part 2

[Arnd Bergmann]

These are the patches required for exynos multiplatform support that
depend on other patches that are already merged in the arm-soc next/drivers
branch. The series applies cleanly on top of the next/drivers branch
at the moment, and in combination with various patches that were
accepted by subsystem maintainers and part 1 of the series (which
applies on top of 3.9-rc5) brings full support for ARCH_MULTIPLATFORM
to device tree based exynos systems.

	Arnd

Arnd Bergmann (7):
  clk: exynos: prepare for multiplatform
  clocksource: exynos_mct: remove platform header dependency
  ARM: exynos: add missing properties for combiner IRQs
  irqchip: exynos: pass max combiner number to combiner_init
  irqchip: exynos: allocate combiner_data dynamically
  irqchip: exynos: localize irq lookup for ATAGS
  irqchip: exynos: pass irq_base from platform

 arch/arm/boot/dts/exynos4210.dtsi    |   1 +
 arch/arm/boot/dts/exynos4212.dtsi    |   9 +++
 arch/arm/boot/dts/exynos4412.dtsi    |   9 +++
 arch/arm/mach-exynos/common.c        |  20 +++++-
 arch/arm/mach-exynos/common.h        |   7 ++-
 drivers/clk/samsung/clk-exynos4.c    |  93 +++++++++++++---------------
 drivers/clk/samsung/clk-exynos5250.c |   1 -
 drivers/clk/samsung/clk-exynos5440.c |   1 -
 drivers/clk/samsung/clk.h            |   2 -
 drivers/clocksource/exynos_mct.c     |  21 ++-----
 drivers/irqchip/exynos-combiner.c    | 116 ++++++++++++++++++-----------------
 11 files changed, 148 insertions(+), 132 deletions(-)

-- 
1.8.1.2

[PATCH 0/7] exynos multiplatform support, part 2

[Arnd Bergmann]

These are the patches required for exynos multiplatform support that
depend on other patches that are already merged in the arm-soc next/drivers
branch. The series applies cleanly on top of the next/drivers branch
at the moment, and in combination with various patches that were
accepted by subsystem maintainers and part 1 of the series (which
applies on top of 3.9-rc5) brings full support for ARCH_MULTIPLATFORM
to device tree based exynos systems.

	Arnd

Arnd Bergmann (7):
  clk: exynos: prepare for multiplatform
  clocksource: exynos_mct: remove platform header dependency
  ARM: exynos: add missing properties for combiner IRQs
  irqchip: exynos: pass max combiner number to combiner_init
  irqchip: exynos: allocate combiner_data dynamically
  irqchip: exynos: localize irq lookup for ATAGS
  irqchip: exynos: pass irq_base from platform

 arch/arm/boot/dts/exynos4210.dtsi    |   1 +
 arch/arm/boot/dts/exynos4212.dtsi    |   9 +++
 arch/arm/boot/dts/exynos4412.dtsi    |   9 +++
 arch/arm/mach-exynos/common.c        |  20 +++++-
 arch/arm/mach-exynos/common.h        |   7 ++-
 drivers/clk/samsung/clk-exynos4.c    |  93 +++++++++++++---------------
 drivers/clk/samsung/clk-exynos5250.c |   1 -
 drivers/clk/samsung/clk-exynos5440.c |   1 -
 drivers/clk/samsung/clk.h            |   2 -
 drivers/clocksource/exynos_mct.c     |  21 ++-----
 drivers/irqchip/exynos-combiner.c    | 116 ++++++++++++++++++-----------------
 11 files changed, 148 insertions(+), 132 deletions(-)

-- 
1.8.1.2

[PATCH 1/7] clk: exynos: prepare for multiplatform

[Arnd Bergmann]

The new common clock drivers for exynos are using compile
time constants and soc_is_exynos* macros to provide backwards
compatibility for pre-DT systems, which is not possible with
multiplatform kernels. This moves all the necessary
information back into platform code and removes the mach/*
header inclusions.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Mike Turquette <mturquette@linaro.org>
---
 arch/arm/mach-exynos/common.c        |  2 +-
 arch/arm/mach-exynos/common.h        |  2 +-
 drivers/clk/samsung/clk-exynos4.c    | 93 ++++++++++++++++--------------------
 drivers/clk/samsung/clk-exynos5250.c |  1 -
 drivers/clk/samsung/clk-exynos5440.c |  1 -
 drivers/clk/samsung/clk.h            |  2 -
 6 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 939bda7..42f399a 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -410,7 +410,7 @@ void __init exynos_init_time(void)
 	} else {
 		/* todo: remove after migrating legacy E4 platforms to dt */
 #ifdef CONFIG_ARCH_EXYNOS4
-		exynos4_clk_init(NULL);
+		exynos4_clk_init(NULL, !soc_is_exynos4210(), S5P_VA_CMU, readl(S5P_VA_CHIPID + 8) & 1);
 		exynos4_clk_register_fixed_ext(xxti_f, xusbxti_f);
 #endif
 		mct_init();
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index cb89ab8..4ed8eef 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -27,7 +27,7 @@ void exynos5_restart(char mode, const char *cmd);
 void exynos_init_late(void);
 
 /* ToDo: remove these after migrating legacy exynos4 platforms to dt */
-void exynos4_clk_init(struct device_node *np);
+void exynos4_clk_init(struct device_node *np, int is_exynos4210, void __iomem *reg_base, unsigned long xom);
 void exynos4_clk_register_fixed_ext(unsigned long, unsigned long);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 7104669..d0940e6 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -16,7 +16,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
@@ -910,16 +909,6 @@ struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
 			CLK_IGNORE_UNUSED, 0),
 };
 
-#ifdef CONFIG_OF
-static struct of_device_id exynos4_clk_ids[] __initdata = {
-	{ .compatible = "samsung,exynos4210-clock",
-			.data = (void *)EXYNOS4210, },
-	{ .compatible = "samsung,exynos4412-clock",
-			.data = (void *)EXYNOS4X12, },
-	{ },
-};
-#endif
-
 /*
  * The parent of the fin_pll clock is selected by the XOM[0] bit. This bit
  * resides in chipid register space, outside of the clock controller memory
@@ -927,33 +916,40 @@ static struct of_device_id exynos4_clk_ids[] __initdata = {
  * controller is first remapped and the value of XOM[0] bit is read to
  * determine the parent clock.
  */
-static void __init exynos4_clk_register_finpll(void)
+static unsigned long exynos4_get_xom(void)
 {
-	struct samsung_fixed_rate_clock fclk;
+	unsigned long xom = 0;
+	void __iomem *chipid_base;
 	struct device_node *np;
-	struct clk *clk;
-	void __iomem *chipid_base = S5P_VA_CHIPID;
-	unsigned long xom, finpll_f = 24000000;
-	char *parent_name;
 
 	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
-	if (np)
+	if (np) {
 		chipid_base = of_iomap(np, 0);
 
-	if (chipid_base) {
-		xom = readl(chipid_base + 8);
-		parent_name = xom & 1 ? "xusbxti" : "xxti";
-		clk = clk_get(NULL, parent_name);
-		if (IS_ERR(clk)) {
-			pr_err("%s: failed to lookup parent clock %s, assuming "
-				"fin_pll clock frequency is 24MHz\n", __func__,
-				parent_name);
-		} else {
-			finpll_f = clk_get_rate(clk);
-		}
+		if (chipid_base)
+			xom = readl(chipid_base + 8);
+
+		iounmap(chipid_base);
+	}
+
+	return xom;
+}
+
+static void __init exynos4_clk_register_finpll(unsigned long xom)
+{
+	struct samsung_fixed_rate_clock fclk;
+	struct clk *clk;
+	unsigned long finpll_f = 24000000;
+	char *parent_name;
+
+	parent_name = xom & 1 ? "xusbxti" : "xxti";
+	clk = clk_get(NULL, parent_name);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to lookup parent clock %s, assuming "
+			"fin_pll clock frequency is 24MHz\n", __func__,
+			parent_name);
 	} else {
-		pr_err("%s: failed to map chipid registers, assuming "
-			"fin_pll clock frequency is 24MHz\n", __func__);
+		finpll_f = clk_get_rate(clk);
 	}
 
 	fclk.id = fin_pll;
@@ -963,8 +959,6 @@ static void __init exynos4_clk_register_finpll(void)
 	fclk.fixed_rate = finpll_f;
 	samsung_clk_register_fixed_rate(&fclk, 1);
 
-	if (np)
-		iounmap(chipid_base);
 }
 
 /*
@@ -988,28 +982,14 @@ static __initdata struct of_device_id ext_clk_match[] = {
 };
 
 /* register exynos4 clocks */
-void __init exynos4_clk_init(struct device_node *np)
+void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_soc, void __iomem *reg_base, unsigned long xom)
 {
-	void __iomem *reg_base;
 	struct clk *apll, *mpll, *epll, *vpll;
-	u32 exynos4_soc;
 
 	if (np) {
-		const struct of_device_id *match;
-		match = of_match_node(exynos4_clk_ids, np);
-		exynos4_soc = (u32)match->data;
-
 		reg_base = of_iomap(np, 0);
 		if (!reg_base)
 			panic("%s: failed to map registers\n", __func__);
-	} else {
-		reg_base = S5P_VA_CMU;
-		if (soc_is_exynos4210())
-			exynos4_soc = EXYNOS4210;
-		else if (soc_is_exynos4212() || soc_is_exynos4412())
-			exynos4_soc = EXYNOS4X12;
-		else
-			panic("%s: unable to determine soc\n", __func__);
 	}
 
 	if (exynos4_soc == EXYNOS4210)
@@ -1026,7 +1006,7 @@ void __init exynos4_clk_init(struct device_node *np)
 			ARRAY_SIZE(exynos4_fixed_rate_ext_clks),
 			ext_clk_match);
 
-	exynos4_clk_register_finpll();
+	exynos4_clk_register_finpll(xom);
 
 	if (exynos4_soc == EXYNOS4210) {
 		apll = samsung_clk_register_pll45xx("fout_apll", "fin_pll",
@@ -1087,5 +1067,16 @@ void __init exynos4_clk_init(struct device_node *np)
 		_get_rate("sclk_epll"), _get_rate("sclk_vpll"),
 		_get_rate("arm_clk"));
 }
-CLK_OF_DECLARE(exynos4210_clk, "samsung,exynos4210-clock", exynos4_clk_init);
-CLK_OF_DECLARE(exynos4412_clk, "samsung,exynos4412-clock", exynos4_clk_init);
+
+
+static void __init exynos4210_clk_init(struct device_node *np)
+{
+	exynos4_clk_init(np, EXYNOS4210, NULL, exynos4_get_xom());
+}
+CLK_OF_DECLARE(exynos4210_clk, "samsung,exynos4210-clock", exynos4210_clk_init);
+
+static void __init exynos4412_clk_init(struct device_node *np)
+{
+	exynos4_clk_init(np, EXYNOS4X12, NULL, exynos4_get_xom());
+}
+CLK_OF_DECLARE(exynos4412_clk, "samsung,exynos4412-clock", exynos4412_clk_init);
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 7290faa..61068cd 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -16,7 +16,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
diff --git a/drivers/clk/samsung/clk-exynos5440.c b/drivers/clk/samsung/clk-exynos5440.c
index a0a094c..7d54341 100644
--- a/drivers/clk/samsung/clk-exynos5440.c
+++ b/drivers/clk/samsung/clk-exynos5440.c
@@ -15,7 +15,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 10b2111..e4ad6ea 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -20,8 +20,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <mach/map.h>
-
 /**
  * struct samsung_clock_alias: information about mux clock
  * @id: platform specific id of the clock.
-- 
1.8.1.2

[PATCH 1/7] clk: exynos: prepare for multiplatform

[Arnd Bergmann]

The new common clock drivers for exynos are using compile
time constants and soc_is_exynos* macros to provide backwards
compatibility for pre-DT systems, which is not possible with
multiplatform kernels. This moves all the necessary
information back into platform code and removes the mach/*
header inclusions.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Mike Turquette <mturquette@linaro.org>
---
 arch/arm/mach-exynos/common.c        |  2 +-
 arch/arm/mach-exynos/common.h        |  2 +-
 drivers/clk/samsung/clk-exynos4.c    | 93 ++++++++++++++++--------------------
 drivers/clk/samsung/clk-exynos5250.c |  1 -
 drivers/clk/samsung/clk-exynos5440.c |  1 -
 drivers/clk/samsung/clk.h            |  2 -
 6 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 939bda7..42f399a 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -410,7 +410,7 @@ void __init exynos_init_time(void)
 	} else {
 		/* todo: remove after migrating legacy E4 platforms to dt */
 #ifdef CONFIG_ARCH_EXYNOS4
-		exynos4_clk_init(NULL);
+		exynos4_clk_init(NULL, !soc_is_exynos4210(), S5P_VA_CMU, readl(S5P_VA_CHIPID + 8) & 1);
 		exynos4_clk_register_fixed_ext(xxti_f, xusbxti_f);
 #endif
 		mct_init();
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index cb89ab8..4ed8eef 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -27,7 +27,7 @@ void exynos5_restart(char mode, const char *cmd);
 void exynos_init_late(void);
 
 /* ToDo: remove these after migrating legacy exynos4 platforms to dt */
-void exynos4_clk_init(struct device_node *np);
+void exynos4_clk_init(struct device_node *np, int is_exynos4210, void __iomem *reg_base, unsigned long xom);
 void exynos4_clk_register_fixed_ext(unsigned long, unsigned long);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 7104669..d0940e6 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -16,7 +16,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
@@ -910,16 +909,6 @@ struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
 			CLK_IGNORE_UNUSED, 0),
 };
 
-#ifdef CONFIG_OF
-static struct of_device_id exynos4_clk_ids[] __initdata = {
-	{ .compatible = "samsung,exynos4210-clock",
-			.data = (void *)EXYNOS4210, },
-	{ .compatible = "samsung,exynos4412-clock",
-			.data = (void *)EXYNOS4X12, },
-	{ },
-};
-#endif
-
 /*
  * The parent of the fin_pll clock is selected by the XOM[0] bit. This bit
  * resides in chipid register space, outside of the clock controller memory
@@ -927,33 +916,40 @@ static struct of_device_id exynos4_clk_ids[] __initdata = {
  * controller is first remapped and the value of XOM[0] bit is read to
  * determine the parent clock.
  */
-static void __init exynos4_clk_register_finpll(void)
+static unsigned long exynos4_get_xom(void)
 {
-	struct samsung_fixed_rate_clock fclk;
+	unsigned long xom = 0;
+	void __iomem *chipid_base;
 	struct device_node *np;
-	struct clk *clk;
-	void __iomem *chipid_base = S5P_VA_CHIPID;
-	unsigned long xom, finpll_f = 24000000;
-	char *parent_name;
 
 	np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-chipid");
-	if (np)
+	if (np) {
 		chipid_base = of_iomap(np, 0);
 
-	if (chipid_base) {
-		xom = readl(chipid_base + 8);
-		parent_name = xom & 1 ? "xusbxti" : "xxti";
-		clk = clk_get(NULL, parent_name);
-		if (IS_ERR(clk)) {
-			pr_err("%s: failed to lookup parent clock %s, assuming "
-				"fin_pll clock frequency is 24MHz\n", __func__,
-				parent_name);
-		} else {
-			finpll_f = clk_get_rate(clk);
-		}
+		if (chipid_base)
+			xom = readl(chipid_base + 8);
+
+		iounmap(chipid_base);
+	}
+
+	return xom;
+}
+
+static void __init exynos4_clk_register_finpll(unsigned long xom)
+{
+	struct samsung_fixed_rate_clock fclk;
+	struct clk *clk;
+	unsigned long finpll_f = 24000000;
+	char *parent_name;
+
+	parent_name = xom & 1 ? "xusbxti" : "xxti";
+	clk = clk_get(NULL, parent_name);
+	if (IS_ERR(clk)) {
+		pr_err("%s: failed to lookup parent clock %s, assuming "
+			"fin_pll clock frequency is 24MHz\n", __func__,
+			parent_name);
 	} else {
-		pr_err("%s: failed to map chipid registers, assuming "
-			"fin_pll clock frequency is 24MHz\n", __func__);
+		finpll_f = clk_get_rate(clk);
 	}
 
 	fclk.id = fin_pll;
@@ -963,8 +959,6 @@ static void __init exynos4_clk_register_finpll(void)
 	fclk.fixed_rate = finpll_f;
 	samsung_clk_register_fixed_rate(&fclk, 1);
 
-	if (np)
-		iounmap(chipid_base);
 }
 
 /*
@@ -988,28 +982,14 @@ static __initdata struct of_device_id ext_clk_match[] = {
 };
 
 /* register exynos4 clocks */
-void __init exynos4_clk_init(struct device_node *np)
+void __init exynos4_clk_init(struct device_node *np, enum exynos4_soc exynos4_soc, void __iomem *reg_base, unsigned long xom)
 {
-	void __iomem *reg_base;
 	struct clk *apll, *mpll, *epll, *vpll;
-	u32 exynos4_soc;
 
 	if (np) {
-		const struct of_device_id *match;
-		match = of_match_node(exynos4_clk_ids, np);
-		exynos4_soc = (u32)match->data;
-
 		reg_base = of_iomap(np, 0);
 		if (!reg_base)
 			panic("%s: failed to map registers\n", __func__);
-	} else {
-		reg_base = S5P_VA_CMU;
-		if (soc_is_exynos4210())
-			exynos4_soc = EXYNOS4210;
-		else if (soc_is_exynos4212() || soc_is_exynos4412())
-			exynos4_soc = EXYNOS4X12;
-		else
-			panic("%s: unable to determine soc\n", __func__);
 	}
 
 	if (exynos4_soc == EXYNOS4210)
@@ -1026,7 +1006,7 @@ void __init exynos4_clk_init(struct device_node *np)
 			ARRAY_SIZE(exynos4_fixed_rate_ext_clks),
 			ext_clk_match);
 
-	exynos4_clk_register_finpll();
+	exynos4_clk_register_finpll(xom);
 
 	if (exynos4_soc == EXYNOS4210) {
 		apll = samsung_clk_register_pll45xx("fout_apll", "fin_pll",
@@ -1087,5 +1067,16 @@ void __init exynos4_clk_init(struct device_node *np)
 		_get_rate("sclk_epll"), _get_rate("sclk_vpll"),
 		_get_rate("arm_clk"));
 }
-CLK_OF_DECLARE(exynos4210_clk, "samsung,exynos4210-clock", exynos4_clk_init);
-CLK_OF_DECLARE(exynos4412_clk, "samsung,exynos4412-clock", exynos4_clk_init);
+
+
+static void __init exynos4210_clk_init(struct device_node *np)
+{
+	exynos4_clk_init(np, EXYNOS4210, NULL, exynos4_get_xom());
+}
+CLK_OF_DECLARE(exynos4210_clk, "samsung,exynos4210-clock", exynos4210_clk_init);
+
+static void __init exynos4412_clk_init(struct device_node *np)
+{
+	exynos4_clk_init(np, EXYNOS4X12, NULL, exynos4_get_xom());
+}
+CLK_OF_DECLARE(exynos4412_clk, "samsung,exynos4412-clock", exynos4412_clk_init);
diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 7290faa..61068cd 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -16,7 +16,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
diff --git a/drivers/clk/samsung/clk-exynos5440.c b/drivers/clk/samsung/clk-exynos5440.c
index a0a094c..7d54341 100644
--- a/drivers/clk/samsung/clk-exynos5440.c
+++ b/drivers/clk/samsung/clk-exynos5440.c
@@ -15,7 +15,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <plat/cpu.h>
 #include "clk.h"
 #include "clk-pll.h"
 
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index 10b2111..e4ad6ea 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -20,8 +20,6 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-#include <mach/map.h>
-
 /**
  * struct samsung_clock_alias: information about mux clock
  * @id: platform specific id of the clock.
-- 
1.8.1.2

Re: [PATCH 1/7] clk: exynos: prepare for multiplatform

[Thomas Abraham]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The new common clock drivers for exynos are using compile
> time constants and soc_is_exynos* macros to provide backwards
> compatibility for pre-DT systems, which is not possible with
> multiplatform kernels. This moves all the necessary
> information back into platform code and removes the mach/*
> header inclusions.


Since kgene has submitted a patch to remove pre-dt Exynos support
which looks like it will be accepted, we could just remove the use of
soc_is_xyz() instead of continuing to support non-dt Exynos.

Thanks,
Thomas.

[PATCH 1/7] clk: exynos: prepare for multiplatform

[Thomas Abraham]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The new common clock drivers for exynos are using compile
> time constants and soc_is_exynos* macros to provide backwards
> compatibility for pre-DT systems, which is not possible with
> multiplatform kernels. This moves all the necessary
> information back into platform code and removes the mach/*
> header inclusions.


Since kgene has submitted a patch to remove pre-dt Exynos support
which looks like it will be accepted, we could just remove the use of
soc_is_xyz() instead of continuing to support non-dt Exynos.

Thanks,
Thomas.

Re: [PATCH 1/7] clk: exynos: prepare for multiplatform

[Arnd Bergmann]

On Monday 15 April 2013, Thomas Abraham wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> > The new common clock drivers for exynos are using compile
> > time constants and soc_is_exynos* macros to provide backwards
> > compatibility for pre-DT systems, which is not possible with
> > multiplatform kernels. This moves all the necessary
> > information back into platform code and removes the mach/*
> > header inclusions.
> 
> 
> Since kgene has submitted a patch to remove pre-dt Exynos support
> which looks like it will be accepted, we could just remove the use of
> soc_is_xyz() instead of continuing to support non-dt Exynos.

I think based on the most recent discussion, we are not actually doing
that for 3.10, since there are still people that require ATAGS support.

	Arnd

[PATCH 1/7] clk: exynos: prepare for multiplatform

[Arnd Bergmann]

On Monday 15 April 2013, Thomas Abraham wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> > The new common clock drivers for exynos are using compile
> > time constants and soc_is_exynos* macros to provide backwards
> > compatibility for pre-DT systems, which is not possible with
> > multiplatform kernels. This moves all the necessary
> > information back into platform code and removes the mach/*
> > header inclusions.
> 
> 
> Since kgene has submitted a patch to remove pre-dt Exynos support
> which looks like it will be accepted, we could just remove the use of
> soc_is_xyz() instead of continuing to support non-dt Exynos.

I think based on the most recent discussion, we are not actually doing
that for 3.10, since there are still people that require ATAGS support.

	Arnd

Re: [PATCH 1/7] clk: exynos: prepare for multiplatform

[Sylwester Nawrocki]

On 04/15/2013 10:35 AM, Arnd Bergmann wrote:
> On Monday 15 April 2013, Thomas Abraham wrote:
>> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
>>> The new common clock drivers for exynos are using compile
>>> time constants and soc_is_exynos* macros to provide backwards
>>> compatibility for pre-DT systems, which is not possible with
>>> multiplatform kernels. This moves all the necessary
>>> information back into platform code and removes the mach/*
>>> header inclusions.
>>
>>
>> Since kgene has submitted a patch to remove pre-dt Exynos support
>> which looks like it will be accepted, we could just remove the use of
>> soc_is_xyz() instead of continuing to support non-dt Exynos.
> 
> I think based on the most recent discussion, we are not actually doing
> that for 3.10, since there are still people that require ATAGS support.

It seems patches from Tomasz [1] will likely make it to 3.10, thus
it is fine to apply Kukjin's patch as far as we are concerned.

Besides, it seems some drivers are already broken with ATAGS booting
(e.g. Exynos DRM, SDHCI) due to introduction of the common clock
framework driver. It doesn't seem worthwhile to spend time fixing
this, I would rather focus on improving the DT support.

Sorry for any confusion.

[1] http://www.spinics.net/lists/arm-kernel/msg238387.html

Sylwester

[PATCH 1/7] clk: exynos: prepare for multiplatform

[Sylwester Nawrocki]

On 04/15/2013 10:35 AM, Arnd Bergmann wrote:
> On Monday 15 April 2013, Thomas Abraham wrote:
>> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
>>> The new common clock drivers for exynos are using compile
>>> time constants and soc_is_exynos* macros to provide backwards
>>> compatibility for pre-DT systems, which is not possible with
>>> multiplatform kernels. This moves all the necessary
>>> information back into platform code and removes the mach/*
>>> header inclusions.
>>
>>
>> Since kgene has submitted a patch to remove pre-dt Exynos support
>> which looks like it will be accepted, we could just remove the use of
>> soc_is_xyz() instead of continuing to support non-dt Exynos.
> 
> I think based on the most recent discussion, we are not actually doing
> that for 3.10, since there are still people that require ATAGS support.

It seems patches from Tomasz [1] will likely make it to 3.10, thus
it is fine to apply Kukjin's patch as far as we are concerned.

Besides, it seems some drivers are already broken with ATAGS booting
(e.g. Exynos DRM, SDHCI) due to introduction of the common clock
framework driver. It doesn't seem worthwhile to spend time fixing
this, I would rather focus on improving the DT support.

Sorry for any confusion.

[1] http://www.spinics.net/lists/arm-kernel/msg238387.html

Sylwester

[PATCH 2/7] clocksource: exynos_mct: remove platform header dependency

[Arnd Bergmann]

For the non-DT case, the mct_init() function requires access
to a couple of platform specific constants, but cannot include
the header files in case we are building for multiplatform.

This changes the interface to the platform so we pass all
the necessary data as arguments to mct_init.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
---
 arch/arm/mach-exynos/common.c    |  2 +-
 arch/arm/mach-exynos/common.h    |  2 +-
 drivers/clocksource/exynos_mct.c | 21 ++++++---------------
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 42f399a..59cb8fe 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -413,7 +413,7 @@ void __init exynos_init_time(void)
 		exynos4_clk_init(NULL, !soc_is_exynos4210(), S5P_VA_CMU, readl(S5P_VA_CHIPID + 8) & 1);
 		exynos4_clk_register_fixed_ext(xxti_f, xusbxti_f);
 #endif
-		mct_init();
+		mct_init(S5P_VA_SYSTIMER, EXYNOS4_IRQ_MCT_G0, EXYNOS4_IRQ_MCT_L0, EXYNOS4_IRQ_MCT_L1);
 	}
 }
 
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 4ed8eef..9717d0f 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -14,7 +14,7 @@
 
 #include <linux/of.h>
 
-extern void mct_init(void);
+void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
 void exynos_init_time(void);
 extern unsigned long xxti_f, xusbxti_f;
 
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 509a601..2292452 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -26,11 +26,6 @@
 
 #include <asm/arch_timer.h>
 #include <asm/localtimer.h>
-
-#include <plat/cpu.h>
-
-#include <mach/map.h>
-#include <mach/irqs.h>
 #include <asm/mach/time.h>
 
 #define EXYNOS4_MCTREG(x)		(x)
@@ -511,18 +506,14 @@ static void __init exynos4_timer_resources(struct device_node *np, void __iomem
 #endif /* CONFIG_LOCAL_TIMERS */
 }
 
-void __init mct_init(void)
+void __init mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1)
 {
-	if (soc_is_exynos4210()) {
-		mct_irqs[MCT_G0_IRQ] = EXYNOS4_IRQ_MCT_G0;
-		mct_irqs[MCT_L0_IRQ] = EXYNOS4_IRQ_MCT_L0;
-		mct_irqs[MCT_L1_IRQ] = EXYNOS4_IRQ_MCT_L1;
-		mct_int_type = MCT_INT_SPI;
-	} else {
-		panic("unable to determine mct controller type\n");
-	}
+	mct_irqs[MCT_G0_IRQ] = irq_g0;
+	mct_irqs[MCT_L0_IRQ] = irq_l0;
+	mct_irqs[MCT_L1_IRQ] = irq_l1;
+	mct_int_type = MCT_INT_SPI;
 
-	exynos4_timer_resources(NULL, S5P_VA_SYSTIMER);
+	exynos4_timer_resources(NULL, base);
 	exynos4_clocksource_init();
 	exynos4_clockevent_init();
 }
-- 
1.8.1.2

[PATCH 2/7] clocksource: exynos_mct: remove platform header dependency

[Arnd Bergmann]

For the non-DT case, the mct_init() function requires access
to a couple of platform specific constants, but cannot include
the header files in case we are building for multiplatform.

This changes the interface to the platform so we pass all
the necessary data as arguments to mct_init.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
---
 arch/arm/mach-exynos/common.c    |  2 +-
 arch/arm/mach-exynos/common.h    |  2 +-
 drivers/clocksource/exynos_mct.c | 21 ++++++---------------
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 42f399a..59cb8fe 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -413,7 +413,7 @@ void __init exynos_init_time(void)
 		exynos4_clk_init(NULL, !soc_is_exynos4210(), S5P_VA_CMU, readl(S5P_VA_CHIPID + 8) & 1);
 		exynos4_clk_register_fixed_ext(xxti_f, xusbxti_f);
 #endif
-		mct_init();
+		mct_init(S5P_VA_SYSTIMER, EXYNOS4_IRQ_MCT_G0, EXYNOS4_IRQ_MCT_L0, EXYNOS4_IRQ_MCT_L1);
 	}
 }
 
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 4ed8eef..9717d0f 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -14,7 +14,7 @@
 
 #include <linux/of.h>
 
-extern void mct_init(void);
+void mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1);
 void exynos_init_time(void);
 extern unsigned long xxti_f, xusbxti_f;
 
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 509a601..2292452 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -26,11 +26,6 @@
 
 #include <asm/arch_timer.h>
 #include <asm/localtimer.h>
-
-#include <plat/cpu.h>
-
-#include <mach/map.h>
-#include <mach/irqs.h>
 #include <asm/mach/time.h>
 
 #define EXYNOS4_MCTREG(x)		(x)
@@ -511,18 +506,14 @@ static void __init exynos4_timer_resources(struct device_node *np, void __iomem
 #endif /* CONFIG_LOCAL_TIMERS */
 }
 
-void __init mct_init(void)
+void __init mct_init(void __iomem *base, int irq_g0, int irq_l0, int irq_l1)
 {
-	if (soc_is_exynos4210()) {
-		mct_irqs[MCT_G0_IRQ] = EXYNOS4_IRQ_MCT_G0;
-		mct_irqs[MCT_L0_IRQ] = EXYNOS4_IRQ_MCT_L0;
-		mct_irqs[MCT_L1_IRQ] = EXYNOS4_IRQ_MCT_L1;
-		mct_int_type = MCT_INT_SPI;
-	} else {
-		panic("unable to determine mct controller type\n");
-	}
+	mct_irqs[MCT_G0_IRQ] = irq_g0;
+	mct_irqs[MCT_L0_IRQ] = irq_l0;
+	mct_irqs[MCT_L1_IRQ] = irq_l1;
+	mct_int_type = MCT_INT_SPI;
 
-	exynos4_timer_resources(NULL, S5P_VA_SYSTIMER);
+	exynos4_timer_resources(NULL, base);
 	exynos4_clocksource_init();
 	exynos4_clockevent_init();
 }
-- 
1.8.1.2

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

The exynos combiner irqchip needs to find the parent interrupts
and needs to know their number, so add the missing properties
for exynos4 as they were already present for exynos5.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/dts/exynos4210.dtsi | 1 +
 arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
 arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
 3 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index 15143bd..c102869 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -41,6 +41,7 @@
 	};
 
 	combiner:interrupt-controller@10440000 {
+		combiner-nr = <16>;
 		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
 			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
 			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
index 36d4299..f83c3c1 100644
--- a/arch/arm/boot/dts/exynos4212.dtsi
+++ b/arch/arm/boot/dts/exynos4212.dtsi
@@ -26,6 +26,15 @@
 		cpu-offset = <0x8000>;
 	};
 
+	interrupt-controller@10440000 {
+                combiner-nr = <18>;
+		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
+			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
+			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
+			     <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
+			     <0 107 0>, <0 108 0>;
+	};
+
 	mct@10050000 {
 		compatible = "samsung,exynos4412-mct";
 		reg = <0x10050000 0x800>;
diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index d75c047..4cb657e 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -26,6 +26,15 @@
 		cpu-offset = <0x4000>;
 	};
 
+	interrupt-controller@10440000 {
+		combiner-nr = <20>;
+		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
+			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
+			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
+			     <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
+			     <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
+	};
+
 	mct@10050000 {
 		compatible = "samsung,exynos4412-mct";
 		reg = <0x10050000 0x800>;
-- 
1.8.1.2

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

The exynos combiner irqchip needs to find the parent interrupts
and needs to know their number, so add the missing properties
for exynos4 as they were already present for exynos5.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/boot/dts/exynos4210.dtsi | 1 +
 arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
 arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
 3 files changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
index 15143bd..c102869 100644
--- a/arch/arm/boot/dts/exynos4210.dtsi
+++ b/arch/arm/boot/dts/exynos4210.dtsi
@@ -41,6 +41,7 @@
 	};
 
 	combiner:interrupt-controller at 10440000 {
+		combiner-nr = <16>;
 		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
 			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
 			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
index 36d4299..f83c3c1 100644
--- a/arch/arm/boot/dts/exynos4212.dtsi
+++ b/arch/arm/boot/dts/exynos4212.dtsi
@@ -26,6 +26,15 @@
 		cpu-offset = <0x8000>;
 	};
 
+	interrupt-controller at 10440000 {
+                combiner-nr = <18>;
+		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
+			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
+			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
+			     <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
+			     <0 107 0>, <0 108 0>;
+	};
+
 	mct at 10050000 {
 		compatible = "samsung,exynos4412-mct";
 		reg = <0x10050000 0x800>;
diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
index d75c047..4cb657e 100644
--- a/arch/arm/boot/dts/exynos4412.dtsi
+++ b/arch/arm/boot/dts/exynos4412.dtsi
@@ -26,6 +26,15 @@
 		cpu-offset = <0x4000>;
 	};
 
+	interrupt-controller at 10440000 {
+		combiner-nr = <20>;
+		interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
+			     <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
+			     <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
+			     <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
+			     <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
+	};
+
 	mct at 10050000 {
 		compatible = "samsung,exynos4412-mct";
 		reg = <0x10050000 0x800>;
-- 
1.8.1.2

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Thomas Abraham]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The exynos combiner irqchip needs to find the parent interrupts
> and needs to know their number, so add the missing properties
> for exynos4 as they were already present for exynos5.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
>  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 15143bd..c102869 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -41,6 +41,7 @@
>         };
>
>         combiner:interrupt-controller@10440000 {
> +               combiner-nr = <16>;

The default value of the combiner-nr is case it is not specified is
16. The default value is documented in the bindings documentation.

>                 interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>                              <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>                              <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 36d4299..f83c3c1 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x8000>;
>         };
>
> +       interrupt-controller@10440000 {
> +                combiner-nr = <18>;
> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>;
> +       };
> +
>         mct@10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index d75c047..4cb657e 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x4000>;
>         };
>
> +       interrupt-controller@10440000 {
> +               combiner-nr = <20>;
> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;

Extra '>' for <0 108 0> can be removed.

Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org?

> +       };
> +
>         mct@10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> --
> 1.8.1.2
>

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Thomas Abraham]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The exynos combiner irqchip needs to find the parent interrupts
> and needs to know their number, so add the missing properties
> for exynos4 as they were already present for exynos5.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
>  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 15143bd..c102869 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -41,6 +41,7 @@
>         };
>
>         combiner:interrupt-controller at 10440000 {
> +               combiner-nr = <16>;

The default value of the combiner-nr is case it is not specified is
16. The default value is documented in the bindings documentation.

>                 interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>                              <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>                              <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 36d4299..f83c3c1 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x8000>;
>         };
>
> +       interrupt-controller at 10440000 {
> +                combiner-nr = <18>;
> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>;
> +       };
> +
>         mct at 10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index d75c047..4cb657e 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x4000>;
>         };
>
> +       interrupt-controller at 10440000 {
> +               combiner-nr = <20>;
> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;

Extra '>' for <0 108 0> can be removed.

Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org?

> +       };
> +
>         mct at 10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> --
> 1.8.1.2
>

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Thomas Abraham wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> > The exynos combiner irqchip needs to find the parent interrupts
> > and needs to know their number, so add the missing properties
> > for exynos4 as they were already present for exynos5.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm/boot/dts/exynos4210.dtsi | 1 +
> >  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
> >  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
> >  3 files changed, 19 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> > index 15143bd..c102869 100644
> > --- a/arch/arm/boot/dts/exynos4210.dtsi
> > +++ b/arch/arm/boot/dts/exynos4210.dtsi
> > @@ -41,6 +41,7 @@
> >         };
> >
> >         combiner:interrupt-controller@10440000 {
> > +               combiner-nr = <16>;
> 
> The default value of the combiner-nr is case it is not specified is
> 16. The default value is documented in the bindings documentation.

Unfortunately, the driver now warns about a missing attribute, which
seems right. I also made the default "20" instead of "16" to cover the
other exynos4 chips. I'd say it's best to keep this one explicitly listed
even if it's optional.

> > +       interrupt-controller@10440000 {
> > +               combiner-nr = <20>;
> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
> 
> Extra '>' for <0 108 0> can be removed.
> 
> Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org?

Thanks!

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Thomas Abraham wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> > The exynos combiner irqchip needs to find the parent interrupts
> > and needs to know their number, so add the missing properties
> > for exynos4 as they were already present for exynos5.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm/boot/dts/exynos4210.dtsi | 1 +
> >  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
> >  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
> >  3 files changed, 19 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> > index 15143bd..c102869 100644
> > --- a/arch/arm/boot/dts/exynos4210.dtsi
> > +++ b/arch/arm/boot/dts/exynos4210.dtsi
> > @@ -41,6 +41,7 @@
> >         };
> >
> >         combiner:interrupt-controller at 10440000 {
> > +               combiner-nr = <16>;
> 
> The default value of the combiner-nr is case it is not specified is
> 16. The default value is documented in the bindings documentation.

Unfortunately, the driver now warns about a missing attribute, which
seems right. I also made the default "20" instead of "16" to cover the
other exynos4 chips. I'd say it's best to keep this one explicitly listed
even if it's optional.

> > +       interrupt-controller at 10440000 {
> > +               combiner-nr = <20>;
> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
> 
> Extra '>' for <0 108 0> can be removed.
> 
> Reviewed-by: Thomas Abraham <thomas.abraham@linaro.org?

Thanks!

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The exynos combiner irqchip needs to find the parent interrupts
> and needs to know their number, so add the missing properties
> for exynos4 as they were already present for exynos5.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
>  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 15143bd..c102869 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -41,6 +41,7 @@
>         };
>
>         combiner:interrupt-controller@10440000 {
> +               combiner-nr = <16>;

I think this should be "samsung,combiner-nr"
Without this i see the following message in boot log:
"combiner_of_init: number of combiners not specified"

>                 interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>                              <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>                              <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 36d4299..f83c3c1 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x8000>;
>         };
>
> +       interrupt-controller@10440000 {

Don't we need a node name here (combiner:interrupt-controller@10440000)?

> +                combiner-nr = <18>;
    ^^^^^^^^^^^
nit: tabs instead of space would look better.

> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>;
> +       };
> +
>         mct@10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index d75c047..4cb657e 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x4000>;
>         };
>
> +       interrupt-controller@10440000 {
> +               combiner-nr = <20>;
Same as above (for exynos4212.dtsi).

> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
                                                              ^^^
Syntax error.

We have a combiner node defined in exynos4x12.dtsi. With the bindings
now defined separately in 4212 and 4412 dtsi files, probably the one
in 4x12 could be dropped?


-- 
With warm regards,
Sachin

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:
> The exynos combiner irqchip needs to find the parent interrupts
> and needs to know their number, so add the missing properties
> for exynos4 as they were already present for exynos5.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/boot/dts/exynos4210.dtsi | 1 +
>  arch/arm/boot/dts/exynos4212.dtsi | 9 +++++++++
>  arch/arm/boot/dts/exynos4412.dtsi | 9 +++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4210.dtsi b/arch/arm/boot/dts/exynos4210.dtsi
> index 15143bd..c102869 100644
> --- a/arch/arm/boot/dts/exynos4210.dtsi
> +++ b/arch/arm/boot/dts/exynos4210.dtsi
> @@ -41,6 +41,7 @@
>         };
>
>         combiner:interrupt-controller at 10440000 {
> +               combiner-nr = <16>;

I think this should be "samsung,combiner-nr"
Without this i see the following message in boot log:
"combiner_of_init: number of combiners not specified"

>                 interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>                              <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>                              <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> index 36d4299..f83c3c1 100644
> --- a/arch/arm/boot/dts/exynos4212.dtsi
> +++ b/arch/arm/boot/dts/exynos4212.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x8000>;
>         };
>
> +       interrupt-controller at 10440000 {

Don't we need a node name here (combiner:interrupt-controller at 10440000)?

> +                combiner-nr = <18>;
    ^^^^^^^^^^^
nit: tabs instead of space would look better.

> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>;
> +       };
> +
>         mct at 10050000 {
>                 compatible = "samsung,exynos4412-mct";
>                 reg = <0x10050000 0x800>;
> diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> index d75c047..4cb657e 100644
> --- a/arch/arm/boot/dts/exynos4412.dtsi
> +++ b/arch/arm/boot/dts/exynos4412.dtsi
> @@ -26,6 +26,15 @@
>                 cpu-offset = <0x4000>;
>         };
>
> +       interrupt-controller at 10440000 {
> +               combiner-nr = <20>;
Same as above (for exynos4212.dtsi).

> +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
                                                              ^^^
Syntax error.

We have a combiner node defined in exynos4x12.dtsi. With the bindings
now defined separately in 4212 and 4412 dtsi files, probably the one
in 4x12 could be dropped?


-- 
With warm regards,
Sachin

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Sachin Kamat wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:

> >
> >         combiner:interrupt-controller@10440000 {
> > +               combiner-nr = <16>;
> 
> I think this should be "samsung,combiner-nr"
> Without this i see the following message in boot log:
> "combiner_of_init: number of combiners not specified"

Right, thanks!

> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> > index 36d4299..f83c3c1 100644
> > --- a/arch/arm/boot/dts/exynos4212.dtsi
> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
> > @@ -26,6 +26,15 @@
> >                 cpu-offset = <0x8000>;
> >         };
> >
> > +       interrupt-controller@10440000 {
> 
> Don't we need a node name here (combiner:interrupt-controller@10440000)?

Why? The "combiner:" part is just a label. Actually I think the preferred
syntax is 

&combiner {
	samsung,combiner-nr = <18>;	
};

which is much shorter to write, but I did not want to change the style used
in the rest of the file.

> > +                combiner-nr = <18>;
>     ^^^^^^^^^^^
> nit: tabs instead of space would look better.

I fixed that up locally, but I think I sent out the wrong version.

> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> > index d75c047..4cb657e 100644
> > --- a/arch/arm/boot/dts/exynos4412.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412.dtsi
> > @@ -26,6 +26,15 @@
> >                 cpu-offset = <0x4000>;
> >         };
> >
> > +       interrupt-controller@10440000 {
> > +               combiner-nr = <20>;
> Same as above (for exynos4212.dtsi).
> 
> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
>                                                               ^^^
> Syntax error.
> 
> We have a combiner node defined in exynos4x12.dtsi. With the bindings
> now defined separately in 4212 and 4412 dtsi files, probably the one
> in 4x12 could be dropped?

I did not see that one, but it seems to have the wrong numbers in the last
four interrupt specifiers. I think it would be better to just fix that one
and keep using it.

	Arnd

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Sachin Kamat wrote:
> On 12 April 2013 19:33, Arnd Bergmann <arnd@arndb.de> wrote:

> >
> >         combiner:interrupt-controller at 10440000 {
> > +               combiner-nr = <16>;
> 
> I think this should be "samsung,combiner-nr"
> Without this i see the following message in boot log:
> "combiner_of_init: number of combiners not specified"

Right, thanks!

> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> > index 36d4299..f83c3c1 100644
> > --- a/arch/arm/boot/dts/exynos4212.dtsi
> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
> > @@ -26,6 +26,15 @@
> >                 cpu-offset = <0x8000>;
> >         };
> >
> > +       interrupt-controller at 10440000 {
> 
> Don't we need a node name here (combiner:interrupt-controller at 10440000)?

Why? The "combiner:" part is just a label. Actually I think the preferred
syntax is 

&combiner {
	samsung,combiner-nr = <18>;	
};

which is much shorter to write, but I did not want to change the style used
in the rest of the file.

> > +                combiner-nr = <18>;
>     ^^^^^^^^^^^
> nit: tabs instead of space would look better.

I fixed that up locally, but I think I sent out the wrong version.

> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
> > index d75c047..4cb657e 100644
> > --- a/arch/arm/boot/dts/exynos4412.dtsi
> > +++ b/arch/arm/boot/dts/exynos4412.dtsi
> > @@ -26,6 +26,15 @@
> >                 cpu-offset = <0x4000>;
> >         };
> >
> > +       interrupt-controller at 10440000 {
> > +               combiner-nr = <20>;
> Same as above (for exynos4212.dtsi).
> 
> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
>                                                               ^^^
> Syntax error.
> 
> We have a combiner node defined in exynos4x12.dtsi. With the bindings
> now defined separately in 4212 and 4412 dtsi files, probably the one
> in 4x12 could be dropped?

I did not see that one, but it seems to have the wrong numbers in the last
four interrupt specifiers. I think it would be better to just fix that one
and keep using it.

	Arnd

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

>> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
>> > index 36d4299..f83c3c1 100644
>> > --- a/arch/arm/boot/dts/exynos4212.dtsi
>> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
>> > @@ -26,6 +26,15 @@
>> >                 cpu-offset = <0x8000>;
>> >         };
>> >
>> > +       interrupt-controller@10440000 {
>>
>> Don't we need a node name here (combiner:interrupt-controller@10440000)?
>
> Why?

Since it is being referenced as "&combiner" by other bindings, i
thought that was required.

The "combiner:" part is just a label. Actually I think the preferred
> syntax is
>
> &combiner {
>         samsung,combiner-nr = <18>;
> };
>
> which is much shorter to write, but I did not want to change the style used
> in the rest of the file.
>
>> > +                combiner-nr = <18>;
>>     ^^^^^^^^^^^
>> nit: tabs instead of space would look better.
>
> I fixed that up locally, but I think I sent out the wrong version.

OK.

>
>> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
>> > index d75c047..4cb657e 100644
>> > --- a/arch/arm/boot/dts/exynos4412.dtsi
>> > +++ b/arch/arm/boot/dts/exynos4412.dtsi
>> > @@ -26,6 +26,15 @@
>> >                 cpu-offset = <0x4000>;
>> >         };
>> >
>> > +       interrupt-controller@10440000 {
>> > +               combiner-nr = <20>;
>> Same as above (for exynos4212.dtsi).
>>
>> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
>> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
>> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
>>                                                               ^^^
>> Syntax error.
>>
>> We have a combiner node defined in exynos4x12.dtsi. With the bindings
>> now defined separately in 4212 and 4412 dtsi files, probably the one
>> in 4x12 could be dropped?
>
> I did not see that one, but it seems to have the wrong numbers in the last
> four interrupt specifiers. I think it would be better to just fix that one
> and keep using it.

OK. Do you want me to fix it and send as a patch or would you like to do it?


-- 
With warm regards,
Sachin

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

>> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
>> > index 36d4299..f83c3c1 100644
>> > --- a/arch/arm/boot/dts/exynos4212.dtsi
>> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
>> > @@ -26,6 +26,15 @@
>> >                 cpu-offset = <0x8000>;
>> >         };
>> >
>> > +       interrupt-controller at 10440000 {
>>
>> Don't we need a node name here (combiner:interrupt-controller at 10440000)?
>
> Why?

Since it is being referenced as "&combiner" by other bindings, i
thought that was required.

The "combiner:" part is just a label. Actually I think the preferred
> syntax is
>
> &combiner {
>         samsung,combiner-nr = <18>;
> };
>
> which is much shorter to write, but I did not want to change the style used
> in the rest of the file.
>
>> > +                combiner-nr = <18>;
>>     ^^^^^^^^^^^
>> nit: tabs instead of space would look better.
>
> I fixed that up locally, but I think I sent out the wrong version.

OK.

>
>> > diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
>> > index d75c047..4cb657e 100644
>> > --- a/arch/arm/boot/dts/exynos4412.dtsi
>> > +++ b/arch/arm/boot/dts/exynos4412.dtsi
>> > @@ -26,6 +26,15 @@
>> >                 cpu-offset = <0x4000>;
>> >         };
>> >
>> > +       interrupt-controller at 10440000 {
>> > +               combiner-nr = <20>;
>> Same as above (for exynos4212.dtsi).
>>
>> > +               interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
>> > +                            <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>,
>> > +                            <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>,
>> > +                            <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>,
>> > +                            <0 107 0>, <0 108 0>>, <0 48 0>, <0 42 0>;
>>                                                               ^^^
>> Syntax error.
>>
>> We have a combiner node defined in exynos4x12.dtsi. With the bindings
>> now defined separately in 4212 and 4412 dtsi files, probably the one
>> in 4x12 could be dropped?
>
> I did not see that one, but it seems to have the wrong numbers in the last
> four interrupt specifiers. I think it would be better to just fix that one
> and keep using it.

OK. Do you want me to fix it and send as a patch or would you like to do it?


-- 
With warm regards,
Sachin

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Sachin Kamat wrote:
> >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> >> > index 36d4299..f83c3c1 100644
> >> > --- a/arch/arm/boot/dts/exynos4212.dtsi
> >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
> >> > @@ -26,6 +26,15 @@
> >> >                 cpu-offset = <0x8000>;
> >> >         };
> >> >
> >> > +       interrupt-controller@10440000 {
> >>
> >> Don't we need a node name here (combiner:interrupt-controller@10440000)?
> >
> > Why?
> 
> Since it is being referenced as "&combiner" by other bindings, i
> thought that was required.

The original definition actually comes from exynos4.dtsi. It's enough
to have the label in one place.

> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings
> >> now defined separately in 4212 and 4412 dtsi files, probably the one
> >> in 4x12 could be dropped?
> >
> > I did not see that one, but it seems to have the wrong numbers in the last
> > four interrupt specifiers. I think it would be better to just fix that one
> > and keep using it.
> 
> OK. Do you want me to fix it and send as a patch or would you like to do it?

If you have a patch that works, you could just send it out, oetherwise I'd
do it.

	ARnd

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Arnd Bergmann]

On Monday 15 April 2013, Sachin Kamat wrote:
> >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
> >> > index 36d4299..f83c3c1 100644
> >> > --- a/arch/arm/boot/dts/exynos4212.dtsi
> >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
> >> > @@ -26,6 +26,15 @@
> >> >                 cpu-offset = <0x8000>;
> >> >         };
> >> >
> >> > +       interrupt-controller at 10440000 {
> >>
> >> Don't we need a node name here (combiner:interrupt-controller at 10440000)?
> >
> > Why?
> 
> Since it is being referenced as "&combiner" by other bindings, i
> thought that was required.

The original definition actually comes from exynos4.dtsi. It's enough
to have the label in one place.

> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings
> >> now defined separately in 4212 and 4412 dtsi files, probably the one
> >> in 4x12 could be dropped?
> >
> > I did not see that one, but it seems to have the wrong numbers in the last
> > four interrupt specifiers. I think it would be better to just fix that one
> > and keep using it.
> 
> OK. Do you want me to fix it and send as a patch or would you like to do it?

If you have a patch that works, you could just send it out, oetherwise I'd
do it.

	ARnd

Re: [PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

On 15 April 2013 17:50, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 15 April 2013, Sachin Kamat wrote:
>> >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
>> >> > index 36d4299..f83c3c1 100644
>> >> > --- a/arch/arm/boot/dts/exynos4212.dtsi
>> >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
>> >> > @@ -26,6 +26,15 @@
>> >> >                 cpu-offset = <0x8000>;
>> >> >         };
>> >> >
>> >> > +       interrupt-controller@10440000 {
>> >>
>> >> Don't we need a node name here (combiner:interrupt-controller@10440000)?
>> >
>> > Why?
>>
>> Since it is being referenced as "&combiner" by other bindings, i
>> thought that was required.
>
> The original definition actually comes from exynos4.dtsi. It's enough
> to have the label in one place.

Yes, right. I overlooked that.

>
>> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings
>> >> now defined separately in 4212 and 4412 dtsi files, probably the one
>> >> in 4x12 could be dropped?
>> >
>> > I did not see that one, but it seems to have the wrong numbers in the last
>> > four interrupt specifiers. I think it would be better to just fix that one
>> > and keep using it.
>>
>> OK. Do you want me to fix it and send as a patch or would you like to do it?
>
> If you have a patch that works, you could just send it out, oetherwise I'd
> do it.

Not ATM, so please go ahead.

-- 
With warm regards,
Sachin

[PATCH 3/7] ARM: exynos: add missing properties for combiner IRQs

[Sachin Kamat]

On 15 April 2013 17:50, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 15 April 2013, Sachin Kamat wrote:
>> >> > diff --git a/arch/arm/boot/dts/exynos4212.dtsi b/arch/arm/boot/dts/exynos4212.dtsi
>> >> > index 36d4299..f83c3c1 100644
>> >> > --- a/arch/arm/boot/dts/exynos4212.dtsi
>> >> > +++ b/arch/arm/boot/dts/exynos4212.dtsi
>> >> > @@ -26,6 +26,15 @@
>> >> >                 cpu-offset = <0x8000>;
>> >> >         };
>> >> >
>> >> > +       interrupt-controller at 10440000 {
>> >>
>> >> Don't we need a node name here (combiner:interrupt-controller at 10440000)?
>> >
>> > Why?
>>
>> Since it is being referenced as "&combiner" by other bindings, i
>> thought that was required.
>
> The original definition actually comes from exynos4.dtsi. It's enough
> to have the label in one place.

Yes, right. I overlooked that.

>
>> >> We have a combiner node defined in exynos4x12.dtsi. With the bindings
>> >> now defined separately in 4212 and 4412 dtsi files, probably the one
>> >> in 4x12 could be dropped?
>> >
>> > I did not see that one, but it seems to have the wrong numbers in the last
>> > four interrupt specifiers. I think it would be better to just fix that one
>> > and keep using it.
>>
>> OK. Do you want me to fix it and send as a patch or would you like to do it?
>
> If you have a patch that works, you could just send it out, oetherwise I'd
> do it.

Not ATM, so please go ahead.

-- 
With warm regards,
Sachin

[PATCH 4/7] irqchip: exynos: pass max combiner number to combiner_init

[Arnd Bergmann]

We can find out the number of combined IRQs from the device
tree, but in case of ATAGS boot, the driver currently uses
hardcoded values based on the SoC type. We can't do that
in general for a multiplatform kernel, so let's instead pass
this information from platform code directly in case of
ATAGS boot.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-exynos/common.c     | 15 ++++++++++++-
 arch/arm/mach-exynos/common.h     |  3 ++-
 drivers/irqchip/exynos-combiner.c | 46 ++++++++++++++-------------------------
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 59cb8fe..ecdd212 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -417,6 +417,19 @@ void __init exynos_init_time(void)
 	}
 }
 
+static unsigned int max_combiner_nr(void)
+{
+	if (soc_is_exynos5250())
+		return EXYNOS5_MAX_COMBINER_NR;
+	else if (soc_is_exynos4412())
+		return EXYNOS4412_MAX_COMBINER_NR;
+	else if (soc_is_exynos4212())
+		return EXYNOS4212_MAX_COMBINER_NR;
+	else
+		return EXYNOS4210_MAX_COMBINER_NR;
+}
+
+
 void __init exynos4_init_irq(void)
 {
 	unsigned int gic_bank_offset;
@@ -431,7 +444,7 @@ void __init exynos4_init_irq(void)
 #endif
 
 	if (!of_have_populated_dt())
-		combiner_init(S5P_VA_COMBINER_BASE, NULL);
+		combiner_init(S5P_VA_COMBINER_BASE, NULL, max_combiner_nr());
 
 	/*
 	 * The parameters of s5p_init_irq() are for VIC init.
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9717d0f..4ba8cbe 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -69,7 +69,8 @@ void exynos4212_register_clocks(void);
 #endif
 
 struct device_node;
-void combiner_init(void __iomem *combiner_base, struct device_node *np);
+void combiner_init(void __iomem *combiner_base, struct device_node *np,
+			unsigned int max_nr);
 
 extern struct smp_operations exynos_smp_ops;
 
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index e8501db..d868383 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -25,6 +25,8 @@
 #define COMBINER_ENABLE_CLEAR	0x4
 #define COMBINER_INT_STATUS	0xC
 
+#define IRQ_IN_COMBINER		8
+
 static DEFINE_SPINLOCK(irq_controller_lock);
 
 struct combiner_chip_data {
@@ -112,23 +114,9 @@ static struct irq_chip combiner_chip = {
 #endif
 };
 
-static unsigned int max_combiner_nr(void)
-{
-	if (soc_is_exynos5250())
-		return EXYNOS5_MAX_COMBINER_NR;
-	else if (soc_is_exynos4412())
-		return EXYNOS4412_MAX_COMBINER_NR;
-	else if (soc_is_exynos4212())
-		return EXYNOS4212_MAX_COMBINER_NR;
-	else
-		return EXYNOS4210_MAX_COMBINER_NR;
-}
-
 static void __init combiner_cascade_irq(unsigned int combiner_nr,
 					unsigned int irq)
 {
-	if (combiner_nr >= max_combiner_nr())
-		BUG();
 	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
 		BUG();
 	irq_set_chained_handler(irq, combiner_handle_cascade_irq);
@@ -139,7 +127,7 @@ static void __init combiner_init_one(unsigned int combiner_nr,
 {
 	combiner_data[combiner_nr].base = base;
 	combiner_data[combiner_nr].irq_offset = irq_find_mapping(
-		combiner_irq_domain, combiner_nr * MAX_IRQ_IN_COMBINER);
+		combiner_irq_domain, combiner_nr * IRQ_IN_COMBINER);
 	combiner_data[combiner_nr].irq_mask = 0xff << ((combiner_nr % 4) << 3);
 	combiner_data[combiner_nr].parent_irq = irq;
 
@@ -161,7 +149,7 @@ static int combiner_irq_domain_xlate(struct irq_domain *d,
 	if (intsize < 2)
 		return -EINVAL;
 
-	*out_hwirq = intspec[0] * MAX_IRQ_IN_COMBINER + intspec[1];
+	*out_hwirq = intspec[0] * IRQ_IN_COMBINER + intspec[1];
 	*out_type = 0;
 
 	return 0;
@@ -209,22 +197,13 @@ static unsigned int exynos4x12_combiner_extra_irq(int group)
 }
 
 void __init combiner_init(void __iomem *combiner_base,
-			  struct device_node *np)
+			  struct device_node *np,
+			  unsigned int max_nr)
 {
 	int i, irq, irq_base;
-	unsigned int max_nr, nr_irq;
+	unsigned int nr_irq;
 
-	max_nr = max_combiner_nr();
-
-	if (np) {
-		if (of_property_read_u32(np, "samsung,combiner-nr", &max_nr)) {
-			pr_info("%s: number of combiners not specified, "
-				"setting default as %d.\n",
-				__func__, max_nr);
-		}
-	}
-
-	nr_irq = max_nr * MAX_IRQ_IN_COMBINER;
+	nr_irq = max_nr * IRQ_IN_COMBINER;
 
 	irq_base = irq_alloc_descs(COMBINER_IRQ(0, 0), 1, nr_irq, 0);
 	if (IS_ERR_VALUE(irq_base)) {
@@ -258,6 +237,7 @@ static int __init combiner_of_init(struct device_node *np,
 				   struct device_node *parent)
 {
 	void __iomem *combiner_base;
+	unsigned int max_nr = 20;
 
 	combiner_base = of_iomap(np, 0);
 	if (!combiner_base) {
@@ -265,7 +245,13 @@ static int __init combiner_of_init(struct device_node *np,
 		return -ENXIO;
 	}
 
-	combiner_init(combiner_base, np);
+	if (of_property_read_u32(np, "samsung,combiner-nr", &max_nr)) {
+		pr_info("%s: number of combiners not specified, "
+			"setting default as %d.\n",
+			__func__, max_nr);
+	}
+
+	combiner_init(combiner_base, np, max_nr);
 
 	return 0;
 }
-- 
1.8.1.2

[PATCH 4/7] irqchip: exynos: pass max combiner number to combiner_init

[Arnd Bergmann]

We can find out the number of combined IRQs from the device
tree, but in case of ATAGS boot, the driver currently uses
hardcoded values based on the SoC type. We can't do that
in general for a multiplatform kernel, so let's instead pass
this information from platform code directly in case of
ATAGS boot.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-exynos/common.c     | 15 ++++++++++++-
 arch/arm/mach-exynos/common.h     |  3 ++-
 drivers/irqchip/exynos-combiner.c | 46 ++++++++++++++-------------------------
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index 59cb8fe..ecdd212 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -417,6 +417,19 @@ void __init exynos_init_time(void)
 	}
 }
 
+static unsigned int max_combiner_nr(void)
+{
+	if (soc_is_exynos5250())
+		return EXYNOS5_MAX_COMBINER_NR;
+	else if (soc_is_exynos4412())
+		return EXYNOS4412_MAX_COMBINER_NR;
+	else if (soc_is_exynos4212())
+		return EXYNOS4212_MAX_COMBINER_NR;
+	else
+		return EXYNOS4210_MAX_COMBINER_NR;
+}
+
+
 void __init exynos4_init_irq(void)
 {
 	unsigned int gic_bank_offset;
@@ -431,7 +444,7 @@ void __init exynos4_init_irq(void)
 #endif
 
 	if (!of_have_populated_dt())
-		combiner_init(S5P_VA_COMBINER_BASE, NULL);
+		combiner_init(S5P_VA_COMBINER_BASE, NULL, max_combiner_nr());
 
 	/*
 	 * The parameters of s5p_init_irq() are for VIC init.
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 9717d0f..4ba8cbe 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -69,7 +69,8 @@ void exynos4212_register_clocks(void);
 #endif
 
 struct device_node;
-void combiner_init(void __iomem *combiner_base, struct device_node *np);
+void combiner_init(void __iomem *combiner_base, struct device_node *np,
+			unsigned int max_nr);
 
 extern struct smp_operations exynos_smp_ops;
 
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index e8501db..d868383 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -25,6 +25,8 @@
 #define COMBINER_ENABLE_CLEAR	0x4
 #define COMBINER_INT_STATUS	0xC
 
+#define IRQ_IN_COMBINER		8
+
 static DEFINE_SPINLOCK(irq_controller_lock);
 
 struct combiner_chip_data {
@@ -112,23 +114,9 @@ static struct irq_chip combiner_chip = {
 #endif
 };
 
-static unsigned int max_combiner_nr(void)
-{
-	if (soc_is_exynos5250())
-		return EXYNOS5_MAX_COMBINER_NR;
-	else if (soc_is_exynos4412())
-		return EXYNOS4412_MAX_COMBINER_NR;
-	else if (soc_is_exynos4212())
-		return EXYNOS4212_MAX_COMBINER_NR;
-	else
-		return EXYNOS4210_MAX_COMBINER_NR;
-}
-
 static void __init combiner_cascade_irq(unsigned int combiner_nr,
 					unsigned int irq)
 {
-	if (combiner_nr >= max_combiner_nr())
-		BUG();
 	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
 		BUG();
 	irq_set_chained_handler(irq, combiner_handle_cascade_irq);
@@ -139,7 +127,7 @@ static void __init combiner_init_one(unsigned int combiner_nr,
 {
 	combiner_data[combiner_nr].base = base;
 	combiner_data[combiner_nr].irq_offset = irq_find_mapping(
-		combiner_irq_domain, combiner_nr * MAX_IRQ_IN_COMBINER);
+		combiner_irq_domain, combiner_nr * IRQ_IN_COMBINER);
 	combiner_data[combiner_nr].irq_mask = 0xff << ((combiner_nr % 4) << 3);
 	combiner_data[combiner_nr].parent_irq = irq;
 
@@ -161,7 +149,7 @@ static int combiner_irq_domain_xlate(struct irq_domain *d,
 	if (intsize < 2)
 		return -EINVAL;
 
-	*out_hwirq = intspec[0] * MAX_IRQ_IN_COMBINER + intspec[1];
+	*out_hwirq = intspec[0] * IRQ_IN_COMBINER + intspec[1];
 	*out_type = 0;
 
 	return 0;
@@ -209,22 +197,13 @@ static unsigned int exynos4x12_combiner_extra_irq(int group)
 }
 
 void __init combiner_init(void __iomem *combiner_base,
-			  struct device_node *np)
+			  struct device_node *np,
+			  unsigned int max_nr)
 {
 	int i, irq, irq_base;
-	unsigned int max_nr, nr_irq;
+	unsigned int nr_irq;
 
-	max_nr = max_combiner_nr();
-
-	if (np) {
-		if (of_property_read_u32(np, "samsung,combiner-nr", &max_nr)) {
-			pr_info("%s: number of combiners not specified, "
-				"setting default as %d.\n",
-				__func__, max_nr);
-		}
-	}
-
-	nr_irq = max_nr * MAX_IRQ_IN_COMBINER;
+	nr_irq = max_nr * IRQ_IN_COMBINER;
 
 	irq_base = irq_alloc_descs(COMBINER_IRQ(0, 0), 1, nr_irq, 0);
 	if (IS_ERR_VALUE(irq_base)) {
@@ -258,6 +237,7 @@ static int __init combiner_of_init(struct device_node *np,
 				   struct device_node *parent)
 {
 	void __iomem *combiner_base;
+	unsigned int max_nr = 20;
 
 	combiner_base = of_iomap(np, 0);
 	if (!combiner_base) {
@@ -265,7 +245,13 @@ static int __init combiner_of_init(struct device_node *np,
 		return -ENXIO;
 	}
 
-	combiner_init(combiner_base, np);
+	if (of_property_read_u32(np, "samsung,combiner-nr", &max_nr)) {
+		pr_info("%s: number of combiners not specified, "
+			"setting default as %d.\n",
+			__func__, max_nr);
+	}
+
+	combiner_init(combiner_base, np, max_nr);
 
 	return 0;
 }
-- 
1.8.1.2

[PATCH 5/7] irqchip: exynos: allocate combiner_data dynamically

[Arnd Bergmann]

The number of combiners on a given SoC is a platform specific
constant, and we cannot encode this number on a multiplatform
kernel since the header file defining it is not available.

Allocating the structure dynamically ends up cleaner anyway
since we keep all the data local.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/exynos-combiner.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index d868383..7fcdeee 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -12,6 +12,7 @@
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/slab.h>
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -37,7 +38,6 @@ struct combiner_chip_data {
 };
 
 static struct irq_domain *combiner_irq_domain;
-static struct combiner_chip_data combiner_data[MAX_COMBINER_NR];
 
 static inline void __iomem *combiner_base(struct irq_data *data)
 {
@@ -114,26 +114,26 @@ static struct irq_chip combiner_chip = {
 #endif
 };
 
-static void __init combiner_cascade_irq(unsigned int combiner_nr,
+static void __init combiner_cascade_irq(struct combiner_chip_data *combiner_data,
 					unsigned int irq)
 {
-	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
+	if (irq_set_handler_data(irq, combiner_data) != 0)
 		BUG();
 	irq_set_chained_handler(irq, combiner_handle_cascade_irq);
 }
 
-static void __init combiner_init_one(unsigned int combiner_nr,
+static void __init combiner_init_one(struct combiner_chip_data *combiner_data,
+				     unsigned int combiner_nr,
 				     void __iomem *base, unsigned int irq)
 {
-	combiner_data[combiner_nr].base = base;
-	combiner_data[combiner_nr].irq_offset = irq_find_mapping(
+	combiner_data->base = base;
+	combiner_data->irq_offset = irq_find_mapping(
 		combiner_irq_domain, combiner_nr * IRQ_IN_COMBINER);
-	combiner_data[combiner_nr].irq_mask = 0xff << ((combiner_nr % 4) << 3);
-	combiner_data[combiner_nr].parent_irq = irq;
+	combiner_data->irq_mask = 0xff << ((combiner_nr % 4) << 3);
+	combiner_data->parent_irq = irq;
 
 	/* Disable all interrupts */
-	__raw_writel(combiner_data[combiner_nr].irq_mask,
-		     base + COMBINER_ENABLE_CLEAR);
+	__raw_writel(combiner_data->irq_mask, base + COMBINER_ENABLE_CLEAR);
 }
 
 #ifdef CONFIG_OF
@@ -168,6 +168,8 @@ static int combiner_irq_domain_xlate(struct irq_domain *d,
 static int combiner_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				   irq_hw_number_t hw)
 {
+	struct combiner_chip_data *combiner_data = d->host_data;
+
 	irq_set_chip_and_handler(irq, &combiner_chip, handle_level_irq);
 	irq_set_chip_data(irq, &combiner_data[hw >> 3]);
 	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
@@ -202,6 +204,7 @@ void __init combiner_init(void __iomem *combiner_base,
 {
 	int i, irq, irq_base;
 	unsigned int nr_irq;
+	struct combiner_chip_data *combiner_data;
 
 	nr_irq = max_nr * IRQ_IN_COMBINER;
 
@@ -211,8 +214,14 @@ void __init combiner_init(void __iomem *combiner_base,
 		pr_warning("%s: irq desc alloc failed. Continuing with %d as linux irq base\n", __func__, irq_base);
 	}
 
+	combiner_data = kcalloc(max_nr, sizeof (*combiner_data), GFP_KERNEL);
+	if (!combiner_data) {
+		pr_warning("%s: could not allocate combiner data\n", __func__);
+		return;
+	}
+
 	combiner_irq_domain = irq_domain_add_legacy(np, nr_irq, irq_base, 0,
-				&combiner_irq_domain_ops, &combiner_data);
+				&combiner_irq_domain_ops, combiner_data);
 	if (WARN_ON(!combiner_irq_domain)) {
 		pr_warning("%s: irq domain init failed\n", __func__);
 		return;
@@ -227,8 +236,9 @@ void __init combiner_init(void __iomem *combiner_base,
 		if (np)
 			irq = irq_of_parse_and_map(np, i);
 #endif
-		combiner_init_one(i, combiner_base + (i >> 2) * 0x10, irq);
-		combiner_cascade_irq(i, irq);
+		combiner_init_one(&combiner_data[i], i,
+				  combiner_base + (i >> 2) * 0x10, irq);
+		combiner_cascade_irq(&combiner_data[i], irq);
 	}
 }
 
-- 
1.8.1.2

[PATCH 5/7] irqchip: exynos: allocate combiner_data dynamically

[Arnd Bergmann]

The number of combiners on a given SoC is a platform specific
constant, and we cannot encode this number on a multiplatform
kernel since the header file defining it is not available.

Allocating the structure dynamically ends up cleaner anyway
since we keep all the data local.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/exynos-combiner.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index d868383..7fcdeee 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -12,6 +12,7 @@
 #include <linux/export.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/slab.h>
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
@@ -37,7 +38,6 @@ struct combiner_chip_data {
 };
 
 static struct irq_domain *combiner_irq_domain;
-static struct combiner_chip_data combiner_data[MAX_COMBINER_NR];
 
 static inline void __iomem *combiner_base(struct irq_data *data)
 {
@@ -114,26 +114,26 @@ static struct irq_chip combiner_chip = {
 #endif
 };
 
-static void __init combiner_cascade_irq(unsigned int combiner_nr,
+static void __init combiner_cascade_irq(struct combiner_chip_data *combiner_data,
 					unsigned int irq)
 {
-	if (irq_set_handler_data(irq, &combiner_data[combiner_nr]) != 0)
+	if (irq_set_handler_data(irq, combiner_data) != 0)
 		BUG();
 	irq_set_chained_handler(irq, combiner_handle_cascade_irq);
 }
 
-static void __init combiner_init_one(unsigned int combiner_nr,
+static void __init combiner_init_one(struct combiner_chip_data *combiner_data,
+				     unsigned int combiner_nr,
 				     void __iomem *base, unsigned int irq)
 {
-	combiner_data[combiner_nr].base = base;
-	combiner_data[combiner_nr].irq_offset = irq_find_mapping(
+	combiner_data->base = base;
+	combiner_data->irq_offset = irq_find_mapping(
 		combiner_irq_domain, combiner_nr * IRQ_IN_COMBINER);
-	combiner_data[combiner_nr].irq_mask = 0xff << ((combiner_nr % 4) << 3);
-	combiner_data[combiner_nr].parent_irq = irq;
+	combiner_data->irq_mask = 0xff << ((combiner_nr % 4) << 3);
+	combiner_data->parent_irq = irq;
 
 	/* Disable all interrupts */
-	__raw_writel(combiner_data[combiner_nr].irq_mask,
-		     base + COMBINER_ENABLE_CLEAR);
+	__raw_writel(combiner_data->irq_mask, base + COMBINER_ENABLE_CLEAR);
 }
 
 #ifdef CONFIG_OF
@@ -168,6 +168,8 @@ static int combiner_irq_domain_xlate(struct irq_domain *d,
 static int combiner_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				   irq_hw_number_t hw)
 {
+	struct combiner_chip_data *combiner_data = d->host_data;
+
 	irq_set_chip_and_handler(irq, &combiner_chip, handle_level_irq);
 	irq_set_chip_data(irq, &combiner_data[hw >> 3]);
 	set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
@@ -202,6 +204,7 @@ void __init combiner_init(void __iomem *combiner_base,
 {
 	int i, irq, irq_base;
 	unsigned int nr_irq;
+	struct combiner_chip_data *combiner_data;
 
 	nr_irq = max_nr * IRQ_IN_COMBINER;
 
@@ -211,8 +214,14 @@ void __init combiner_init(void __iomem *combiner_base,
 		pr_warning("%s: irq desc alloc failed. Continuing with %d as linux irq base\n", __func__, irq_base);
 	}
 
+	combiner_data = kcalloc(max_nr, sizeof (*combiner_data), GFP_KERNEL);
+	if (!combiner_data) {
+		pr_warning("%s: could not allocate combiner data\n", __func__);
+		return;
+	}
+
 	combiner_irq_domain = irq_domain_add_legacy(np, nr_irq, irq_base, 0,
-				&combiner_irq_domain_ops, &combiner_data);
+				&combiner_irq_domain_ops, combiner_data);
 	if (WARN_ON(!combiner_irq_domain)) {
 		pr_warning("%s: irq domain init failed\n", __func__);
 		return;
@@ -227,8 +236,9 @@ void __init combiner_init(void __iomem *combiner_base,
 		if (np)
 			irq = irq_of_parse_and_map(np, i);
 #endif
-		combiner_init_one(i, combiner_base + (i >> 2) * 0x10, irq);
-		combiner_cascade_irq(i, irq);
+		combiner_init_one(&combiner_data[i], i,
+				  combiner_base + (i >> 2) * 0x10, irq);
+		combiner_cascade_irq(&combiner_data[i], irq);
 	}
 }
 
-- 
1.8.1.2

[PATCH 6/7] irqchip: exynos: localize irq lookup for ATAGS

[Arnd Bergmann]

The IRQ_SPI() macro is not available in the driver when building with sparse
IRQs or multiplatform, so let's move all users of this into one function
that we can leave out when building DT-only.

Signed-off-by: Arnd Bergmann <arnd@arnd.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/exynos-combiner.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index 7fcdeee..acb9c74 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -18,7 +18,9 @@
 #include <linux/of_irq.h>
 #include <asm/mach/irq.h>
 
+#ifdef CONFIG_EXYNOS_ATAGS
 #include <plat/cpu.h>
+#endif
 
 #include "irqchip.h"
 
@@ -182,8 +184,12 @@ static struct irq_domain_ops combiner_irq_domain_ops = {
 	.map	= combiner_irq_domain_map,
 };
 
-static unsigned int exynos4x12_combiner_extra_irq(int group)
+static unsigned int combiner_lookup_irq(int group)
 {
+#ifdef CONFIG_EXYNOS_ATAGS
+	if (group < EXYNOS4210_MAX_COMBINER_NR || soc_is_exynos5250())
+		return IRQ_SPI(group);
+
 	switch (group) {
 	case 16:
 		return IRQ_SPI(107);
@@ -193,9 +199,9 @@ static unsigned int exynos4x12_combiner_extra_irq(int group)
 		return IRQ_SPI(48);
 	case 19:
 		return IRQ_SPI(42);
-	default:
-		return 0;
 	}
+#endif
+	return 0;
 }
 
 void __init combiner_init(void __iomem *combiner_base,
@@ -228,14 +234,13 @@ void __init combiner_init(void __iomem *combiner_base,
 	}
 
 	for (i = 0; i < max_nr; i++) {
-		if (i < EXYNOS4210_MAX_COMBINER_NR || soc_is_exynos5250())
-			irq = IRQ_SPI(i);
-		else
-			irq = exynos4x12_combiner_extra_irq(i);
 #ifdef CONFIG_OF
 		if (np)
 			irq = irq_of_parse_and_map(np, i);
+		else
 #endif
+			irq = combiner_lookup_irq(i);
+
 		combiner_init_one(&combiner_data[i], i,
 				  combiner_base + (i >> 2) * 0x10, irq);
 		combiner_cascade_irq(&combiner_data[i], irq);
-- 
1.8.1.2

[PATCH 6/7] irqchip: exynos: localize irq lookup for ATAGS

[Arnd Bergmann]

The IRQ_SPI() macro is not available in the driver when building with sparse
IRQs or multiplatform, so let's move all users of this into one function
that we can leave out when building DT-only.

Signed-off-by: Arnd Bergmann <arnd@arnd.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/irqchip/exynos-combiner.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index 7fcdeee..acb9c74 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -18,7 +18,9 @@
 #include <linux/of_irq.h>
 #include <asm/mach/irq.h>
 
+#ifdef CONFIG_EXYNOS_ATAGS
 #include <plat/cpu.h>
+#endif
 
 #include "irqchip.h"
 
@@ -182,8 +184,12 @@ static struct irq_domain_ops combiner_irq_domain_ops = {
 	.map	= combiner_irq_domain_map,
 };
 
-static unsigned int exynos4x12_combiner_extra_irq(int group)
+static unsigned int combiner_lookup_irq(int group)
 {
+#ifdef CONFIG_EXYNOS_ATAGS
+	if (group < EXYNOS4210_MAX_COMBINER_NR || soc_is_exynos5250())
+		return IRQ_SPI(group);
+
 	switch (group) {
 	case 16:
 		return IRQ_SPI(107);
@@ -193,9 +199,9 @@ static unsigned int exynos4x12_combiner_extra_irq(int group)
 		return IRQ_SPI(48);
 	case 19:
 		return IRQ_SPI(42);
-	default:
-		return 0;
 	}
+#endif
+	return 0;
 }
 
 void __init combiner_init(void __iomem *combiner_base,
@@ -228,14 +234,13 @@ void __init combiner_init(void __iomem *combiner_base,
 	}
 
 	for (i = 0; i < max_nr; i++) {
-		if (i < EXYNOS4210_MAX_COMBINER_NR || soc_is_exynos5250())
-			irq = IRQ_SPI(i);
-		else
-			irq = exynos4x12_combiner_extra_irq(i);
 #ifdef CONFIG_OF
 		if (np)
 			irq = irq_of_parse_and_map(np, i);
+		else
 #endif
+			irq = combiner_lookup_irq(i);
+
 		combiner_init_one(&combiner_data[i], i,
 				  combiner_base + (i >> 2) * 0x10, irq);
 		combiner_cascade_irq(&combiner_data[i], irq);
-- 
1.8.1.2

[PATCH 7/7] irqchip: exynos: pass irq_base from platform

[Arnd Bergmann]

The platform code knows the IRQ base, while the irqchip driver
should really not. This is a littly hacky because we still
hardwire the IRQ base to 160 for the combiner in the DT case,
when we should really use -1. Removing that line will cause
a linear IRQ domain to be use, as we should.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-exynos/common.c     |  3 ++-
 arch/arm/mach-exynos/common.h     |  2 +-
 drivers/irqchip/exynos-combiner.c | 23 +++++++++++++----------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index ecdd212..12f43fc 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -444,7 +444,8 @@ void __init exynos4_init_irq(void)
 #endif
 
 	if (!of_have_populated_dt())
-		combiner_init(S5P_VA_COMBINER_BASE, NULL, max_combiner_nr());
+		combiner_init(S5P_VA_COMBINER_BASE, NULL,
+			      max_combiner_nr(), COMBINER_IRQ(0, 0));
 
 	/*
 	 * The parameters of s5p_init_irq() are for VIC init.
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 4ba8cbe..3e72d03 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -70,7 +70,7 @@ void exynos4212_register_clocks(void);
 
 struct device_node;
 void combiner_init(void __iomem *combiner_base, struct device_node *np,
-			unsigned int max_nr);
+			unsigned int max_nr, int irq_base);
 
 extern struct smp_operations exynos_smp_ops;
 
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index acb9c74..6855c92 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -206,27 +206,22 @@ static unsigned int combiner_lookup_irq(int group)
 
 void __init combiner_init(void __iomem *combiner_base,
 			  struct device_node *np,
-			  unsigned int max_nr)
+			  unsigned int max_nr,
+			  int irq_base)
 {
-	int i, irq, irq_base;
+	int i, irq;
 	unsigned int nr_irq;
 	struct combiner_chip_data *combiner_data;
 
 	nr_irq = max_nr * IRQ_IN_COMBINER;
 
-	irq_base = irq_alloc_descs(COMBINER_IRQ(0, 0), 1, nr_irq, 0);
-	if (IS_ERR_VALUE(irq_base)) {
-		irq_base = COMBINER_IRQ(0, 0);
-		pr_warning("%s: irq desc alloc failed. Continuing with %d as linux irq base\n", __func__, irq_base);
-	}
-
 	combiner_data = kcalloc(max_nr, sizeof (*combiner_data), GFP_KERNEL);
 	if (!combiner_data) {
 		pr_warning("%s: could not allocate combiner data\n", __func__);
 		return;
 	}
 
-	combiner_irq_domain = irq_domain_add_legacy(np, nr_irq, irq_base, 0,
+	combiner_irq_domain = irq_domain_add_simple(np, nr_irq, irq_base,
 				&combiner_irq_domain_ops, combiner_data);
 	if (WARN_ON(!combiner_irq_domain)) {
 		pr_warning("%s: irq domain init failed\n", __func__);
@@ -253,6 +248,7 @@ static int __init combiner_of_init(struct device_node *np,
 {
 	void __iomem *combiner_base;
 	unsigned int max_nr = 20;
+	int irq_base = -1;
 
 	combiner_base = of_iomap(np, 0);
 	if (!combiner_base) {
@@ -266,7 +262,14 @@ static int __init combiner_of_init(struct device_node *np,
 			__func__, max_nr);
 	}
 
-	combiner_init(combiner_base, np, max_nr);
+	/* 
+	 * FIXME: This is a hardwired COMBINER_IRQ(0,0). Once all devices
+	 * get their IRQ from DT, remove this in order to get dynamic
+	 * allocation.
+	 */
+	irq_base = 160;
+
+	combiner_init(combiner_base, np, max_nr, irq_base);
 
 	return 0;
 }
-- 
1.8.1.2

[PATCH 7/7] irqchip: exynos: pass irq_base from platform

[Arnd Bergmann]

The platform code knows the IRQ base, while the irqchip driver
should really not. This is a littly hacky because we still
hardwire the IRQ base to 160 for the combiner in the DT case,
when we should really use -1. Removing that line will cause
a linear IRQ domain to be use, as we should.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 arch/arm/mach-exynos/common.c     |  3 ++-
 arch/arm/mach-exynos/common.h     |  2 +-
 drivers/irqchip/exynos-combiner.c | 23 +++++++++++++----------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index ecdd212..12f43fc 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -444,7 +444,8 @@ void __init exynos4_init_irq(void)
 #endif
 
 	if (!of_have_populated_dt())
-		combiner_init(S5P_VA_COMBINER_BASE, NULL, max_combiner_nr());
+		combiner_init(S5P_VA_COMBINER_BASE, NULL,
+			      max_combiner_nr(), COMBINER_IRQ(0, 0));
 
 	/*
 	 * The parameters of s5p_init_irq() are for VIC init.
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index 4ba8cbe..3e72d03 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -70,7 +70,7 @@ void exynos4212_register_clocks(void);
 
 struct device_node;
 void combiner_init(void __iomem *combiner_base, struct device_node *np,
-			unsigned int max_nr);
+			unsigned int max_nr, int irq_base);
 
 extern struct smp_operations exynos_smp_ops;
 
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index acb9c74..6855c92 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -206,27 +206,22 @@ static unsigned int combiner_lookup_irq(int group)
 
 void __init combiner_init(void __iomem *combiner_base,
 			  struct device_node *np,
-			  unsigned int max_nr)
+			  unsigned int max_nr,
+			  int irq_base)
 {
-	int i, irq, irq_base;
+	int i, irq;
 	unsigned int nr_irq;
 	struct combiner_chip_data *combiner_data;
 
 	nr_irq = max_nr * IRQ_IN_COMBINER;
 
-	irq_base = irq_alloc_descs(COMBINER_IRQ(0, 0), 1, nr_irq, 0);
-	if (IS_ERR_VALUE(irq_base)) {
-		irq_base = COMBINER_IRQ(0, 0);
-		pr_warning("%s: irq desc alloc failed. Continuing with %d as linux irq base\n", __func__, irq_base);
-	}
-
 	combiner_data = kcalloc(max_nr, sizeof (*combiner_data), GFP_KERNEL);
 	if (!combiner_data) {
 		pr_warning("%s: could not allocate combiner data\n", __func__);
 		return;
 	}
 
-	combiner_irq_domain = irq_domain_add_legacy(np, nr_irq, irq_base, 0,
+	combiner_irq_domain = irq_domain_add_simple(np, nr_irq, irq_base,
 				&combiner_irq_domain_ops, combiner_data);
 	if (WARN_ON(!combiner_irq_domain)) {
 		pr_warning("%s: irq domain init failed\n", __func__);
@@ -253,6 +248,7 @@ static int __init combiner_of_init(struct device_node *np,
 {
 	void __iomem *combiner_base;
 	unsigned int max_nr = 20;
+	int irq_base = -1;
 
 	combiner_base = of_iomap(np, 0);
 	if (!combiner_base) {
@@ -266,7 +262,14 @@ static int __init combiner_of_init(struct device_node *np,
 			__func__, max_nr);
 	}
 
-	combiner_init(combiner_base, np, max_nr);
+	/* 
+	 * FIXME: This is a hardwired COMBINER_IRQ(0,0). Once all devices
+	 * get their IRQ from DT, remove this in order to get dynamic
+	 * allocation.
+	 */
+	irq_base = 160;
+
+	combiner_init(combiner_base, np, max_nr, irq_base);
 
 	return 0;
 }
-- 
1.8.1.2