[PATCH 0/4] ARM: OMAP2+: Interrupt controllers adaptation to DT

[PATCH 0/4] ARM: OMAP2+: Interrupt controllers adaptation to DT

[Benoit Cousson]

Hi Tony and Rob,

Here is the series to take advantage of the new DT interrupt init mechanism.
Thanks to Mark's CONFIG_MULTI_IRQ_HANDLER series, OMAP4 just has to
use the default GIC binding and does not need some OMAP specific hacks
anymore.
OMAP2 and 3 are using a simple interrupt controller that can thus expose
a simpler binding.

This series is based on rmk/devel-stable branch to get Mark's series +
a couple of OMAP fixes.

An extra fix is needed before for the board-generic.c file: 
[PATCH] ARM: OMAP2+: board-generic: Add missing handle_irq

The series with all the dependencies merged is available here for reference:
git://gitorious.org/omap-pm/linux.git for_3.3/2_dt_irq

Regards,
Benoit


Benoit Cousson (4):
  arm/dts: OMAP4: Update DTS file with new GIC bindings
  ARM: OMAP2/3: intc: Add DT support for TI interrupt controller
  arm/dts: OMAP3: Add interrupt-controller bindings for INTC
  ARM: OMAP2+: board-generic: Use of_irq_init API

 .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
 arch/arm/boot/dts/omap3.dtsi                       |    6 ++-
 arch/arm/boot/dts/omap4.dtsi                       |    3 +-
 arch/arm/mach-omap2/board-generic.c                |   30 +++++++++--------
 arch/arm/mach-omap2/common.h                       |   10 ++++++
 arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
 6 files changed, 91 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt

[PATCH 0/4] ARM: OMAP2+: Interrupt controllers adaptation to DT

[Benoit Cousson]

Hi Tony and Rob,

Here is the series to take advantage of the new DT interrupt init mechanism.
Thanks to Mark's CONFIG_MULTI_IRQ_HANDLER series, OMAP4 just has to
use the default GIC binding and does not need some OMAP specific hacks
anymore.
OMAP2 and 3 are using a simple interrupt controller that can thus expose
a simpler binding.

This series is based on rmk/devel-stable branch to get Mark's series +
a couple of OMAP fixes.

An extra fix is needed before for the board-generic.c file: 
[PATCH] ARM: OMAP2+: board-generic: Add missing handle_irq

The series with all the dependencies merged is available here for reference:
git://gitorious.org/omap-pm/linux.git for_3.3/2_dt_irq

Regards,
Benoit


Benoit Cousson (4):
  arm/dts: OMAP4: Update DTS file with new GIC bindings
  ARM: OMAP2/3: intc: Add DT support for TI interrupt controller
  arm/dts: OMAP3: Add interrupt-controller bindings for INTC
  ARM: OMAP2+: board-generic: Use of_irq_init API

 .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
 arch/arm/boot/dts/omap3.dtsi                       |    6 ++-
 arch/arm/boot/dts/omap4.dtsi                       |    3 +-
 arch/arm/mach-omap2/board-generic.c                |   30 +++++++++--------
 arch/arm/mach-omap2/common.h                       |   10 ++++++
 arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
 6 files changed, 91 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt

[PATCH 1/4] arm/dts: OMAP4: Update DTS file with new GIC bindings

[Benoit Cousson]

The GIC binding was updated in 3.2 and expect 3 interrupt-cells.
- Update the #interrupt-cells
- interrupt-parent seems to be needed as well for the top level GIC

Signed-off-by: Benoit Cousson <b-cousson-l0cyMroinI0@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/boot/dts/omap4.dtsi |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 4c61c82..bede009 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -95,7 +95,8 @@
 		gic: interrupt-controller@48241000 {
 			compatible = "arm,cortex-a9-gic";
 			interrupt-controller;
-			#interrupt-cells = <1>;
+			interrupt-parent;
+			#interrupt-cells = <3>;
 			reg = <0x48241000 0x1000>,
 			      <0x48240100 0x0100>;
 		};
-- 
1.7.0.4

[PATCH 1/4] arm/dts: OMAP4: Update DTS file with new GIC bindings

[Benoit Cousson]

The GIC binding was updated in 3.2 and expect 3 interrupt-cells.
- Update the #interrupt-cells
- interrupt-parent seems to be needed as well for the top level GIC

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/boot/dts/omap4.dtsi |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 4c61c82..bede009 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -95,7 +95,8 @@
 		gic: interrupt-controller at 48241000 {
 			compatible = "arm,cortex-a9-gic";
 			interrupt-controller;
-			#interrupt-cells = <1>;
+			interrupt-parent;
+			#interrupt-cells = <3>;
 			reg = <0x48241000 0x1000>,
 			      <0x48240100 0x0100>;
 		};
-- 
1.7.0.4

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Benoit Cousson]

Add a function to initialize the OMAP2/3 interrupt controller (INTC)
using a device tree node.

Replace some printk() with the proper pr_ macro.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
 arch/arm/mach-omap2/common.h                       |   10 ++++++
 arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
 3 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/intc.txt b/Documentation/devicetree/bindings/arm/omap/intc.txt
new file mode 100644
index 0000000..f2583e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/intc.txt
@@ -0,0 +1,27 @@
+* OMAP Interrupt Controller
+
+OMAP2/3 are using a TI interrupt controller that can support several
+configurable number of interrupts.
+
+Main node required properties:
+
+- compatible : should be:
+	"ti,omap2-intc"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source. The type shall be a <u32> and the value shall be 1.
+
+  The cell contains the interrupt number in the range [0-128].
+- ti,intc-size: Number of interrupts handled by the interrupt controller.
+- reg: physical base address and size of the intc registers map.
+
+Example:
+
+	intc: interrupt-controller@1 {
+		compatible = "ti,omap2-intc";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		ti,intc-size = <96>;
+		reg = <0x48200000 0x1000>;
+	};
+
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 012bac7..bcfccc2 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
 void omap2_intc_handle_irq(struct pt_regs *regs);
 void omap3_intc_handle_irq(struct pt_regs *regs);
 
+struct device_node;
+#ifdef CONFIG_OF
+int __init intc_of_init(struct device_node *node, struct device_node *parent);
+#else
+int __init intc_of_init(struct device_node *node, struct device_node *parent)
+{
+	return 0;
+}
+#endif
+
 /*
  * wfi used in low power code. Directly opcode is used instead
  * of instruction to avoid mulit-omap build break
diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 42b1d65..cafc663 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -17,6 +17,9 @@
 #include <mach/hardware.h>
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/irqdomain.h>
 
 
 /* selected INTC register offsets */
@@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 		/* Static mapping, never released */
 		bank->base_reg = ioremap(base, SZ_4K);
 		if (!bank->base_reg) {
-			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
+			pr_err("Could not ioremap irq bank%i\n", i);
 			continue;
 		}
 
@@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 		nr_banks++;
 	}
 
-	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
-	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
+	pr_info("Total of %ld interrupts on %d active controller%s\n",
+		nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
 }
 
 void __init omap2_init_irq(void)
@@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
 	omap_intc_handle_irq(base_addr, regs);
 }
 
+#ifdef CONFIG_OF
+int __init intc_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct resource res;
+	u32 nr_irqs;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	if (of_address_to_resource(node, 0, &res)) {
+		WARN(1, "unable to get intc registers\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "ti,intc-size", &nr_irqs)) {
+		WARN(1, "unable to get intc-size\n");
+		return -EINVAL;
+	}
+
+	omap_init_irq(res.start, nr_irqs);
+	irq_domain_add_simple(node, 0);
+
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_ARCH_OMAP3
 static struct omap3_intc_regs intc_context[ARRAY_SIZE(irq_banks)];
 
-- 
1.7.0.4

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Benoit Cousson]

Add a function to initialize the OMAP2/3 interrupt controller (INTC)
using a device tree node.

Replace some printk() with the proper pr_ macro.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
 arch/arm/mach-omap2/common.h                       |   10 ++++++
 arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
 3 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt

diff --git a/Documentation/devicetree/bindings/arm/omap/intc.txt b/Documentation/devicetree/bindings/arm/omap/intc.txt
new file mode 100644
index 0000000..f2583e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/omap/intc.txt
@@ -0,0 +1,27 @@
+* OMAP Interrupt Controller
+
+OMAP2/3 are using a TI interrupt controller that can support several
+configurable number of interrupts.
+
+Main node required properties:
+
+- compatible : should be:
+	"ti,omap2-intc"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source. The type shall be a <u32> and the value shall be 1.
+
+  The cell contains the interrupt number in the range [0-128].
+- ti,intc-size: Number of interrupts handled by the interrupt controller.
+- reg: physical base address and size of the intc registers map.
+
+Example:
+
+	intc: interrupt-controller at 1 {
+		compatible = "ti,omap2-intc";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		ti,intc-size = <96>;
+		reg = <0x48200000 0x1000>;
+	};
+
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index 012bac7..bcfccc2 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
 void omap2_intc_handle_irq(struct pt_regs *regs);
 void omap3_intc_handle_irq(struct pt_regs *regs);
 
+struct device_node;
+#ifdef CONFIG_OF
+int __init intc_of_init(struct device_node *node, struct device_node *parent);
+#else
+int __init intc_of_init(struct device_node *node, struct device_node *parent)
+{
+	return 0;
+}
+#endif
+
 /*
  * wfi used in low power code. Directly opcode is used instead
  * of instruction to avoid mulit-omap build break
diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 42b1d65..cafc663 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -17,6 +17,9 @@
 #include <mach/hardware.h>
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/irqdomain.h>
 
 
 /* selected INTC register offsets */
@@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 		/* Static mapping, never released */
 		bank->base_reg = ioremap(base, SZ_4K);
 		if (!bank->base_reg) {
-			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
+			pr_err("Could not ioremap irq bank%i\n", i);
 			continue;
 		}
 
