Re: [PATCH v3 13/25] irq_domain: Remove 'new' irq_domain in favour of the ppc one

On Fri, Feb 03, 2012 at 03:48:09PM +0100, Cousson, Benoit wrote:

Hi Grant,

I finally had the time to rebase most of the OMAP3 and OMAP4 DT
patches on your latest irq_domain series and found a couple of
minors regressions that breaks OMAP3 boot.

On 1/27/2012 10:36 PM, Grant Likely wrote:

[...]

quoted

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index e04e04dd..aab236f 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c

@@ -263,8 +263,6 @@ struct twl_client {

 static struct twl_client twl_modules[TWL_NUM_SLAVES];

-static struct irq_domain domain;
-
 /* mapping the module id to slave id and base address */
 struct twl_mapping {
 	unsigned char sid;	/* Slave ID */

@@ -1225,14 +1223,8 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)

 	pdata->irq_base = status;
 	pdata->irq_end = pdata->irq_base + nr_irqs;
-
-	domain.irq_base = pdata->irq_base;
-	domain.nr_irq = nr_irqs;
-#ifdef CONFIG_OF_IRQ
-	domain.of_node = of_node_get(node);
-	domain.ops =&irq_domain_simple_ops;
-#endif
-	irq_domain_add(&domain);
+	irq_domain_add_legacy(node, nr_irqs, pdata->irq_base, 0,
+			&irq_domain_simple_ops);

This commit cannot build due to the missing last parameter.

And in fact you fixed that in the next commit (#14), but the will
break git bisect and anyway that fix does not really belong to this
commit.

[PATCH v3 14/25] irq_domain: Remove irq_domain_add_simple()

 	irq_domain_add_legacy(node, nr_irqs, pdata->irq_base, 0,
-			      &irq_domain_simple_ops);
+			      &irq_domain_simple_ops, NULL);

Good catch, thanks.  I'll update the series to fix that.

Moreover, it looks like this new irq_domain code is checking the
number of hwirq and is not as lazy as the previous one :-)

Because of that and because of the wrong number of IRQs I put for
the twl4030 :-(, it does not handle properly the children of the
twl4030 now and print a big warning at boot time due to the
following check.

  WARN_ON(hwirq < first_hwirq || hwirq >= first_hwirq + size)

Good! The warning is doing it's job.  :-)

In fact 8 was just the number for the core functionality, but that
chip does have some other interrupts for sub function like GPIOs and
power events.

Okay, I'll add this patch to the series before patch 13.