irqdomain: Initialize number of IRQs for simple domains

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

Signed-off-by Thierry Reding <[email protected]>

---
 arch/arm/mach-at91/board-dt.c         |    6 +++++-
 arch/arm/mach-imx/imx51-dt.c          |   13 +++++++++++--
 arch/arm/mach-imx/imx53-dt.c          |   13 +++++++++++--
 arch/arm/mach-imx/mach-imx6q.c        |    4 +++-
 arch/arm/mach-msm/board-msm8x60.c     |    7 +++++--
 arch/arm/mach-omap2/board-generic.c   |    7 +++++--
 arch/arm/mach-prima2/irq.c            |    4 +++-
 arch/arm/mach-versatile/core.c        |   10 ++++++++--
 arch/arm/plat-mxc/include/mach/irqs.h |    1 +
 arch/arm/plat-mxc/tzic.c              |    2 --
 include/linux/irqdomain.h             |   27 ++++++++++++++++++++++-----
 kernel/irq/irqdomain.c                |   30 +++++++++++++++++++-----------
 12 files changed, 93 insertions(+), 31 deletions(-)

arch/arm/mach-at91/board-dt.c +5 -1

index bb6b434..12f003d 100644
@@ -95,7 +95,11 @@ static const struct of_device_id aic_of_match[] __initconst = {
 
 static void __init at91_dt_init_irq(void)
 {
-	irq_domain_generate_simple(aic_of_match, 0xfffff000, 0);
+	struct irq_domain *domain;
+
+	domain = irq_domain_generate_simple(aic_of_match, 0xfffff000, 0,
+			NR_AIC_IRQS);
+	WARN_ON(IS_ERR(domain));
 	at91_init_irq_default();
 }
 

arch/arm/mach-imx/imx51-dt.c +11 -2

index e6bad17..72bf94c 100644
@@ -47,7 +47,12 @@ static const struct of_dev_auxdata imx51_auxdata_lookup[] __initconst = {
 static int __init imx51_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	irq_domain_add_simple(np, 0);
+	struct irq_domain *domain;
+
+	domain = irq_domain_add_simple(np, 0, TZIC_NUM_IRQS);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
+
 	return 0;
 }
 

@@ -55,9 +60,13 @@ static int __init imx51_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 
 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
 
 	return 0;
 }

arch/arm/mach-imx/imx53-dt.c +11 -2

index 05ebb3e..ccae620 100644
@@ -51,7 +51,12 @@ static const struct of_dev_auxdata imx53_auxdata_lookup[] __initconst = {
 static int __init imx53_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	irq_domain_add_simple(np, 0);
+	struct irq_domain *domain;
+
+	domain = irq_domain_add_simple(np, 0, TZIC_NUM_IRQS);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
+
 	return 0;
 }
 

@@ -59,9 +64,13 @@ static int __init imx53_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 
 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
 
 	return 0;
 }

arch/arm/mach-imx/mach-imx6q.c +3 -1

index c257281..9ed7812 100644
@@ -95,9 +95,11 @@ static int __init imx6q_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 
 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	WARN_ON(IS_ERR(domain));
 
 	return 0;
 }

arch/arm/mach-msm/board-msm8x60.c +5 -2

index 0a11342..a50c7e2 100644
@@ -84,8 +84,11 @@ static void __init msm8x60_dt_init(void)
 
 	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
 			MSM8X60_QGIC_DIST_PHYS);