@@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 		nr_banks++;
 	}
 
-	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
-	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
+	pr_info("Total of %ld interrupts on %d active controller%s\n",
+		nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
 }
 
 void __init omap2_init_irq(void)
@@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
 	omap_intc_handle_irq(base_addr, regs);
 }
 
+#ifdef CONFIG_OF
+int __init intc_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct resource res;
+	u32 nr_irqs;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	if (of_address_to_resource(node, 0, &res)) {
+		WARN(1, "unable to get intc registers\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "ti,intc-size", &nr_irqs)) {
+		WARN(1, "unable to get intc-size\n");
+		return -EINVAL;
+	}
+
+	omap_init_irq(res.start, nr_irqs);
+	irq_domain_add_simple(node, 0);
+
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_ARCH_OMAP3
 static struct omap3_intc_regs intc_context[ARRAY_SIZE(irq_banks)];
 
-- 
1.7.0.4

[PATCH 3/4] arm/dts: OMAP3: Add interrupt-controller bindings for INTC

[Benoit Cousson]

Update the DTS with the proper information required by the
INTC bindings.

- Add the number of interrupt lines
- Add the reg and the compatible entries.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/boot/dts/omap3.dtsi |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index d202bb5..6866dc7 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -54,10 +54,12 @@
 		ranges;
 		ti,hwmods = "l3_main";
 
-		intc: interrupt-controller@1 {
-			compatible = "ti,omap3-intc";
+		intc: interrupt-controller@48200000 {
+			compatible = "ti,omap2-intc";
 			interrupt-controller;
 			#interrupt-cells = <1>;
+			ti,intc-size = <96>;
+			reg = <0x48200000 0x1000>;
 		};
 	};
 };
-- 
1.7.0.4

[PATCH 3/4] arm/dts: OMAP3: Add interrupt-controller bindings for INTC

[Benoit Cousson]

Update the DTS with the proper information required by the
INTC bindings.

- Add the number of interrupt lines
- Add the reg and the compatible entries.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/boot/dts/omap3.dtsi |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index d202bb5..6866dc7 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -54,10 +54,12 @@
 		ranges;
 		ti,hwmods = "l3_main";
 
-		intc: interrupt-controller at 1 {
-			compatible = "ti,omap3-intc";
+		intc: interrupt-controller at 48200000 {
+			compatible = "ti,omap2-intc";
 			interrupt-controller;
 			#interrupt-cells = <1>;
+			ti,intc-size = <96>;
+			reg = <0x48200000 0x1000>;
 		};
 	};
 };
-- 
1.7.0.4

[PATCH 4/4] ARM: OMAP2+: board-generic: Use of_irq_init API

[Benoit Cousson]

Use the of_irq_init API introduced in 3.2 to handle
interrupt-controller with DT.
Update the irq_match table to map the proper XXX_of_init
functions for INTC and GIC drivers.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/mach-omap2/board-generic.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index a0e9595..16c301e 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -12,6 +12,7 @@
  * published by the Free Software Foundation.
  */
 #include <linux/io.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>
 #include <linux/i2c/twl.h>
@@ -24,6 +25,17 @@
 #include "common.h"
 #include "common-board-devices.h"
 
+static struct of_device_id irq_match[] __initdata = {
+	{ .compatible = "ti,omap2-intc", .data = intc_of_init, },
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+	{ }
+};
+
+static void __init omap_init_irq(void)
+{
+	of_irq_init(irq_match);
+}
+
 /*
  * XXX: Still needed to boot until the i2c & twl driver is adapted to
  * device-tree
@@ -58,18 +70,8 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
 	{ }
 };
 
-static struct of_device_id intc_match[] __initdata = {
-	{ .compatible = "ti,omap3-intc", },
-	{ .compatible = "arm,cortex-a9-gic", },
-	{ }
-};
-
 static void __init omap_generic_init(void)
 {
-	struct device_node *node = of_find_matching_node(NULL, intc_match);
-	if (node)
-		irq_domain_add_simple(node, 0);
-
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 
@@ -103,7 +105,7 @@ DT_MACHINE_START(OMAP242X_DT, "Generic OMAP2420 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap242x_map_io,
 	.init_early	= omap2420_init_early,
-	.init_irq	= omap2_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap2_intc_handle_irq,
 	.init_machine	= omap_generic_init,
 	.timer		= &omap2_timer,
@@ -122,7 +124,7 @@ DT_MACHINE_START(OMAP243X_DT, "Generic OMAP2430 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap243x_map_io,
 	.init_early	= omap2430_init_early,
-	.init_irq	= omap2_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap2_intc_handle_irq,
 	.init_machine	= omap_generic_init,
 	.timer		= &omap2_timer,
@@ -141,7 +143,7 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap3_map_io,
 	.init_early	= omap3430_init_early,
-	.init_irq	= omap3_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap3_intc_handle_irq,
 	.init_machine	= omap3_init,
 	.timer		= &omap3_timer,
@@ -160,7 +162,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap4_map_io,
 	.init_early	= omap4430_init_early,
-	.init_irq	= gic_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= gic_handle_irq,
 	.init_machine	= omap4_init,
 	.timer		= &omap4_timer,
-- 
1.7.0.4

[PATCH 4/4] ARM: OMAP2+: board-generic: Use of_irq_init API

[Benoit Cousson]

Use the of_irq_init API introduced in 3.2 to handle
interrupt-controller with DT.
Update the irq_match table to map the proper XXX_of_init
functions for INTC and GIC drivers.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/mach-omap2/board-generic.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index a0e9595..16c301e 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -12,6 +12,7 @@
  * published by the Free Software Foundation.
  */
 #include <linux/io.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/irqdomain.h>
 #include <linux/i2c/twl.h>
@@ -24,6 +25,17 @@
 #include "common.h"
 #include "common-board-devices.h"
 
