Patchwork irqdomain: Initialize number of IRQs for simple domains

login
register
mail settings
Submitter Thierry Reding
Date Jan. 6, 2012, 2:28 p.m.
Message ID <1325860112-22051-1-git-send-email-thierry.reding@avionic-design.de>
Download mbox | patch
Permalink /patch/134645/
State New
Headers show

Comments

Thierry Reding - Jan. 6, 2012, 2:28 p.m.
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 <thierry.reding@avionic-design.de>
---
 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(-)
Nicolas Ferre - Jan. 6, 2012, 2:36 p.m.
On 01/06/2012 03:28 PM, 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.
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>

Hi Thierry,

Good to see this patch ;-)
Thanks for having done this modification.

For AT91 and irqdomain (as far as I can tell)

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Best regards,

> ---
>  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(-)
> 
> diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
> index bb6b434..12f003d 100644
> --- a/arch/arm/mach-at91/board-dt.c
> +++ b/arch/arm/mach-at91/board-dt.c
> @@ -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();
>  }
>  
> diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
> index e6bad17..72bf94c 100644
> --- a/arch/arm/mach-imx/imx51-dt.c
> +++ b/arch/arm/mach-imx/imx51-dt.c
> @@ -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;
>  }
> diff --git a/arch/arm/mach-imx/imx53-dt.c b/arch/arm/mach-imx/imx53-dt.c
> index 05ebb3e..ccae620 100644
> --- a/arch/arm/mach-imx/imx53-dt.c
> +++ b/arch/arm/mach-imx/imx53-dt.c
> @@ -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;
>  }
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index c257281..9ed7812 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -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;
>  }
> diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
> index 0a11342..a50c7e2 100644
> --- a/arch/arm/mach-msm/board-msm8x60.c
> +++ b/arch/arm/mach-msm/board-msm8x60.c
> @@ -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");
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index d587560..bf67781 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -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);
>  
> diff --git a/arch/arm/mach-prima2/irq.c b/arch/arm/mach-prima2/irq.c
> index d93ceef..d3c5136 100644
> --- a/arch/arm/mach-prima2/irq.c
> +++ b/arch/arm/mach-prima2/irq.c
> @@ -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);
>  
> diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
> index 02b7b93..6c40be4 100644
> --- a/arch/arm/mach-versatile/core.c
> +++ b/arch/arm/mach-versatile/core.c
> @@ -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
> diff --git a/arch/arm/plat-mxc/include/mach/irqs.h b/arch/arm/plat-mxc/include/mach/irqs.h
> index fd9efb0..2fda5aa 100644
> --- a/arch/arm/plat-mxc/include/mach/irqs.h
> +++ b/arch/arm/plat-mxc/include/mach/irqs.h
> @@ -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);
>  
> diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
> index 98308ec..98b23b8 100644
> --- a/arch/arm/plat-mxc/tzic.c
> +++ b/arch/arm/plat-mxc/tzic.c
> @@ -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)
>  {
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index bd4272b..1538d4c 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -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 */
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 1f9e265..807c44b 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -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 */
Grant Likely - Jan. 6, 2012, 3:04 p.m.
Hi Thierry,

On Fri, Jan 6, 2012 at 7:28 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> 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()

>
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---
>  /**
> - * 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);

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.

g.
David Brown - Jan. 6, 2012, 4:07 p.m.
On Fri, Jan 06, 2012 at 03:28:25PM +0100, Thierry Reding wrote:
> diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
> index 0a11342..a50c7e2 100644
> --- a/arch/arm/mach-msm/board-msm8x60.c
> +++ b/arch/arm/mach-msm/board-msm8x60.c
> @@ -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");

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.

David
Thierry Reding - Jan. 6, 2012, 4:12 p.m.
* David Brown wrote:
> On Fri, Jan 06, 2012 at 03:28:25PM +0100, Thierry Reding wrote:
> > diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
> > index 0a11342..a50c7e2 100644
> > --- a/arch/arm/mach-msm/board-msm8x60.c
> > +++ b/arch/arm/mach-msm/board-msm8x60.c
> > @@ -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");
> 
> 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,

Yes.

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

Okay, I'll make it NR_IRQS then.

Thierry
Thierry Reding - Jan. 6, 2012, 4:20 p.m.
* Grant Likely wrote:
> Hi Thierry,
> 
> On Fri, Jan 6, 2012 at 7:28 AM, Thierry Reding
> <thierry.reding@avionic-design.de> wrote:
> > 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()

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.

> >        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).

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.

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

I thought it would be better to pull the WARN_ON out of the function because
we now actually have a way to determine if the call succeeded in the caller.
In most cases I assume the caller will be much better suited to handle the
situation gracefully such that the WARN_ON is not required.

Thierry
Rob Herring - Jan. 6, 2012, 4:26 p.m.
On 01/06/2012 10:07 AM, David Brown wrote:
> On Fri, Jan 06, 2012 at 03:28:25PM +0100, Thierry Reding wrote:
>> diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
>> index 0a11342..a50c7e2 100644
>> --- a/arch/arm/mach-msm/board-msm8x60.c
>> +++ b/arch/arm/mach-msm/board-msm8x60.c
>> @@ -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");
> 
> 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.

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.

Rob
Cousson, Benoit - Jan. 6, 2012, 4:58 p.m.
Hi Thierry,

On 1/6/2012 3:28 PM, Thierry Reding wrote:
> 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.

[...]

> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index d587560..bf67781 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -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.

Regards,
Benoit


[1] http://www.spinics.net/lists/linux-omap/msg62124.html
David Brown - Jan. 6, 2012, 6:52 p.m.
On Fri, Jan 06, 2012 at 10:26:17AM -0600, Rob Herring wrote:
> On 01/06/2012 10:07 AM, David Brown wrote:
> > On Fri, Jan 06, 2012 at 03:28:25PM +0100, Thierry Reding wrote:
> >> diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
> >> index 0a11342..a50c7e2 100644
> >> --- a/arch/arm/mach-msm/board-msm8x60.c
> >> +++ b/arch/arm/mach-msm/board-msm8x60.c
> >> @@ -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");
> > 
> > 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.
> 
> 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.

Yes, _should only be_.  And, you're probably right that we shouldn't
change this, and should use the NR_MSM_IRQS.  When a DTS device needs
to use a GPIO IRQ, the GPIO controller should properly register its
own domain.  The only IRQ in the DT is an MSM IRQ, so this doesn't
actually break anything.

So, Thierry, please leave it as NR_MSM_IRQS as in your original
patch.  In that form.

Acked-by: David Brown <davidb@codeaurora.org>
Grant Likely - Jan. 6, 2012, 9:34 p.m.
On Fri, Jan 06, 2012 at 05:20:16PM +0100, Thierry Reding wrote:
> * Grant Likely wrote:
> > Hi Thierry,
> > 
> > On Fri, Jan 6, 2012 at 7:28 AM, Thierry Reding
> > <thierry.reding@avionic-design.de> wrote:
> > > 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()
> 
> 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.

Hahaha.  Oops, you're right.  :-)

> 
> > >        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).
> 
> 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.

g.
Thierry Reding - Jan. 7, 2012, 11:40 a.m.
* Grant Likely wrote:
> 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.

Okay, I'll keep the WARN_ON(1) and simply return NULL.

> 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.

So I'll just handle errors in irq_domain_generate_simple() the same way as in
irq_domain_add_simple() and will return NULL if no matching node is found.

Thierry
Thierry Reding - Jan. 9, 2012, 9:03 a.m.
* Cousson, Benoit wrote:
> Hi Thierry,
> 
> On 1/6/2012 3:28 PM, Thierry Reding wrote:
> >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.
> 
> [...]
> 
> >diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> >index d587560..bf67781 100644
> >--- a/arch/arm/mach-omap2/board-generic.c
> >+++ b/arch/arm/mach-omap2/board-generic.c
> >@@ -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.

Okay, so I guess we should wait for you patch to go in first and I'll update
this patch on top of that. I assume we can coordinate things in linux-next?

Thierry

Patch

diff --git a/arch/arm/mach-at91/board-dt.c b/arch/arm/mach-at91/board-dt.c
index bb6b434..12f003d 100644
--- a/arch/arm/mach-at91/board-dt.c
+++ b/arch/arm/mach-at91/board-dt.c
@@ -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();
 }
 
diff --git a/arch/arm/mach-imx/imx51-dt.c b/arch/arm/mach-imx/imx51-dt.c
index e6bad17..72bf94c 100644
--- a/arch/arm/mach-imx/imx51-dt.c
+++ b/arch/arm/mach-imx/imx51-dt.c
@@ -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;
 }
diff --git a/arch/arm/mach-imx/imx53-dt.c b/arch/arm/mach-imx/imx53-dt.c
index 05ebb3e..ccae620 100644
--- a/arch/arm/mach-imx/imx53-dt.c
+++ b/arch/arm/mach-imx/imx53-dt.c
@@ -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;
 }
diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index c257281..9ed7812 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -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;
 }
diff --git a/arch/arm/mach-msm/board-msm8x60.c b/arch/arm/mach-msm/board-msm8x60.c
index 0a11342..a50c7e2 100644
--- a/arch/arm/mach-msm/board-msm8x60.c
+++ b/arch/arm/mach-msm/board-msm8x60.c
@@ -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");
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index d587560..bf67781 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -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);
 
diff --git a/arch/arm/mach-prima2/irq.c b/arch/arm/mach-prima2/irq.c
index d93ceef..d3c5136 100644
--- a/arch/arm/mach-prima2/irq.c
+++ b/arch/arm/mach-prima2/irq.c
@@ -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);
 
diff --git a/arch/arm/mach-versatile/core.c b/arch/arm/mach-versatile/core.c
index 02b7b93..6c40be4 100644
--- a/arch/arm/mach-versatile/core.c
+++ b/arch/arm/mach-versatile/core.c
@@ -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
diff --git a/arch/arm/plat-mxc/include/mach/irqs.h b/arch/arm/plat-mxc/include/mach/irqs.h
index fd9efb0..2fda5aa 100644
--- a/arch/arm/plat-mxc/include/mach/irqs.h
+++ b/arch/arm/plat-mxc/include/mach/irqs.h
@@ -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);
 
diff --git a/arch/arm/plat-mxc/tzic.c b/arch/arm/plat-mxc/tzic.c
index 98308ec..98b23b8 100644
--- a/arch/arm/plat-mxc/tzic.c
+++ b/arch/arm/plat-mxc/tzic.c
@@ -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)
 {
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index bd4272b..1538d4c 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -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 */
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 1f9e265..807c44b 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -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 */