-	if (node)
-		irq_domain_add_simple(node, GIC_SPI_START);
+	if (node) {
+		struct irq_domain *domain = irq_domain_add_simple(node,
+				GIC_SPI_START, NR_MSM_IRQS);
+		WARN_ON(IS_ERR(domain));
+	}
 
 	if (of_machine_is_compatible("qcom,msm8660-surf")) {
 		printk(KERN_INFO "Init surf UART registers\n");

arch/arm/mach-omap2/board-generic.c +5 -2

index d587560..bf67781 100644
@@ -66,8 +66,11 @@ static struct of_device_id intc_match[] __initdata = {
 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);
+	if (node) {
+		struct irq_domain *domain;
+		domain = irq_domain_add_simple(node, 0, INTCPS_NR_IRQS);
+		WARN_ON(IS_ERR(domain));
+	}
 
 	omap_sdrc_init(NULL, NULL);
 

arch/arm/mach-prima2/irq.c +3 -1

index d93ceef..d3c5136 100644
@@ -58,6 +58,7 @@ static struct of_device_id intc_ids[]  = {
 
 void __init sirfsoc_of_irq_init(void)
 {
+	struct irq_domain *domain;
 	struct device_node *np;
 
 	np = of_find_matching_node(NULL, intc_ids);

@@ -68,7 +69,8 @@ void __init sirfsoc_of_irq_init(void)
 	if (!sirfsoc_intc_base)
 		panic("unable to map intc cpu registers\n");
 
-	irq_domain_add_simple(np, 0);
+	domain = irq_domain_add_simple(np, 0, NR_IRQS);
+	WARN_ON(IS_ERR(domain));
 
 	of_node_put(np);
 

arch/arm/mach-versatile/core.c +8 -2

index 02b7b93..6c40be4 100644
@@ -98,13 +98,19 @@ static const struct of_device_id sic_of_match[] __initconst = {
 
 void __init versatile_init_irq(void)
 {
+	struct irq_domain *domain;
+
 	vic_init(VA_VIC_BASE, IRQ_VIC_START, ~0, 0);
-	irq_domain_generate_simple(vic_of_match, VERSATILE_VIC_BASE, IRQ_VIC_START);
+	domain = irq_domain_generate_simple(vic_of_match, VERSATILE_VIC_BASE,
+			IRQ_VIC_START, IRQ_VIC_END - IRQ_VIC_START + 1);
+	WARN_ON(IS_ERR(domain));
 
 	writel(~0, VA_SIC_BASE + SIC_IRQ_ENABLE_CLEAR);
 
 	fpga_irq_init(IRQ_VICSOURCE31, ~PIC_MASK, &sic_irq);
-	irq_domain_generate_simple(sic_of_match, VERSATILE_SIC_BASE, IRQ_SIC_START);
+	domain = irq_domain_generate_simple(sic_of_match, VERSATILE_SIC_BASE,
+			IRQ_SIC_START, IRQ_SIC_END - IRQ_SIC_START + 1);
+	WARN_ON(IS_ERR(domain));
 
 	/*
 	 * Interrupts on secondary controller from 0 to 8 are routed to

arch/arm/plat-mxc/include/mach/irqs.h +1 -0

index fd9efb0..2fda5aa 100644
@@ -54,6 +54,7 @@
 /* REVISIT: Add IPU irqs on IMX51 */
 
 #define NR_IRQS			(MXC_IPU_IRQ_START + MX3_IPU_IRQS)
+#define TZIC_NUM_IRQS		128
 
 extern int imx_irq_set_priority(unsigned char irq, unsigned char prio);
 

arch/arm/plat-mxc/tzic.c +0 -2

index 98308ec..98b23b8 100644
@@ -50,8 +50,6 @@
 
 void __iomem *tzic_base; /* Used as irq controller base in entry-macro.S */
 
-#define TZIC_NUM_IRQS 128
-
 #ifdef CONFIG_FIQ
 static int tzic_set_irq_fiq(unsigned int irq, unsigned int type)
 {

include/linux/irqdomain.h +22 -5

index bd4272b..1538d4c 100644
@@ -15,6 +15,7 @@
 #ifndef _LINUX_IRQDOMAIN_H
 #define _LINUX_IRQDOMAIN_H
 
+#include <linux/err.h>
 #include <linux/irq.h>
 #include <linux/mod_devicetable.h>
 

@@ -96,12 +97,28 @@ extern struct irq_domain_ops irq_domain_simple_ops;
 #endif /* CONFIG_IRQ_DOMAIN */
 
 #if defined(CONFIG_IRQ_DOMAIN) && defined(CONFIG_OF_IRQ)
-extern void irq_domain_add_simple(struct device_node *controller, int irq_base);
-extern void irq_domain_generate_simple(const struct of_device_id *match,
-					u64 phys_base, unsigned int irq_start);
+extern struct irq_domain *irq_domain_add_simple(struct device_node *controller,
+						unsigned int irq_base,
+						unsigned int nr_irq);
+extern struct irq_domain *irq_domain_generate_simple(const struct of_device_id *match,
+						     u64 phys_base,
+						     unsigned int irq_start,
+						     unsigned int nr_irq);
 #else /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */
-static inline void irq_domain_generate_simple(const struct of_device_id *match,
-					u64 phys_base, unsigned int irq_start) { }
+static inline struct irq_domain *irq_domain_add_simple(struct device_node *controller,
+						       int irq_base,
+						       unsigned int nr_irq)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
+static inline struct irq_domain *irq_domain_generate_simple(const struct of_device_id *match,
+							    u64 phys_base,
+							    unsigned int irq_start,
+							    unsigned int nr_irq)
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif /* CONFIG_IRQ_DOMAIN && CONFIG_OF_IRQ */
 
 #endif /* _LINUX_IRQDOMAIN_H */

kernel/irq/irqdomain.c +19 -11

index 1f9e265..807c44b 100644
@@ -147,36 +147,44 @@ int irq_domain_simple_dt_translate(struct irq_domain *d,
 }
 
 /**
- * irq_domain_create_simple() - Set up a 'simple' translation range
+ * irq_domain_add_simple() - Set up a 'simple' translation range
  */
-void irq_domain_add_simple(struct device_node *controller, int irq_base)
+struct irq_domain *irq_domain_add_simple(struct device_node *controller,
+					 unsigned int irq_base,
+					 unsigned int nr_irq)
 {
 	struct irq_domain *domain;
 
 	domain = kzalloc(sizeof(*domain), GFP_KERNEL);
-	if (!domain) {
-		WARN_ON(1);
-		return;
-	}
+	if (!domain)
+		return ERR_PTR(-ENOMEM);
 
 	domain->irq_base = irq_base;
+	domain->nr_irq = nr_irq;
 	domain->of_node = of_node_get(controller);
 	domain->ops = &irq_domain_simple_ops;
 	irq_domain_add(domain);
+
+	return domain;
 }
 EXPORT_SYMBOL_GPL(irq_domain_add_simple);
 
-void irq_domain_generate_simple(const struct of_device_id *match,
-				u64 phys_base, unsigned int irq_start)
+struct irq_domain *irq_domain_generate_simple(const struct of_device_id *match,
+					      u64 phys_base,
+					      unsigned int irq_start,
+					      unsigned int nr_irq)
 {
+	struct irq_domain *domain = ERR_PTR(-ENODEV);
 	struct device_node *node;
-	pr_info("looking for phys_base=%llx, irq_start=%i\n",
-		(unsigned long long) phys_base, (int) irq_start);
+	pr_info("looking for phys_base=%llx, irq_start=%u, nr_irq=%u\n",
+		(unsigned long long) phys_base, irq_start, nr_irq);
 	node = of_find_matching_node_by_address(NULL, match, phys_base);
 	if (node)
-		irq_domain_add_simple(node, irq_start);
+		domain = irq_domain_add_simple(node, irq_start, nr_irq);
 	else
 		pr_info("no node found\n");
+
+	return domain;
 }
 EXPORT_SYMBOL_GPL(irq_domain_generate_simple);
 #endif /* CONFIG_OF_IRQ */

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

Signed-off-by Thierry Reding <[email protected]>

---
?/**
- * irq_domain_create_simple() - Set up a 'simple' translation range
+ * irq_domain_add_simple() - Set up a 'simple' translation range ?*/

kernel/irq/irqdomain.c +3 -1

-void irq_domain_add_simple(struct device_node *controller, int irq_base)
+struct irq_domain *irq_domain_add_simple(struct device_node *controller,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int irq_base,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int nr_irq)

?{
? ? ? ?struct irq_domain *domain;

? ? ? ?domain = kzalloc(sizeof(*domain), GFP_KERNEL);
- ? ? ? if (!domain) {
- ? ? ? ? ? ? ? WARN_ON(1);
- ? ? ? ? ? ? ? return;
- ? ? ? }
+ ? ? ? if (!domain)
+ ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);

Hi Thierry,

Quoted 3 lines · Thierry Reding

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

The commit text should also include the justification for renaming irq_domain_create_simple() -> irq_domain_add_simple()

Quoted 7 lines

? ? ? ?domain = kzalloc(sizeof(*domain), GFP_KERNEL);
- ? ? ? if (!domain) {
- ? ? ? ? ? ? ? WARN_ON(1);
- ? ? ? ? ? ? ? return;
- ? ? ? }
+ ? ? ? if (!domain)
+ ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);

Don't use the ERR_PTR() pattern (it's a horrible pattern IMHO).

Return NULL on failure and keep the WARN_ON() in this function. Otherwise looks good. Thanks for writing this patch.

Quoted 8 lines · Grant Likely

Hi Thierry,

Quoted 3 lines · Thierry Reding

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

Quoted 2 lines

The commit text should also include the justification for renaming irq_domain_create_simple() -> irq_domain_add_simple()

Actually the commit only fixes up the comment. The function has always been called irq_domain_add_simple().

For reference, this was introduced in commit 7e71330.

Quoted 9 lines

Quoted 8 lines

? ? ? ?domain = kzalloc(sizeof(*domain), GFP_KERNEL);
- ? ? ? if (!domain) {
- ? ? ? ? ? ? ? WARN_ON(1);
- ? ? ? ? ? ? ? return;
- ? ? ? }
+ ? ? ? if (!domain)
+ ? ? ? ? ? ? ? return ERR_PTR(-ENOMEM);

Quoted 1 line

Don't use the ERR_PTR() pattern (it's a horrible pattern IMHO).

Returning NULL here is probably okay. Can the ERR_PTR stay in irq_domain_generate_simple(), though? It has two error conditions and handling both by returning NULL may not be what we want.

No. ERR_PTR is a horrible pattern because you cannot tell by looking at a prototype that returns a pointer whether or not the correct failure test is "if (!ptr)" or "if (IS_ERR(ptr))". Unless it is absolutely critical for an error code to be returned (which isn't the case here) I will not accept new code that uses ERR_PTR().

In this case, if irq_domain_add_simple() fails, then something is very wrong. I'd much rather the routine complain loudly regardless of the error condition.

Actually, looking again at irq_domain_generate_simple() it should probably succeed even if it cannot find a matching node since an irq_domain does more than just device tree translation. Although, irq_domain_generate_simple() is a stop-gap solution that will eventually be removed.

arch/arm/mach-msm/board-msm8x60.c +0 -0

index 0a11342..a50c7e2 100644
@@ -84,8 +84,11 @@ static void __init msm8x60_dt_init(void)
 
 	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
 			MSM8X60_QGIC_DIST_PHYS);

Quoted 8 lines · Thierry Reding

arch/arm/mach-msm/board-msm8x60.c +0 -0

index 0a11342..a50c7e2 100644
@@ -84,8 +84,11 @@ static void __init msm8x60_dt_init(void)
 
 	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
 			MSM8X60_QGIC_DIST_PHYS);

arch/arm/mach-msm/board-msm8x60.c static void __init msm8x60_dt_init(void) · line 94

Quoted 10 lines

arch/arm/mach-msm/board-msm8x60.c +5 -2

-	if (node)
-		irq_domain_add_simple(node, GIC_SPI_START);
+	if (node) {
+		struct irq_domain *domain = irq_domain_add_simple(node,
+				GIC_SPI_START, NR_MSM_IRQS);
+		WARN_ON(IS_ERR(domain));
+	}
 
 	if (of_machine_is_compatible("qcom,msm8660-surf")) {
 		printk(KERN_INFO "Init surf UART registers\n");

This is probably a consequence of MSM not really being "simple", but just using that. However, NR_MSM_IRQS is only the number of IRQs on the MSM core. There are also GPIO irqs, and potentially board IRQs (the board has an I2C-based chip with a bunch of IRQ lines on it).

The only define that captures this now is 'NR_IRQS', even though we're trying to get rid of that.

Ultimately, the correct answer will be to get the various interrupt controllers using their own domains,

but for now, this needs to be a larger value to avoid missing a bunch of the interrupts.

Quoted 8 lines · Thierry Reding

arch/arm/mach-msm/board-msm8x60.c +0 -0

index 0a11342..a50c7e2 100644
@@ -84,8 +84,11 @@ static void __init msm8x60_dt_init(void)
 
 	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
 			MSM8X60_QGIC_DIST_PHYS);

arch/arm/mach-msm/board-msm8x60.c static void __init msm8x60_dt_init(void) · line 94

Quoted 10 lines

arch/arm/mach-msm/board-msm8x60.c +5 -2

-	if (node)
-		irq_domain_add_simple(node, GIC_SPI_START);
+	if (node) {
+		struct irq_domain *domain = irq_domain_add_simple(node,
+				GIC_SPI_START, NR_MSM_IRQS);
+		WARN_ON(IS_ERR(domain));
+	}
 
 	if (of_machine_is_compatible("qcom,msm8660-surf")) {
 		printk(KERN_INFO "Init surf UART registers\n");

This is probably a consequence of MSM not really being "simple", but just using that. However, NR_MSM_IRQS is only the number of IRQs on the MSM core. There are also GPIO irqs, and potentially board IRQs (the board has an I2C-based chip with a bunch of IRQ lines on it).

The only define that captures this now is 'NR_IRQS', even though we're trying to get rid of that.

Ultimately, the correct answer will be to get the various interrupt controllers using their own domains, but for now, this needs to be a larger value to avoid missing a bunch of the interrupts.

Quoted 25 lines · David Brown

Quoted 8 lines · Thierry Reding

arch/arm/mach-msm/board-msm8x60.c +0 -0

index 0a11342..a50c7e2 100644
@@ -84,8 +84,11 @@ static void __init msm8x60_dt_init(void)
 
 	node = of_find_matching_node_by_address(NULL, msm_dt_gic_match,
 			MSM8X60_QGIC_DIST_PHYS);

arch/arm/mach-msm/board-msm8x60.c static void __init msm8x60_dt_init(void) · line 94

Quoted 10 lines

arch/arm/mach-msm/board-msm8x60.c +5 -2

-	if (node)
-		irq_domain_add_simple(node, GIC_SPI_START);
+	if (node) {
+		struct irq_domain *domain = irq_domain_add_simple(node,
+				GIC_SPI_START, NR_MSM_IRQS);
+		WARN_ON(IS_ERR(domain));
+	}
 
 	if (of_machine_is_compatible("qcom,msm8660-surf")) {
 		printk(KERN_INFO "Init surf UART registers\n");

This is probably a consequence of MSM not really being "simple", but just using that. However, NR_MSM_IRQS is only the number of IRQs on the MSM core. There are also GPIO irqs, and potentially board IRQs (the board has an I2C-based chip with a bunch of IRQ lines on it).

Quoted 6 lines

The only define that captures this now is 'NR_IRQS', even though we're trying to get rid of that.

Ultimately, the correct answer will be to get the various interrupt controllers using their own domains, but for now, this needs to be a larger value to avoid missing a bunch of the interrupts.

No. This should only be the number of interrupts for a controller as the interrupt numbers in the device tree should be relative to a controller and not the Linux virq number. The numbering in the dts needs to be correct. You don't need a domain until you start getting the interrupts from the dts.

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

Hi Thierry,

Quoted 3 lines · Thierry Reding

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

[...]

arch/arm/mach-omap2/board-generic.c static struct of_device_id intc_match[] __initdata = { · line 71

Quoted 13 lines

arch/arm/mach-omap2/board-generic.c +3 -2

index d587560..bf67781 100644
@@ -66,8 +66,11 @@ static struct of_device_id intc_match[] __initdata = {
 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);
+	if (node) {
+		struct irq_domain *domain;
+		domain = irq_domain_add_simple(node, 0, INTCPS_NR_IRQS);

The number of interrupts will depend on the OMAP generation. That one is just valid for the 3430 INTC controller.
Since the previous code was using zero, I guess that using 0 there should be fine.

Moreover, that piece of code should not exist anymore on 3.3 if the series I sent last month to leverage Rob's DT interrupt init is merged [1].

I've just ping Rob and Grant on that series to get a status.

The irq_domain_add() function needs the number of interrupts in the domain to properly initialize them. In addition the allocated domain is now returned by the irq_domain_{add,generate}_simple() helpers.

Signed-off-by Thierry Reding <[email protected]>

---

 arch/arm/mach-imx/imx51-dt.c          |   13 +++++++++++--
 arch/arm/mach-imx/imx53-dt.c          |   13 +++++++++++--
 arch/arm/mach-imx/mach-imx6q.c        |    4 +++-

 arch/arm/plat-mxc/include/mach/irqs.h |    1 +
 arch/arm/plat-mxc/tzic.c              |    2 --

arch/arm/mach-imx/imx51-dt.c +11 -2

index e6bad17..72bf94c 100644
@@ -47,7 +47,12 @@ static const struct of_dev_auxdata imx51_auxdata_lookup[] __initconst = {
 static int __init imx51_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	irq_domain_add_simple(np, 0);
+	struct irq_domain *domain;
+
+	domain = irq_domain_add_simple(np, 0, TZIC_NUM_IRQS);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
+
 	return 0;
 }
 

@@ -55,9 +60,13 @@ static int __init imx51_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 
 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
 
 	return 0;
 }

arch/arm/mach-imx/imx53-dt.c +11 -2

index 05ebb3e..ccae620 100644
@@ -51,7 +51,12 @@ static const struct of_dev_auxdata imx53_auxdata_lookup[] __initconst = {
 static int __init imx53_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	irq_domain_add_simple(np, 0);
+	struct irq_domain *domain;
+
+	domain = irq_domain_add_simple(np, 0, TZIC_NUM_IRQS);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
+
 	return 0;
 }
 

@@ -59,9 +64,13 @@ static int __init imx53_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 
 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	if (IS_ERR(domain))
+		return PTR_ERR(domain);
 
 	return 0;
 }

arch/arm/mach-imx/mach-imx6q.c +0 -0

index c257281..9ed7812 100644

arch/arm/mach-imx/mach-imx6q.c +0 -0

 	return 0;
 }

On Fri, Jan 06, 2012 at 03:28:25PM +0100, Thierry Reding wrote:

Quoted 10 lines

arch/arm/mach-imx/mach-imx6q.c +1 -0

index c257281..9ed7812 100644
@@ -95,9 +95,11 @@ static int __init imx6q_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
+	struct irq_domain *domain;
 

arch/arm/mach-imx/mach-imx6q.c static int __init imx6q_gpio_add_irq_domain(struct device_node *np, · line 102

Quoted 4 lines

arch/arm/mach-imx/mach-imx6q.c +2 -1

 	gpio_irq_base -= 32;
-	irq_domain_add_simple(np, gpio_irq_base);
+	domain = irq_domain_add_simple(np, gpio_irq_base, 32);
+	WARN_ON(IS_ERR(domain));

Why do we handle the error in a different pattern that is used for all above?

I just made a small change on top of yours. Can you please consider to amend it to your patch if it looks sane to you?

arch/arm/plat-mxc/include/mach/irqs.h +7 -6

index 2fda5aa..70376e0 100644
@@ -13,19 +13,20 @@
 
 #include <asm-generic/gpio.h>
 
+#define GIC_NUM_IRQS           160
+#define TZIC_NUM_IRQS          128
+#define AVIC_NUM_IRQS          64
+
 /*
- * SoCs with GIC interrupt controller have 160 IRQs, those with TZIC
- * have 128 IRQs, and those with AVIC have 64.
- *
  * To support single image, the biggest number should be defined on
  * top of the list.
  */
 #if defined CONFIG_ARM_GIC
-#define MXC_INTERNAL_IRQS      160
+#define MXC_INTERNAL_IRQS      GIC_NUM_IRQS
 #elif defined CONFIG_MXC_TZIC
-#define MXC_INTERNAL_IRQS      128
+#define MXC_INTERNAL_IRQS      TZIC_NUM_IRQS
 #else
-#define MXC_INTERNAL_IRQS      64
+#define MXC_INTERNAL_IRQS      AVIC_NUM_IRQS
 #endif
 
 #define MXC_GPIO_IRQ_START     MXC_INTERNAL_IRQS