+static struct of_device_id irq_match[] __initdata = {
+	{ .compatible = "ti,omap2-intc", .data = intc_of_init, },
+	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+	{ }
+};
+
+static void __init omap_init_irq(void)
+{
+	of_irq_init(irq_match);
+}
+
 /*
  * XXX: Still needed to boot until the i2c & twl driver is adapted to
  * device-tree
@@ -58,18 +70,8 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
 	{ }
 };
 
-static struct of_device_id intc_match[] __initdata = {
-	{ .compatible = "ti,omap3-intc", },
-	{ .compatible = "arm,cortex-a9-gic", },
-	{ }
-};
-
 static void __init omap_generic_init(void)
 {
-	struct device_node *node = of_find_matching_node(NULL, intc_match);
-	if (node)
-		irq_domain_add_simple(node, 0);
-
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 
@@ -103,7 +105,7 @@ DT_MACHINE_START(OMAP242X_DT, "Generic OMAP2420 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap242x_map_io,
 	.init_early	= omap2420_init_early,
-	.init_irq	= omap2_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap2_intc_handle_irq,
 	.init_machine	= omap_generic_init,
 	.timer		= &omap2_timer,
@@ -122,7 +124,7 @@ DT_MACHINE_START(OMAP243X_DT, "Generic OMAP2430 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap243x_map_io,
 	.init_early	= omap2430_init_early,
-	.init_irq	= omap2_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap2_intc_handle_irq,
 	.init_machine	= omap_generic_init,
 	.timer		= &omap2_timer,
@@ -141,7 +143,7 @@ DT_MACHINE_START(OMAP3_DT, "Generic OMAP3 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap3_map_io,
 	.init_early	= omap3430_init_early,
-	.init_irq	= omap3_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= omap3_intc_handle_irq,
 	.init_machine	= omap3_init,
 	.timer		= &omap3_timer,
@@ -160,7 +162,7 @@ DT_MACHINE_START(OMAP4_DT, "Generic OMAP4 (Flattened Device Tree)")
 	.reserve	= omap_reserve,
 	.map_io		= omap4_map_io,
 	.init_early	= omap4430_init_early,
-	.init_irq	= gic_init_irq,
+	.init_irq	= omap_init_irq,
 	.handle_irq	= gic_handle_irq,
 	.init_machine	= omap4_init,
 	.timer		= &omap4_timer,
-- 
1.7.0.4

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/07/2011 02:50 PM, Benoit Cousson wrote:
> Add a function to initialize the OMAP2/3 interrupt controller (INTC)
> using a device tree node.
> 
> Replace some printk() with the proper pr_ macro.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
>  .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
>  arch/arm/mach-omap2/common.h                       |   10 ++++++
>  arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
>  3 files changed, 69 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/intc.txt b/Documentation/devicetree/bindings/arm/omap/intc.txt
> new file mode 100644
> index 0000000..f2583e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/intc.txt
> @@ -0,0 +1,27 @@
> +* OMAP Interrupt Controller
> +
> +OMAP2/3 are using a TI interrupt controller that can support several
> +configurable number of interrupts.
> +
> +Main node required properties:
> +
> +- compatible : should be:
> +	"ti,omap2-intc"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source. The type shall be a <u32> and the value shall be 1.
> +
> +  The cell contains the interrupt number in the range [0-128].
> +- ti,intc-size: Number of interrupts handled by the interrupt controller.
> +- reg: physical base address and size of the intc registers map.
> +
> +Example:
> +
> +	intc: interrupt-controller@1 {
> +		compatible = "ti,omap2-intc";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		ti,intc-size = <96>;
> +		reg = <0x48200000 0x1000>;
> +	};
> +
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 012bac7..bcfccc2 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
>  void omap2_intc_handle_irq(struct pt_regs *regs);
>  void omap3_intc_handle_irq(struct pt_regs *regs);
>  
> +struct device_node;
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent);
> +#else
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * wfi used in low power code. Directly opcode is used instead
>   * of instruction to avoid mulit-omap build break
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..cafc663 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,9 @@
>  #include <mach/hardware.h>
>  #include <asm/exception.h>
>  #include <asm/mach/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/irqdomain.h>
>  
>  
>  /* selected INTC register offsets */
> @@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  		/* Static mapping, never released */
>  		bank->base_reg = ioremap(base, SZ_4K);
>  		if (!bank->base_reg) {
> -			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
> +			pr_err("Could not ioremap irq bank%i\n", i);
>  			continue;
>  		}
>  
> @@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  		nr_banks++;
>  	}
>  
> -	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
> -	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
> +	pr_info("Total of %ld interrupts on %d active controller%s\n",
> +		nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
>  }
>  
>  void __init omap2_init_irq(void)
> @@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
>  	omap_intc_handle_irq(base_addr, regs);
>  }
>  
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct resource res;
> +	u32 nr_irqs;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_address_to_resource(node, 0, &res)) {
> +		WARN(1, "unable to get intc registers\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "ti,intc-size", &nr_irqs)) {
> +		WARN(1, "unable to get intc-size\n");
> +		return -EINVAL;

There is no default value that makes sense?

> +	}
> +
> +	omap_init_irq(res.start, nr_irqs);
> +	irq_domain_add_simple(node, 0);

Have you read the NO_IRQ thread...

Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
the base. I would pick 16 to skip over legacy ISA irqs.

irqdomains should always be enabled regardless of CONFIG_OF. So either
you can leave it as is if OF is always enabled for OMAP, or you should
move domain setup into omap_init_irq.

Rob

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/07/2011 02:50 PM, Benoit Cousson wrote:
> Add a function to initialize the OMAP2/3 interrupt controller (INTC)
> using a device tree node.
> 
> Replace some printk() with the proper pr_ macro.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> ---
>  .../devicetree/bindings/arm/omap/intc.txt          |   27 +++++++++++++++
>  arch/arm/mach-omap2/common.h                       |   10 ++++++
>  arch/arm/mach-omap2/irq.c                          |   35 ++++++++++++++++++--
>  3 files changed, 69 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/omap/intc.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/omap/intc.txt b/Documentation/devicetree/bindings/arm/omap/intc.txt
> new file mode 100644
> index 0000000..f2583e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/omap/intc.txt
> @@ -0,0 +1,27 @@
> +* OMAP Interrupt Controller
> +
> +OMAP2/3 are using a TI interrupt controller that can support several
> +configurable number of interrupts.
> +
> +Main node required properties:
> +
> +- compatible : should be:
> +	"ti,omap2-intc"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source. The type shall be a <u32> and the value shall be 1.
> +
> +  The cell contains the interrupt number in the range [0-128].
> +- ti,intc-size: Number of interrupts handled by the interrupt controller.
> +- reg: physical base address and size of the intc registers map.
> +
> +Example:
> +
> +	intc: interrupt-controller at 1 {
> +		compatible = "ti,omap2-intc";
> +		interrupt-controller;
> +		#interrupt-cells = <1>;
> +		ti,intc-size = <96>;
> +		reg = <0x48200000 0x1000>;
> +	};
> +
> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 012bac7..bcfccc2 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
>  void omap2_intc_handle_irq(struct pt_regs *regs);
>  void omap3_intc_handle_irq(struct pt_regs *regs);
>  
> +struct device_node;
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent);
> +#else
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * wfi used in low power code. Directly opcode is used instead
>   * of instruction to avoid mulit-omap build break
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..cafc663 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,9 @@
>  #include <mach/hardware.h>
>  #include <asm/exception.h>
>  #include <asm/mach/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/irqdomain.h>
>  
>  
>  /* selected INTC register offsets */
> @@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  		/* Static mapping, never released */
>  		bank->base_reg = ioremap(base, SZ_4K);
>  		if (!bank->base_reg) {
> -			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
> +			pr_err("Could not ioremap irq bank%i\n", i);
>  			continue;
>  		}
>  
> @@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  		nr_banks++;
>  	}
>  
> -	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
> -	       nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
> +	pr_info("Total of %ld interrupts on %d active controller%s\n",
> +		nr_of_irqs, nr_banks, nr_banks > 1 ? "s" : "");
>  }
>  
>  void __init omap2_init_irq(void)
> @@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
>  	omap_intc_handle_irq(base_addr, regs);
>  }
>  
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct resource res;
> +	u32 nr_irqs;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_address_to_resource(node, 0, &res)) {
> +		WARN(1, "unable to get intc registers\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "ti,intc-size", &nr_irqs)) {
> +		WARN(1, "unable to get intc-size\n");
> +		return -EINVAL;

There is no default value that makes sense?

> +	}
> +
> +	omap_init_irq(res.start, nr_irqs);
> +	irq_domain_add_simple(node, 0);

Have you read the NO_IRQ thread...

Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
the base. I would pick 16 to skip over legacy ISA irqs.

irqdomains should always be enabled regardless of CONFIG_OF. So either
you can leave it as is if OF is always enabled for OMAP, or you should
move domain setup into omap_init_irq.

Rob

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/7/2011 10:20 PM, Rob Herring wrote:
> On 12/07/2011 02:50 PM, Benoit Cousson wrote:

[...]

>> +#ifdef CONFIG_OF
>> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
>> +{
>> +	struct resource res;
>> +	u32 nr_irqs;
>> +
>> +	if (WARN_ON(!node))
>> +		return -ENODEV;
>> +
>> +	if (of_address_to_resource(node, 0,&res)) {
>> +		WARN(1, "unable to get intc registers\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>> +		WARN(1, "unable to get intc-size\n");
>> +		return -EINVAL;
>
> There is no default value that makes sense?

So far we have 96 or 128, so I can put 96 as a default and just keep the 
warning without returning an error.

>> +	}
>> +
>> +	omap_init_irq(res.start, nr_irqs);
>> +	irq_domain_add_simple(node, 0);
>
> Have you read the NO_IRQ thread...

Yeah, I tried, but that's a long email thread with some unclear 
conclusion...
The point is that the few users of that API today are using 0 as a base 
as well, so I thought it was still valid.

> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
> the base. I would pick 16 to skip over legacy ISA irqs.

I do not have any 0 interrupts right now, my concern is that I still 
have some legacy non-DT drivers that expect the previous hwirq = irq 
mapping.

> irqdomains should always be enabled regardless of CONFIG_OF. So either
> you can leave it as is if OF is always enabled for OMAP, or you should
> move domain setup into omap_init_irq.

OK, but it looks like I cannot really modify the current INTC to DT 
without having fully adapted the OMAP drivers to use 
irq_of_parse_and_map. Or did I miss something?

Regards,
Benoit

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/7/2011 10:20 PM, Rob Herring wrote:
> On 12/07/2011 02:50 PM, Benoit Cousson wrote:

[...]

>> +#ifdef CONFIG_OF
>> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
>> +{
>> +	struct resource res;
>> +	u32 nr_irqs;
>> +
>> +	if (WARN_ON(!node))
>> +		return -ENODEV;
>> +
>> +	if (of_address_to_resource(node, 0,&res)) {
>> +		WARN(1, "unable to get intc registers\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>> +		WARN(1, "unable to get intc-size\n");
>> +		return -EINVAL;
>
> There is no default value that makes sense?

So far we have 96 or 128, so I can put 96 as a default and just keep the 
warning without returning an error.

>> +	}
>> +
>> +	omap_init_irq(res.start, nr_irqs);
>> +	irq_domain_add_simple(node, 0);
>
> Have you read the NO_IRQ thread...

Yeah, I tried, but that's a long email thread with some unclear 
conclusion...
The point is that the few users of that API today are using 0 as a base 
as well, so I thought it was still valid.

> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
> the base. I would pick 16 to skip over legacy ISA irqs.

I do not have any 0 interrupts right now, my concern is that I still 
have some legacy non-DT drivers that expect the previous hwirq = irq 
mapping.

> irqdomains should always be enabled regardless of CONFIG_OF. So either
> you can leave it as is if OF is always enabled for OMAP, or you should
> move domain setup into omap_init_irq.

OK, but it looks like I cannot really modify the current INTC to DT 
without having fully adapted the OMAP drivers to use 
irq_of_parse_and_map. Or did I miss something?

Regards,
Benoit

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
> On 12/7/2011 10:20 PM, Rob Herring wrote:
>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
> 
> [...]
> 
>>> +#ifdef CONFIG_OF
>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>> *parent)
>>> +{
>>> +    struct resource res;
>>> +    u32 nr_irqs;
>>> +
>>> +    if (WARN_ON(!node))
>>> +        return -ENODEV;
>>> +
>>> +    if (of_address_to_resource(node, 0,&res)) {
>>> +        WARN(1, "unable to get intc registers\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>> +        WARN(1, "unable to get intc-size\n");
>>> +        return -EINVAL;
>>
>> There is no default value that makes sense?
> 
> So far we have 96 or 128, so I can put 96 as a default and just keep the
> warning without returning an error.
> 
>>> +    }
>>> +
>>> +    omap_init_irq(res.start, nr_irqs);
>>> +    irq_domain_add_simple(node, 0);
>>
>> Have you read the NO_IRQ thread...
> 
> Yeah, I tried, but that's a long email thread with some unclear
> conclusion...
> The point is that the few users of that API today are using 0 as a base
> as well, so I thought it was still valid.
> 
>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>> the base. I would pick 16 to skip over legacy ISA irqs.
> 
> I do not have any 0 interrupts right now, my concern is that I still
> have some legacy non-DT drivers that expect the previous hwirq = irq
> mapping.

I guess it depends how easily you can shift all the irq defines. You
allow specifying the base so that you can set it to 0 for non-DT and -1
(dynamic allocation) for DT.

> 
>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>> you can leave it as is if OF is always enabled for OMAP, or you should
>> move domain setup into omap_init_irq.
> 
> OK, but it looks like I cannot really modify the current INTC to DT
> without having fully adapted the OMAP drivers to use
> irq_of_parse_and_map. Or did I miss something?
> 

No, the drivers should not need to be modified as long as they get the
irq's from platform device resources. You just want to make sure the
INTC has no knowledge of it's irq base so it can change.

Rob

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
> On 12/7/2011 10:20 PM, Rob Herring wrote:
>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
> 
> [...]
> 
>>> +#ifdef CONFIG_OF
>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>> *parent)
>>> +{
>>> +    struct resource res;
>>> +    u32 nr_irqs;
>>> +
>>> +    if (WARN_ON(!node))
>>> +        return -ENODEV;
>>> +
>>> +    if (of_address_to_resource(node, 0,&res)) {
>>> +        WARN(1, "unable to get intc registers\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>> +        WARN(1, "unable to get intc-size\n");
>>> +        return -EINVAL;
>>
>> There is no default value that makes sense?
> 
> So far we have 96 or 128, so I can put 96 as a default and just keep the
> warning without returning an error.
> 
>>> +    }
>>> +
>>> +    omap_init_irq(res.start, nr_irqs);
>>> +    irq_domain_add_simple(node, 0);
>>
>> Have you read the NO_IRQ thread...
> 
> Yeah, I tried, but that's a long email thread with some unclear
> conclusion...
> The point is that the few users of that API today are using 0 as a base
> as well, so I thought it was still valid.
> 
>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>> the base. I would pick 16 to skip over legacy ISA irqs.
> 
> I do not have any 0 interrupts right now, my concern is that I still
> have some legacy non-DT drivers that expect the previous hwirq = irq
> mapping.

I guess it depends how easily you can shift all the irq defines. You
allow specifying the base so that you can set it to 0 for non-DT and -1
(dynamic allocation) for DT.

> 
>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>> you can leave it as is if OF is always enabled for OMAP, or you should
>> move domain setup into omap_init_irq.
> 
> OK, but it looks like I cannot really modify the current INTC to DT
> without having fully adapted the OMAP drivers to use
> irq_of_parse_and_map. Or did I miss something?
> 

No, the drivers should not need to be modified as long as they get the
irq's from platform device resources. You just want to make sure the
INTC has no knowledge of it's irq base so it can change.

Rob

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 2:20 PM, Rob Herring wrote:
> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>
>> [...]
>>
>>>> +#ifdef CONFIG_OF
>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>> *parent)
>>>> +{
>>>> +    struct resource res;
>>>> +    u32 nr_irqs;
>>>> +
>>>> +    if (WARN_ON(!node))
>>>> +        return -ENODEV;
>>>> +
>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>> +        WARN(1, "unable to get intc registers\n");
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>> +        WARN(1, "unable to get intc-size\n");
>>>> +        return -EINVAL;
>>>
>>> There is no default value that makes sense?
>>
>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>> warning without returning an error.
>>
>>>> +    }
>>>> +
>>>> +    omap_init_irq(res.start, nr_irqs);
>>>> +    irq_domain_add_simple(node, 0);
>>>
>>> Have you read the NO_IRQ thread...
>>
>> Yeah, I tried, but that's a long email thread with some unclear
>> conclusion...
>> The point is that the few users of that API today are using 0 as a base
>> as well, so I thought it was still valid.
>>
>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>
>> I do not have any 0 interrupts right now, my concern is that I still
>> have some legacy non-DT drivers that expect the previous hwirq = irq
>> mapping.
>
> I guess it depends how easily you can shift all the irq defines. You
> allow specifying the base so that you can set it to 0 for non-DT and -1
> (dynamic allocation) for DT.

The issue, is that the IRQs are not defined anymore but hard coded in 
the hwmod database. And the idea is that this is reflecting exactly the 
hwirq from the spec, so I do not want to add any artificial offset for 
the domain in the original data.

>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>> move domain setup into omap_init_irq.
>>
>> OK, but it looks like I cannot really modify the current INTC to DT
>> without having fully adapted the OMAP drivers to use
>> irq_of_parse_and_map. Or did I miss something?
>>
>
> No, the drivers should not need to be modified as long as they get the
> irq's from platform device resources. You just want to make sure the
> INTC has no knowledge of it's irq base so it can change.

OK, the driver will not have to change but the IRQ value will not be the 
same in the case of DT since it will use the irq_create_of_mapping.

Currently, the driver IRQ resource is 7 for example for the twl, this is 
the hwirq (= irq). If I use a domain with a base of 16, the resource 
will still be 7 except if the resource is created from OF, then it will 
be 23.

The only way I have today to maintain the legacy drivers to work without 
hacking the OMAP resource code is to keep the base_irq at 0.

Since I do not have any hwirq at 0 so far, is it a big deal to keep the 
base_irq at 0 for the moment?
It will be easier to transition to DT that way without breaking the 
existing drivers.

Regards,
Benoit

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 2:20 PM, Rob Herring wrote:
> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>
>> [...]
>>
>>>> +#ifdef CONFIG_OF
>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>> *parent)
>>>> +{
>>>> +    struct resource res;
>>>> +    u32 nr_irqs;
>>>> +
>>>> +    if (WARN_ON(!node))
>>>> +        return -ENODEV;
>>>> +
>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>> +        WARN(1, "unable to get intc registers\n");
>>>> +        return -EINVAL;
>>>> +    }
>>>> +
>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>> +        WARN(1, "unable to get intc-size\n");
>>>> +        return -EINVAL;
>>>
>>> There is no default value that makes sense?
>>
>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>> warning without returning an error.
>>
>>>> +    }
>>>> +
>>>> +    omap_init_irq(res.start, nr_irqs);
>>>> +    irq_domain_add_simple(node, 0);
>>>
>>> Have you read the NO_IRQ thread...
>>
>> Yeah, I tried, but that's a long email thread with some unclear
>> conclusion...
>> The point is that the few users of that API today are using 0 as a base
>> as well, so I thought it was still valid.
>>
>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>
>> I do not have any 0 interrupts right now, my concern is that I still
>> have some legacy non-DT drivers that expect the previous hwirq = irq
>> mapping.
>
> I guess it depends how easily you can shift all the irq defines. You
> allow specifying the base so that you can set it to 0 for non-DT and -1
> (dynamic allocation) for DT.

The issue, is that the IRQs are not defined anymore but hard coded in 
the hwmod database. And the idea is that this is reflecting exactly the 
hwirq from the spec, so I do not want to add any artificial offset for 
the domain in the original data.

>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>> move domain setup into omap_init_irq.
>>
>> OK, but it looks like I cannot really modify the current INTC to DT
>> without having fully adapted the OMAP drivers to use
>> irq_of_parse_and_map. Or did I miss something?
>>
>
> No, the drivers should not need to be modified as long as they get the
> irq's from platform device resources. You just want to make sure the
> INTC has no knowledge of it's irq base so it can change.

OK, the driver will not have to change but the IRQ value will not be the 
same in the case of DT since it will use the irq_create_of_mapping.

Currently, the driver IRQ resource is 7 for example for the twl, this is 
the hwirq (= irq). If I use a domain with a base of 16, the resource 
will still be 7 except if the resource is created from OF, then it will 
be 23.

The only way I have today to maintain the legacy drivers to work without 
hacking the OMAP resource code is to keep the base_irq at 0.

Since I do not have any hwirq at 0 so far, is it a big deal to keep the 
base_irq at 0 for the moment?
It will be easier to transition to DT that way without breaking the 
existing drivers.

Regards,
Benoit

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
> On 12/9/2011 2:20 PM, Rob Herring wrote:
>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>
>>> [...]
>>>
>>>>> +#ifdef CONFIG_OF
>>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>>> *parent)
>>>>> +{
>>>>> +    struct resource res;
>>>>> +    u32 nr_irqs;
>>>>> +
>>>>> +    if (WARN_ON(!node))
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>> +        return -EINVAL;
>>>>
>>>> There is no default value that makes sense?
>>>
>>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>>> warning without returning an error.
>>>
>>>>> +    }
>>>>> +
>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>> +    irq_domain_add_simple(node, 0);
>>>>
>>>> Have you read the NO_IRQ thread...
>>>
>>> Yeah, I tried, but that's a long email thread with some unclear
>>> conclusion...
>>> The point is that the few users of that API today are using 0 as a base
>>> as well, so I thought it was still valid.
>>>
>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>
>>> I do not have any 0 interrupts right now, my concern is that I still
>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>> mapping.
>>
>> I guess it depends how easily you can shift all the irq defines. You
>> allow specifying the base so that you can set it to 0 for non-DT and -1
>> (dynamic allocation) for DT.
> 
> The issue, is that the IRQs are not defined anymore but hard coded in
> the hwmod database. And the idea is that this is reflecting exactly the
> hwirq from the spec, so I do not want to add any artificial offset for
> the domain in the original data.
> 
>>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>>> move domain setup into omap_init_irq.
>>>
>>> OK, but it looks like I cannot really modify the current INTC to DT
>>> without having fully adapted the OMAP drivers to use
>>> irq_of_parse_and_map. Or did I miss something?
>>>
>>
>> No, the drivers should not need to be modified as long as they get the
>> irq's from platform device resources. You just want to make sure the
>> INTC has no knowledge of it's irq base so it can change.
> 
> OK, the driver will not have to change but the IRQ value will not be the
> same in the case of DT since it will use the irq_create_of_mapping.
> 
> Currently, the driver IRQ resource is 7 for example for the twl, this is
> the hwirq (= irq). If I use a domain with a base of 16, the resource
> will still be 7 except if the resource is created from OF, then it will
> be 23.
> 
> The only way I have today to maintain the legacy drivers to work without
> hacking the OMAP resource code is to keep the base_irq at 0.
> 

For non-DT case, that is fine. It should not be hard to support 0 for
legacy and !0 for DT.

> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
> base_irq at 0 for the moment?
> It will be easier to transition to DT that way without breaking the
> existing drivers.

As long as it is trivial to change later. I'm afraid that if it is not
changed, then we won't know if it is trivial.

Rob

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
> On 12/9/2011 2:20 PM, Rob Herring wrote:
>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>
>>> [...]
>>>
>>>>> +#ifdef CONFIG_OF
>>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>>> *parent)
>>>>> +{
>>>>> +    struct resource res;
>>>>> +    u32 nr_irqs;
>>>>> +
>>>>> +    if (WARN_ON(!node))
>>>>> +        return -ENODEV;
>>>>> +
>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>> +        return -EINVAL;
>>>>> +    }
>>>>> +
>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>> +        return -EINVAL;
>>>>
>>>> There is no default value that makes sense?
>>>
>>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>>> warning without returning an error.
>>>
>>>>> +    }
>>>>> +
>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>> +    irq_domain_add_simple(node, 0);
>>>>
>>>> Have you read the NO_IRQ thread...
>>>
>>> Yeah, I tried, but that's a long email thread with some unclear
>>> conclusion...
>>> The point is that the few users of that API today are using 0 as a base
>>> as well, so I thought it was still valid.
>>>
>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>
>>> I do not have any 0 interrupts right now, my concern is that I still
>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>> mapping.
>>
>> I guess it depends how easily you can shift all the irq defines. You
>> allow specifying the base so that you can set it to 0 for non-DT and -1
>> (dynamic allocation) for DT.
> 
> The issue, is that the IRQs are not defined anymore but hard coded in
> the hwmod database. And the idea is that this is reflecting exactly the
> hwirq from the spec, so I do not want to add any artificial offset for
> the domain in the original data.
> 
>>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>>> move domain setup into omap_init_irq.
>>>
>>> OK, but it looks like I cannot really modify the current INTC to DT
>>> without having fully adapted the OMAP drivers to use
>>> irq_of_parse_and_map. Or did I miss something?
>>>
>>
>> No, the drivers should not need to be modified as long as they get the
>> irq's from platform device resources. You just want to make sure the
>> INTC has no knowledge of it's irq base so it can change.
> 
> OK, the driver will not have to change but the IRQ value will not be the
> same in the case of DT since it will use the irq_create_of_mapping.
> 
> Currently, the driver IRQ resource is 7 for example for the twl, this is
> the hwirq (= irq). If I use a domain with a base of 16, the resource
> will still be 7 except if the resource is created from OF, then it will
> be 23.
> 
> The only way I have today to maintain the legacy drivers to work without
> hacking the OMAP resource code is to keep the base_irq at 0.
> 

For non-DT case, that is fine. It should not be hard to support 0 for
legacy and !0 for DT.

> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
> base_irq at 0 for the moment?
> It will be easier to transition to DT that way without breaking the
> existing drivers.

As long as it is trivial to change later. I'm afraid that if it is not
changed, then we won't know if it is trivial.

Rob

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 4:22 PM, Rob Herring wrote:
> On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
>> On 12/9/2011 2:20 PM, Rob Herring wrote:
>>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>>
>>>> [...]
>>>>
>>>>>> +#ifdef CONFIG_OF
>>>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>>>> *parent)
>>>>>> +{
>>>>>> +    struct resource res;
>>>>>> +    u32 nr_irqs;
>>>>>> +
>>>>>> +    if (WARN_ON(!node))
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>>> +        return -EINVAL;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>>> +        return -EINVAL;
>>>>>
>>>>> There is no default value that makes sense?
>>>>
>>>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>>>> warning without returning an error.
>>>>
>>>>>> +    }
>>>>>> +
>>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>>> +    irq_domain_add_simple(node, 0);
>>>>>
>>>>> Have you read the NO_IRQ thread...
>>>>
>>>> Yeah, I tried, but that's a long email thread with some unclear
>>>> conclusion...
>>>> The point is that the few users of that API today are using 0 as a base
>>>> as well, so I thought it was still valid.
>>>>
>>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>>
>>>> I do not have any 0 interrupts right now, my concern is that I still
>>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>>> mapping.
>>>
>>> I guess it depends how easily you can shift all the irq defines. You
>>> allow specifying the base so that you can set it to 0 for non-DT and -1
>>> (dynamic allocation) for DT.
>>
>> The issue, is that the IRQs are not defined anymore but hard coded in
>> the hwmod database. And the idea is that this is reflecting exactly the
>> hwirq from the spec, so I do not want to add any artificial offset for
>> the domain in the original data.
>>
>>>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>>>> move domain setup into omap_init_irq.
>>>>
>>>> OK, but it looks like I cannot really modify the current INTC to DT
>>>> without having fully adapted the OMAP drivers to use
>>>> irq_of_parse_and_map. Or did I miss something?
>>>>
>>>
>>> No, the drivers should not need to be modified as long as they get the
>>> irq's from platform device resources. You just want to make sure the
>>> INTC has no knowledge of it's irq base so it can change.
>>
>> OK, the driver will not have to change but the IRQ value will not be the
>> same in the case of DT since it will use the irq_create_of_mapping.
>>
>> Currently, the driver IRQ resource is 7 for example for the twl, this is
>> the hwirq (= irq). If I use a domain with a base of 16, the resource
>> will still be 7 except if the resource is created from OF, then it will
>> be 23.
>>
>> The only way I have today to maintain the legacy drivers to work without
>> hacking the OMAP resource code is to keep the base_irq at 0.
>>
>
> For non-DT case, that is fine. It should not be hard to support 0 for
> legacy and !0 for DT.

Mmm, I'm probably missing something here.

My point is that even in the DT case I do have some devices that are 
initialized without DT for the moment and thus cannot get access to the 
interrupt-controller node and then cannot retrieve the domain information.

How can I ensure the proper hwirq -> irq translation then for such devices?
Only the one created by DT will have the correct irq number.

Adding some hacks to add a +16 is clearly possible, but that will be a 
dirty hacks in the OMAP core code that will have to be removed later. So 
I'd rather keep the domain with a 0 offset to avoid all these temporary 
hacks.

>> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
>> base_irq at 0 for the moment?
>> It will be easier to transition to DT that way without breaking the
>> existing drivers.
>
> As long as it is trivial to change later. I'm afraid that if it is not
> changed, then we won't know if it is trivial.

As soon as every devices are initialized from DT, the irq_base will 
become transparent, since both the IRQ handler and the IRQ resource will 
use the irq_domain_to_irq to get the irq from the hwirq. At that time 
adding 16 will then be straightforward.
That's why I prefer that approach which is the one that generate the 
minimal temporary effort.

Regards,
Benoit

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 4:22 PM, Rob Herring wrote:
> On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
>> On 12/9/2011 2:20 PM, Rob Herring wrote:
>>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>>
>>>> [...]
>>>>
>>>>>> +#ifdef CONFIG_OF
>>>>>> +int __init intc_of_init(struct device_node *node, struct device_node
>>>>>> *parent)
>>>>>> +{
>>>>>> +    struct resource res;
>>>>>> +    u32 nr_irqs;
>>>>>> +
>>>>>> +    if (WARN_ON(!node))
>>>>>> +        return -ENODEV;
>>>>>> +
>>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>>> +        return -EINVAL;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>>> +        return -EINVAL;
>>>>>
>>>>> There is no default value that makes sense?
>>>>
>>>> So far we have 96 or 128, so I can put 96 as a default and just keep the
>>>> warning without returning an error.
>>>>
>>>>>> +    }
>>>>>> +
>>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>>> +    irq_domain_add_simple(node, 0);
>>>>>
>>>>> Have you read the NO_IRQ thread...
>>>>
>>>> Yeah, I tried, but that's a long email thread with some unclear
>>>> conclusion...
>>>> The point is that the few users of that API today are using 0 as a base
>>>> as well, so I thought it was still valid.
>>>>
>>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use 0 for
>>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>>
>>>> I do not have any 0 interrupts right now, my concern is that I still
>>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>>> mapping.
>>>
>>> I guess it depends how easily you can shift all the irq defines. You
>>> allow specifying the base so that you can set it to 0 for non-DT and -1
>>> (dynamic allocation) for DT.
>>
>> The issue, is that the IRQs are not defined anymore but hard coded in
>> the hwmod database. And the idea is that this is reflecting exactly the
>> hwirq from the spec, so I do not want to add any artificial offset for
>> the domain in the original data.
>>
>>>>> irqdomains should always be enabled regardless of CONFIG_OF. So either
>>>>> you can leave it as is if OF is always enabled for OMAP, or you should
>>>>> move domain setup into omap_init_irq.
>>>>
>>>> OK, but it looks like I cannot really modify the current INTC to DT
>>>> without having fully adapted the OMAP drivers to use
>>>> irq_of_parse_and_map. Or did I miss something?
>>>>
>>>
>>> No, the drivers should not need to be modified as long as they get the
>>> irq's from platform device resources. You just want to make sure the
>>> INTC has no knowledge of it's irq base so it can change.
>>
>> OK, the driver will not have to change but the IRQ value will not be the
>> same in the case of DT since it will use the irq_create_of_mapping.
>>
>> Currently, the driver IRQ resource is 7 for example for the twl, this is
>> the hwirq (= irq). If I use a domain with a base of 16, the resource
>> will still be 7 except if the resource is created from OF, then it will
>> be 23.
>>
>> The only way I have today to maintain the legacy drivers to work without
>> hacking the OMAP resource code is to keep the base_irq at 0.
>>
>
> For non-DT case, that is fine. It should not be hard to support 0 for
> legacy and !0 for DT.

Mmm, I'm probably missing something here.

My point is that even in the DT case I do have some devices that are 
initialized without DT for the moment and thus cannot get access to the 
interrupt-controller node and then cannot retrieve the domain information.

How can I ensure the proper hwirq -> irq translation then for such devices?
Only the one created by DT will have the correct irq number.

Adding some hacks to add a +16 is clearly possible, but that will be a 
dirty hacks in the OMAP core code that will have to be removed later. So 
I'd rather keep the domain with a 0 offset to avoid all these temporary 
hacks.

>> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
>> base_irq at 0 for the moment?
>> It will be easier to transition to DT that way without breaking the
>> existing drivers.
>
> As long as it is trivial to change later. I'm afraid that if it is not
> changed, then we won't know if it is trivial.

As soon as every devices are initialized from DT, the irq_base will 
become transparent, since both the IRQ handler and the IRQ resource will 
use the irq_domain_to_irq to get the irq from the hwirq. At that time 
adding 16 will then be straightforward.
That's why I prefer that approach which is the one that generate the 
minimal temporary effort.

Regards,
Benoit

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/09/2011 10:06 AM, Cousson, Benoit wrote:
> On 12/9/2011 4:22 PM, Rob Herring wrote:
>> On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
>>> On 12/9/2011 2:20 PM, Rob Herring wrote:
>>>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> +int __init intc_of_init(struct device_node *node, struct
>>>>>>> device_node
>>>>>>> *parent)
>>>>>>> +{
>>>>>>> +    struct resource res;
>>>>>>> +    u32 nr_irqs;
>>>>>>> +
>>>>>>> +    if (WARN_ON(!node))
>>>>>>> +        return -ENODEV;
>>>>>>> +
>>>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>>>> +        return -EINVAL;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>>>> +        return -EINVAL;
>>>>>>
>>>>>> There is no default value that makes sense?
>>>>>
>>>>> So far we have 96 or 128, so I can put 96 as a default and just
>>>>> keep the
>>>>> warning without returning an error.
>>>>>
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>>>> +    irq_domain_add_simple(node, 0);
>>>>>>
>>>>>> Have you read the NO_IRQ thread...
>>>>>
>>>>> Yeah, I tried, but that's a long email thread with some unclear
>>>>> conclusion...
>>>>> The point is that the few users of that API today are using 0 as a
>>>>> base
>>>>> as well, so I thought it was still valid.
>>>>>
>>>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use
>>>>>> 0 for
>>>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>>>
>>>>> I do not have any 0 interrupts right now, my concern is that I still
>>>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>>>> mapping.
>>>>
>>>> I guess it depends how easily you can shift all the irq defines. You
>>>> allow specifying the base so that you can set it to 0 for non-DT and -1
>>>> (dynamic allocation) for DT.
>>>
>>> The issue, is that the IRQs are not defined anymore but hard coded in
>>> the hwmod database. And the idea is that this is reflecting exactly the
>>> hwirq from the spec, so I do not want to add any artificial offset for
>>> the domain in the original data.
>>>
>>>>>> irqdomains should always be enabled regardless of CONFIG_OF. So
>>>>>> either
>>>>>> you can leave it as is if OF is always enabled for OMAP, or you
>>>>>> should
>>>>>> move domain setup into omap_init_irq.
>>>>>
>>>>> OK, but it looks like I cannot really modify the current INTC to DT
>>>>> without having fully adapted the OMAP drivers to use
>>>>> irq_of_parse_and_map. Or did I miss something?
>>>>>
>>>>
>>>> No, the drivers should not need to be modified as long as they get the
>>>> irq's from platform device resources. You just want to make sure the
>>>> INTC has no knowledge of it's irq base so it can change.
>>>
>>> OK, the driver will not have to change but the IRQ value will not be the
>>> same in the case of DT since it will use the irq_create_of_mapping.
>>>
>>> Currently, the driver IRQ resource is 7 for example for the twl, this is
>>> the hwirq (= irq). If I use a domain with a base of 16, the resource
>>> will still be 7 except if the resource is created from OF, then it will
>>> be 23.
>>>
>>> The only way I have today to maintain the legacy drivers to work without
>>> hacking the OMAP resource code is to keep the base_irq at 0.
>>>
>>
>> For non-DT case, that is fine. It should not be hard to support 0 for
>> legacy and !0 for DT.
> 
> Mmm, I'm probably missing something here.
> 
> My point is that even in the DT case I do have some devices that are
> initialized without DT for the moment and thus cannot get access to the
> interrupt-controller node and then cannot retrieve the domain information.
> 
> How can I ensure the proper hwirq -> irq translation then for such devices?
> Only the one created by DT will have the correct irq number.
> 

Okay, I missed that aspect of it. So I guess 0 base is fine for now.

Rob

> Adding some hacks to add a +16 is clearly possible, but that will be a
> dirty hacks in the OMAP core code that will have to be removed later. So
> I'd rather keep the domain with a 0 offset to avoid all these temporary
> hacks.
> 
>>> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
>>> base_irq at 0 for the moment?
>>> It will be easier to transition to DT that way without breaking the
>>> existing drivers.
>>
>> As long as it is trivial to change later. I'm afraid that if it is not
>> changed, then we won't know if it is trivial.
> 
> As soon as every devices are initialized from DT, the irq_base will
> become transparent, since both the IRQ handler and the IRQ resource will
> use the irq_domain_to_irq to get the irq from the hwirq. At that time
> adding 16 will then be straightforward.
> That's why I prefer that approach which is the one that generate the
> minimal temporary effort.
> 
> Regards,
> Benoit

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/09/2011 10:06 AM, Cousson, Benoit wrote:
> On 12/9/2011 4:22 PM, Rob Herring wrote:
>> On 12/09/2011 08:52 AM, Cousson, Benoit wrote:
>>> On 12/9/2011 2:20 PM, Rob Herring wrote:
>>>> On 12/08/2011 08:59 AM, Cousson, Benoit wrote:
>>>>> On 12/7/2011 10:20 PM, Rob Herring wrote:
>>>>>> On 12/07/2011 02:50 PM, Benoit Cousson wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>> +#ifdef CONFIG_OF
>>>>>>> +int __init intc_of_init(struct device_node *node, struct
>>>>>>> device_node
>>>>>>> *parent)
>>>>>>> +{
>>>>>>> +    struct resource res;
>>>>>>> +    u32 nr_irqs;
>>>>>>> +
>>>>>>> +    if (WARN_ON(!node))
>>>>>>> +        return -ENODEV;
>>>>>>> +
>>>>>>> +    if (of_address_to_resource(node, 0,&res)) {
>>>>>>> +        WARN(1, "unable to get intc registers\n");
>>>>>>> +        return -EINVAL;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
>>>>>>> +        WARN(1, "unable to get intc-size\n");
>>>>>>> +        return -EINVAL;
>>>>>>
>>>>>> There is no default value that makes sense?
>>>>>
>>>>> So far we have 96 or 128, so I can put 96 as a default and just
>>>>> keep the
>>>>> warning without returning an error.
>>>>>
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    omap_init_irq(res.start, nr_irqs);
>>>>>>> +    irq_domain_add_simple(node, 0);
>>>>>>
>>>>>> Have you read the NO_IRQ thread...
>>>>>
>>>>> Yeah, I tried, but that's a long email thread with some unclear
>>>>> conclusion...
>>>>> The point is that the few users of that API today are using 0 as a
>>>>> base
>>>>> as well, so I thought it was still valid.
>>>>>
>>>>>> Is 0 ever a valid interrupt for a driver? If so, you must not use
>>>>>> 0 for
>>>>>> the base. I would pick 16 to skip over legacy ISA irqs.
>>>>>
>>>>> I do not have any 0 interrupts right now, my concern is that I still
>>>>> have some legacy non-DT drivers that expect the previous hwirq = irq
>>>>> mapping.
>>>>
>>>> I guess it depends how easily you can shift all the irq defines. You
>>>> allow specifying the base so that you can set it to 0 for non-DT and -1
>>>> (dynamic allocation) for DT.
>>>
>>> The issue, is that the IRQs are not defined anymore but hard coded in
>>> the hwmod database. And the idea is that this is reflecting exactly the
>>> hwirq from the spec, so I do not want to add any artificial offset for
>>> the domain in the original data.
>>>
>>>>>> irqdomains should always be enabled regardless of CONFIG_OF. So
>>>>>> either
>>>>>> you can leave it as is if OF is always enabled for OMAP, or you
>>>>>> should
>>>>>> move domain setup into omap_init_irq.
>>>>>
>>>>> OK, but it looks like I cannot really modify the current INTC to DT
>>>>> without having fully adapted the OMAP drivers to use
>>>>> irq_of_parse_and_map. Or did I miss something?
>>>>>
>>>>
>>>> No, the drivers should not need to be modified as long as they get the
>>>> irq's from platform device resources. You just want to make sure the
>>>> INTC has no knowledge of it's irq base so it can change.
>>>
>>> OK, the driver will not have to change but the IRQ value will not be the
>>> same in the case of DT since it will use the irq_create_of_mapping.
>>>
>>> Currently, the driver IRQ resource is 7 for example for the twl, this is
>>> the hwirq (= irq). If I use a domain with a base of 16, the resource
>>> will still be 7 except if the resource is created from OF, then it will
>>> be 23.
>>>
>>> The only way I have today to maintain the legacy drivers to work without
>>> hacking the OMAP resource code is to keep the base_irq at 0.
>>>
>>
>> For non-DT case, that is fine. It should not be hard to support 0 for
>> legacy and !0 for DT.
> 
> Mmm, I'm probably missing something here.
> 
> My point is that even in the DT case I do have some devices that are
> initialized without DT for the moment and thus cannot get access to the
> interrupt-controller node and then cannot retrieve the domain information.
> 
> How can I ensure the proper hwirq -> irq translation then for such devices?
> Only the one created by DT will have the correct irq number.
> 

Okay, I missed that aspect of it. So I guess 0 base is fine for now.

Rob

> Adding some hacks to add a +16 is clearly possible, but that will be a
> dirty hacks in the OMAP core code that will have to be removed later. So
> I'd rather keep the domain with a 0 offset to avoid all these temporary
> hacks.
> 
>>> Since I do not have any hwirq at 0 so far, is it a big deal to keep the
>>> base_irq at 0 for the moment?
>>> It will be easier to transition to DT that way without breaking the
>>> existing drivers.
>>
>> As long as it is trivial to change later. I'm afraid that if it is not
>> changed, then we won't know if it is trivial.
> 
> As soon as every devices are initialized from DT, the irq_base will
> become transparent, since both the IRQ handler and the IRQ resource will
> use the irq_domain_to_irq to get the irq from the hwirq. At that time
> adding 16 will then be straightforward.
> That's why I prefer that approach which is the one that generate the
> minimal temporary effort.
> 
> Regards,
> Benoit

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 5:12 PM, Rob Herring wrote:
> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:

[...]

>> Mmm, I'm probably missing something here.
>>
>> My point is that even in the DT case I do have some devices that are
>> initialized without DT for the moment and thus cannot get access to the
>> interrupt-controller node and then cannot retrieve the domain information.
>>
>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>> Only the one created by DT will have the correct irq number.
>
> Okay, I missed that aspect of it. So I guess 0 base is fine for now.

Cool, thanks.
I'll repost early next week to take into account your other comments.

Thanks,
Benoit

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 5:12 PM, Rob Herring wrote:
> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:

[...]

>> Mmm, I'm probably missing something here.
>>
>> My point is that even in the DT case I do have some devices that are
>> initialized without DT for the moment and thus cannot get access to the
>> interrupt-controller node and then cannot retrieve the domain information.
>>
>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>> Only the one created by DT will have the correct irq number.
>
> Okay, I missed that aspect of it. So I guess 0 base is fine for now.

Cool, thanks.
I'll repost early next week to take into account your other comments.

Thanks,
Benoit

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rajendra Nayak]

Hi Benoit,

> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 012bac7..bcfccc2 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
>   void omap2_intc_handle_irq(struct pt_regs *regs);
>   void omap3_intc_handle_irq(struct pt_regs *regs);
>
> +struct device_node;
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent);
> +#else
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	return 0;
> +}

This seems to break !CONFIG_OF builds as common.h is included in
multiple c files causing 'multiple definition' errors.

regards,
Rajendra

> +#endif
> +
>   /*
>    * wfi used in low power code. Directly opcode is used instead
>    * of instruction to avoid mulit-omap build break
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..cafc663 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,9 @@
>   #include<mach/hardware.h>
>   #include<asm/exception.h>
>   #include<asm/mach/irq.h>
> +#include<linux/of.h>
> +#include<linux/of_address.h>
> +#include<linux/irqdomain.h>
>
>
>   /* selected INTC register offsets */
> @@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>   		/* Static mapping, never released */
>   		bank->base_reg = ioremap(base, SZ_4K);
>   		if (!bank->base_reg) {
> -			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
> +			pr_err("Could not ioremap irq bank%i\n", i);
>   			continue;
>   		}
>
> @@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>   		nr_banks++;
>   	}
>
> -	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
> -	       nr_of_irqs, nr_banks, nr_banks>  1 ? "s" : "");
> +	pr_info("Total of %ld interrupts on %d active controller%s\n",
> +		nr_of_irqs, nr_banks, nr_banks>  1 ? "s" : "");
>   }
>
>   void __init omap2_init_irq(void)
> @@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
>   	omap_intc_handle_irq(base_addr, regs);
>   }
>
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct resource res;
> +	u32 nr_irqs;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_address_to_resource(node, 0,&res)) {
> +		WARN(1, "unable to get intc registers\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
> +		WARN(1, "unable to get intc-size\n");
> +		return -EINVAL;
> +	}
> +
> +	omap_init_irq(res.start, nr_irqs);
> +	irq_domain_add_simple(node, 0);
> +
> +	return 0;
> +}
> +#endif
> +
>   #ifdef CONFIG_ARCH_OMAP3
>   static struct omap3_intc_regs intc_context[ARRAY_SIZE(irq_banks)];
>

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rajendra Nayak]

Hi Benoit,

> diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
> index 012bac7..bcfccc2 100644
> --- a/arch/arm/mach-omap2/common.h
> +++ b/arch/arm/mach-omap2/common.h
> @@ -156,6 +156,16 @@ void omap3_intc_resume_idle(void);
>   void omap2_intc_handle_irq(struct pt_regs *regs);
>   void omap3_intc_handle_irq(struct pt_regs *regs);
>
> +struct device_node;
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent);
> +#else
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	return 0;
> +}

This seems to break !CONFIG_OF builds as common.h is included in
multiple c files causing 'multiple definition' errors.

regards,
Rajendra

> +#endif
> +
>   /*
>    * wfi used in low power code. Directly opcode is used instead
>    * of instruction to avoid mulit-omap build break
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..cafc663 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,9 @@
>   #include<mach/hardware.h>
>   #include<asm/exception.h>
>   #include<asm/mach/irq.h>
> +#include<linux/of.h>
> +#include<linux/of_address.h>
> +#include<linux/irqdomain.h>
>
>
>   /* selected INTC register offsets */
> @@ -166,7 +169,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>   		/* Static mapping, never released */
>   		bank->base_reg = ioremap(base, SZ_4K);
>   		if (!bank->base_reg) {
> -			printk(KERN_ERR "Could not ioremap irq bank%i\n", i);
> +			pr_err("Could not ioremap irq bank%i\n", i);
>   			continue;
>   		}
>
> @@ -179,8 +182,8 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>   		nr_banks++;
>   	}
>
> -	printk(KERN_INFO "Total of %ld interrupts on %d active controller%s\n",
> -	       nr_of_irqs, nr_banks, nr_banks>  1 ? "s" : "");
> +	pr_info("Total of %ld interrupts on %d active controller%s\n",
> +		nr_of_irqs, nr_banks, nr_banks>  1 ? "s" : "");
>   }
>
>   void __init omap2_init_irq(void)
> @@ -236,6 +239,32 @@ asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs
>   	omap_intc_handle_irq(base_addr, regs);
>   }
>
> +#ifdef CONFIG_OF
> +int __init intc_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	struct resource res;
> +	u32 nr_irqs;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_address_to_resource(node, 0,&res)) {
> +		WARN(1, "unable to get intc registers\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "ti,intc-size",&nr_irqs)) {
> +		WARN(1, "unable to get intc-size\n");
> +		return -EINVAL;
> +	}
> +
> +	omap_init_irq(res.start, nr_irqs);
> +	irq_domain_add_simple(node, 0);
> +
> +	return 0;
> +}
> +#endif
> +
>   #ifdef CONFIG_ARCH_OMAP3
>   static struct omap3_intc_regs intc_context[ARRAY_SIZE(irq_banks)];
>

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 5:12 PM, Rob Herring wrote:
> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:

[...]

>> My point is that even in the DT case I do have some devices that are
>> initialized without DT for the moment and thus cannot get access to the
>> interrupt-controller node and then cannot retrieve the domain information.
>>
>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>> Only the one created by DT will have the correct irq number.
> 
> Okay, I missed that aspect of it. So I guess 0 base is fine for now.
> 
> Rob

Following that discussion, I made a patch (included below) last week to add domain support just before this one for DT migration.
I'm now wondering if it makes sense to do it like that considering your recent patch: irq: convert generic-chip to use irq_domain.

What do you think?

Thanks,
Benoit

---
>From 3083589f48604aab3d801179810c2af8339525ae Mon Sep 17 00:00:00 2001
From: Benoit Cousson <b-cousson@ti.com>
Date: Thu, 8 Dec 2011 22:16:51 +0100
Subject: [PATCH] ARM: OMAP2/3: intc: Add irqdomain support

Introduce the usage of the irqdomain to prepare the DT support.
The irq_base is still hard coded to 0 to allow non-DT drivers
to work with the previous assumption that was hwirq = irq.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/mach-omap2/irq.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 42b1d65..2f65dfd 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -17,6 +17,7 @@
 #include <mach/hardware.h>
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
+#include <linux/irqdomain.h>
 
 
 /* selected INTC register offsets */
@@ -57,6 +58,8 @@ static struct omap_irq_bank {
 	},
 };
 
+static struct irq_domain domain;
+
 /* Structure to save interrupt controller context */
 struct omap3_intc_regs {
 	u32 sysconfig;
@@ -158,6 +161,17 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 	if (WARN_ON(!omap_irq_base))
 		return;
 
+	/*
+	 * XXX: Use a 0 irq_base for the moment since the legacy devices
+	 * created statically are expected a hwirq = irq mapping.
+	 * A proper offset will be added later, when IRQ resource creation
+	 * will be handled by DT.
+	 */
+	domain.irq_base = 0;
+	domain.nr_irq = nr_irqs;
+	domain.ops = &irq_domain_simple_ops;
+	irq_domain_add(&domain);
+
 	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
 		struct omap_irq_bank *bank = irq_banks + i;
 
@@ -225,8 +239,10 @@ out:
 		irqnr = readl_relaxed(base_addr + INTCPS_SIR_IRQ_OFFSET);
 		irqnr &= ACTIVEIRQ_MASK;
 
-		if (irqnr)
+		if (irqnr) {
+			irqnr = irq_domain_to_irq(&domain, irqnr);
 			handle_IRQ(irqnr, regs);
+		}
 	} while (irqnr);
 }
 
-- 
1.7.0.4

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Cousson, Benoit]

On 12/9/2011 5:12 PM, Rob Herring wrote:
> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:

[...]

>> My point is that even in the DT case I do have some devices that are
>> initialized without DT for the moment and thus cannot get access to the
>> interrupt-controller node and then cannot retrieve the domain information.
>>
>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>> Only the one created by DT will have the correct irq number.
> 
> Okay, I missed that aspect of it. So I guess 0 base is fine for now.
> 
> Rob

Following that discussion, I made a patch (included below) last week to add domain support just before this one for DT migration.
I'm now wondering if it makes sense to do it like that considering your recent patch: irq: convert generic-chip to use irq_domain.

What do you think?

Thanks,
Benoit

---
>From 3083589f48604aab3d801179810c2af8339525ae Mon Sep 17 00:00:00 2001
From: Benoit Cousson <b-cousson@ti.com>
Date: Thu, 8 Dec 2011 22:16:51 +0100
Subject: [PATCH] ARM: OMAP2/3: intc: Add irqdomain support

Introduce the usage of the irqdomain to prepare the DT support.
The irq_base is still hard coded to 0 to allow non-DT drivers
to work with the previous assumption that was hwirq = irq.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 arch/arm/mach-omap2/irq.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
index 42b1d65..2f65dfd 100644
--- a/arch/arm/mach-omap2/irq.c
+++ b/arch/arm/mach-omap2/irq.c
@@ -17,6 +17,7 @@
 #include <mach/hardware.h>
 #include <asm/exception.h>
 #include <asm/mach/irq.h>
+#include <linux/irqdomain.h>
 
 
 /* selected INTC register offsets */
@@ -57,6 +58,8 @@ static struct omap_irq_bank {
 	},
 };
 
+static struct irq_domain domain;
+
 /* Structure to save interrupt controller context */
 struct omap3_intc_regs {
 	u32 sysconfig;
@@ -158,6 +161,17 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
 	if (WARN_ON(!omap_irq_base))
 		return;
 
+	/*
+	 * XXX: Use a 0 irq_base for the moment since the legacy devices
+	 * created statically are expected a hwirq = irq mapping.
+	 * A proper offset will be added later, when IRQ resource creation
+	 * will be handled by DT.
+	 */
+	domain.irq_base = 0;
+	domain.nr_irq = nr_irqs;
+	domain.ops = &irq_domain_simple_ops;
+	irq_domain_add(&domain);
+
 	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
 		struct omap_irq_bank *bank = irq_banks + i;
 
@@ -225,8 +239,10 @@ out:
 		irqnr = readl_relaxed(base_addr + INTCPS_SIR_IRQ_OFFSET);
 		irqnr &= ACTIVEIRQ_MASK;
 
-		if (irqnr)
+		if (irqnr) {
+			irqnr = irq_domain_to_irq(&domain, irqnr);
 			handle_IRQ(irqnr, regs);
+		}
 	} while (irqnr);
 }
 
-- 
1.7.0.4

Re: [PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/15/2011 11:49 AM, Cousson, Benoit wrote:
> On 12/9/2011 5:12 PM, Rob Herring wrote:
>> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:
> 
> [...]
> 
>>> My point is that even in the DT case I do have some devices that are
>>> initialized without DT for the moment and thus cannot get access to the
>>> interrupt-controller node and then cannot retrieve the domain information.
>>>
>>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>>> Only the one created by DT will have the correct irq number.
>>
>> Okay, I missed that aspect of it. So I guess 0 base is fine for now.
>>
>> Rob
> 
> Following that discussion, I made a patch (included below) last week to add domain support just before this one for DT migration.
> I'm now wondering if it makes sense to do it like that considering your recent patch: irq: convert generic-chip to use irq_domain.
> 
> What do you think?

Using my patch should definitely simplify yours. You'll definitely need
the fix I discussed with Shawn. Also, I do have to sort out that it
breaks on x86 with Grant's suggested change, but that's not too complicated.

Rob

> 
> Thanks,
> Benoit
> 
> ---
> From 3083589f48604aab3d801179810c2af8339525ae Mon Sep 17 00:00:00 2001
> From: Benoit Cousson <b-cousson@ti.com>
> Date: Thu, 8 Dec 2011 22:16:51 +0100
> Subject: [PATCH] ARM: OMAP2/3: intc: Add irqdomain support
> 
> Introduce the usage of the irqdomain to prepare the DT support.
> The irq_base is still hard coded to 0 to allow non-DT drivers
> to work with the previous assumption that was hwirq = irq.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  arch/arm/mach-omap2/irq.c |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..2f65dfd 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,7 @@
>  #include <mach/hardware.h>
>  #include <asm/exception.h>
>  #include <asm/mach/irq.h>
> +#include <linux/irqdomain.h>
>  
>  
>  /* selected INTC register offsets */
> @@ -57,6 +58,8 @@ static struct omap_irq_bank {
>  	},
>  };
>  
> +static struct irq_domain domain;
> +
>  /* Structure to save interrupt controller context */
>  struct omap3_intc_regs {
>  	u32 sysconfig;
> @@ -158,6 +161,17 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  	if (WARN_ON(!omap_irq_base))
>  		return;
>  
> +	/*
> +	 * XXX: Use a 0 irq_base for the moment since the legacy devices
> +	 * created statically are expected a hwirq = irq mapping.
> +	 * A proper offset will be added later, when IRQ resource creation
> +	 * will be handled by DT.
> +	 */
> +	domain.irq_base = 0;
> +	domain.nr_irq = nr_irqs;
> +	domain.ops = &irq_domain_simple_ops;
> +	irq_domain_add(&domain);
> +
>  	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
>  		struct omap_irq_bank *bank = irq_banks + i;
>  
> @@ -225,8 +239,10 @@ out:
>  		irqnr = readl_relaxed(base_addr + INTCPS_SIR_IRQ_OFFSET);
>  		irqnr &= ACTIVEIRQ_MASK;
>  
> -		if (irqnr)
> +		if (irqnr) {
> +			irqnr = irq_domain_to_irq(&domain, irqnr);
>  			handle_IRQ(irqnr, regs);
> +		}
>  	} while (irqnr);
>  }
>  

[PATCH 2/4] ARM: OMAP2/3: intc: Add DT support for TI interrupt controller

[Rob Herring]

On 12/15/2011 11:49 AM, Cousson, Benoit wrote:
> On 12/9/2011 5:12 PM, Rob Herring wrote:
>> On 12/09/2011 10:06 AM, Cousson, Benoit wrote:
> 
> [...]
> 
>>> My point is that even in the DT case I do have some devices that are
>>> initialized without DT for the moment and thus cannot get access to the
>>> interrupt-controller node and then cannot retrieve the domain information.
>>>
>>> How can I ensure the proper hwirq ->  irq translation then for such devices?
>>> Only the one created by DT will have the correct irq number.
>>
>> Okay, I missed that aspect of it. So I guess 0 base is fine for now.
>>
>> Rob
> 
> Following that discussion, I made a patch (included below) last week to add domain support just before this one for DT migration.
> I'm now wondering if it makes sense to do it like that considering your recent patch: irq: convert generic-chip to use irq_domain.
> 
> What do you think?

Using my patch should definitely simplify yours. You'll definitely need
the fix I discussed with Shawn. Also, I do have to sort out that it
breaks on x86 with Grant's suggested change, but that's not too complicated.

Rob

> 
> Thanks,
> Benoit
> 
> ---
> From 3083589f48604aab3d801179810c2af8339525ae Mon Sep 17 00:00:00 2001
> From: Benoit Cousson <b-cousson@ti.com>
> Date: Thu, 8 Dec 2011 22:16:51 +0100
> Subject: [PATCH] ARM: OMAP2/3: intc: Add irqdomain support
> 
> Introduce the usage of the irqdomain to prepare the DT support.
> The irq_base is still hard coded to 0 to allow non-DT drivers
> to work with the previous assumption that was hwirq = irq.
> 
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Rob Herring <rob.herring@calxeda.com>
> ---
>  arch/arm/mach-omap2/irq.c |   18 +++++++++++++++++-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/irq.c b/arch/arm/mach-omap2/irq.c
> index 42b1d65..2f65dfd 100644
> --- a/arch/arm/mach-omap2/irq.c
> +++ b/arch/arm/mach-omap2/irq.c
> @@ -17,6 +17,7 @@
>  #include <mach/hardware.h>
>  #include <asm/exception.h>
>  #include <asm/mach/irq.h>
> +#include <linux/irqdomain.h>
>  
>  
>  /* selected INTC register offsets */
> @@ -57,6 +58,8 @@ static struct omap_irq_bank {
>  	},
>  };
>  
> +static struct irq_domain domain;
> +
>  /* Structure to save interrupt controller context */
>  struct omap3_intc_regs {
>  	u32 sysconfig;
> @@ -158,6 +161,17 @@ static void __init omap_init_irq(u32 base, int nr_irqs)
>  	if (WARN_ON(!omap_irq_base))
>  		return;
>  
> +	/*
> +	 * XXX: Use a 0 irq_base for the moment since the legacy devices
> +	 * created statically are expected a hwirq = irq mapping.
> +	 * A proper offset will be added later, when IRQ resource creation
> +	 * will be handled by DT.
> +	 */
> +	domain.irq_base = 0;
> +	domain.nr_irq = nr_irqs;
> +	domain.ops = &irq_domain_simple_ops;
> +	irq_domain_add(&domain);
> +
>  	for (i = 0; i < ARRAY_SIZE(irq_banks); i++) {
>  		struct omap_irq_bank *bank = irq_banks + i;
>  
> @@ -225,8 +239,10 @@ out:
>  		irqnr = readl_relaxed(base_addr + INTCPS_SIR_IRQ_OFFSET);
>  		irqnr &= ACTIVEIRQ_MASK;
>  
> -		if (irqnr)
> +		if (irqnr) {
> +			irqnr = irq_domain_to_irq(&domain, irqnr);
>  			handle_IRQ(irqnr, regs);
> +		}
>  	} while (irqnr);
>  }
>