[PATCH] mfd wm8350: allocate irq descs dynamically

[PATCH] mfd wm8350: allocate irq descs dynamically

[Sascha Hauer]

This allows boards to leave the irq_base field unitialized and
prevents them having to reserve irqs in the platform.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mfd/wm8350-irq.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

As this seems to be the first user of irq_alloc_descs outside the core I
added Thomas to cc to check if this is the correct usage of this
function.
Unfortunately I have no hardware to test this. I created this patch
since one of the wm8350 users is the mx31ads board which prevents me
from getting rid of the irq ifdeffery in i.MX land, so it would be nice
if someone could give it a test run.

diff --git a/drivers/mfd/wm8350-irq.c b/drivers/mfd/wm8350-irq.c
index ed4b22a..04408a5 100644
--- a/drivers/mfd/wm8350-irq.c
+++ b/drivers/mfd/wm8350-irq.c
@@ -479,8 +479,8 @@ int wm8350_irq_init(struct wm8350 *wm8350, int irq,
 		return 0;
 	}
 
-	if (!pdata || !pdata->irq_base) {
-		dev_warn(wm8350->dev, "No interrupt support, no IRQ base\n");
+	if (!pdata) {
+		dev_warn(wm8350->dev, "No interrupt support, no platform data\n");
 		return 0;
 	}
 
@@ -500,7 +500,17 @@ int wm8350_irq_init(struct wm8350 *wm8350, int irq,
 
 	mutex_init(&wm8350->irq_lock);
 	wm8350->chip_irq = irq;
-	wm8350->irq_base = pdata->irq_base;
+
+	if (!pdata->irq_base) {
+		wm8350->irq_base = irq_alloc_descs(-1, 0, ARRAY_SIZE(wm8350_irqs), 0);
+		if (wm8350->irq_base < 0) {
+			dev_warn(wm8350->dev, "Allocating irqs failed with %d\n",
+				wm8350->irq_base);
+			return 0;
+		}
+	} else {
+		wm8350->irq_base = pdata->irq_base;
+	}
 
 	if (pdata->irq_high) {
 		flags |= IRQF_TRIGGER_HIGH;
-- 
1.7.4.1

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mfd wm8350: allocate irq descs dynamically

[Mark Brown]

On Thu, May 19, 2011 at 08:56:48PM +0200, Sascha Hauer wrote:

> -	if (!pdata || !pdata->irq_base) {
> -		dev_warn(wm8350->dev, "No interrupt support, no IRQ base\n");
> +	if (!pdata) {
> +		dev_warn(wm8350->dev, "No interrupt support, no platform data\n");

This isn't terribly good, the only reason we're checking pdata here is
because we can't dereference it to look up irq_base if it's not there.

Re: [PATCH] mfd wm8350: allocate irq descs dynamically

[Mark Brown]

On Thu, May 19, 2011 at 08:56:48PM +0200, Sascha Hauer wrote:

> +	if (!pdata->irq_base) {
> +		wm8350->irq_base = irq_alloc_descs(-1, 0, ARRAY_SIZE(wm8350_irqs), 0);
> +		if (wm8350->irq_base < 0) {

One other thing - it doesn't seem to be 100% desirable to making the
allocation of the IRQ descriptors depend on not specifying a base - for
many situations we're likely to want to know what the numbers we end up
with are (eg, for passing to another device) but if we don't call
irq_alloc_decs() the platform still has to arrange for the descriptors
to be there in advance.  It feels like the code should always use
irq_alloc_descs(), though obviously that's not going to work right now.

[PATCH] mfd wm8350: allocate irq descs dynamically

[Sascha Hauer]

This allows boards to leave the irq_base field unitialized and
prevents them having to reserve irqs in the platform.
pdata can be optional for irq support now. Without pdata the
driver allocates some free irq range. With pdata and irq_base > 0
the driver allocates exactly the specified irq.
Without pdata the irq defaults to IRQF_TRIGGER_LOW.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mfd/wm8350-irq.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/wm8350-irq.c b/drivers/mfd/wm8350-irq.c
index ed4b22a..8a1fafd 100644
--- a/drivers/mfd/wm8350-irq.c
+++ b/drivers/mfd/wm8350-irq.c
@@ -473,17 +473,13 @@ int wm8350_irq_init(struct wm8350 *wm8350, int irq,
 {
 	int ret, cur_irq, i;
 	int flags = IRQF_ONESHOT;
+	int irq_base = -1;
 
 	if (!irq) {
 		dev_warn(wm8350->dev, "No interrupt support, no core IRQ\n");
 		return 0;
 	}
 
-	if (!pdata || !pdata->irq_base) {
-		dev_warn(wm8350->dev, "No interrupt support, no IRQ base\n");
-		return 0;
-	}
-
 	/* Mask top level interrupts */
 	wm8350_reg_write(wm8350, WM8350_SYSTEM_INTERRUPTS_MASK, 0xFFFF);
 
@@ -502,7 +498,17 @@ int wm8350_irq_init(struct wm8350 *wm8350, int irq,
 	wm8350->chip_irq = irq;
 	wm8350->irq_base = pdata->irq_base;
 
-	if (pdata->irq_high) {
+	if (pdata && pdata->irq_base > 0)
+		irq_base = pdata->irq_base;
+
+	wm8350->irq_base = irq_alloc_descs(irq_base, 0, ARRAY_SIZE(wm8350_irqs), 0);
+	if (wm8350->irq_base < 0) {
+		dev_warn(wm8350->dev, "Allocating irqs failed with %d\n",
+			wm8350->irq_base);
+		return 0;
+	}
+
+	if (pdata && pdata->irq_high) {
 		flags |= IRQF_TRIGGER_HIGH;
 
 		wm8350_set_bits(wm8350, WM8350_SYSTEM_CONTROL_1,
-- 
1.7.5.3

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] mfd wm8350: allocate irq descs dynamically

[Mark Brown]

On Thu, Jun 02, 2011 at 01:45:02PM +0200, Sascha Hauer wrote:
> 
> This allows boards to leave the irq_base field unitialized and
> prevents them having to reserve irqs in the platform.
> pdata can be optional for irq support now. Without pdata the
> driver allocates some free irq range. With pdata and irq_base > 0
> the driver allocates exactly the specified irq.
> Without pdata the irq defaults to IRQF_TRIGGER_LOW.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH] mfd wm8350: allocate irq descs dynamically

[Mark Brown]

On Thu, Jun 02, 2011 at 12:53:49PM +0100, Mark Brown wrote:
> On Thu, Jun 02, 2011 at 01:45:02PM +0200, Sascha Hauer wrote:
> > 
> > This allows boards to leave the irq_base field unitialized and
> > prevents them having to reserve irqs in the platform.
> > pdata can be optional for irq support now. Without pdata the
> > driver allocates some free irq range. With pdata and irq_base > 0
> > the driver allocates exactly the specified irq.
> > Without pdata the irq defaults to IRQF_TRIGGER_LOW.

> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Actually I take that back, it looks like the wm831x equivalent patch is
causing some sort of semi-intermittent issue with the IRQ infrastructure
on non-sparse systems so I suspect this patch is also problematic.  The
major symptom is lockups in softirq handling.

Re: [PATCH] mfd wm8350: allocate irq descs dynamically

[Mark Brown]

On Thu, Jun 02, 2011 at 02:37:44PM +0100, Mark Brown wrote:

> Actually I take that back, it looks like the wm831x equivalent patch is
> causing some sort of semi-intermittent issue with the IRQ infrastructure
> on non-sparse systems so I suspect this patch is also problematic.  The
> major symptom is lockups in softirq handling.

I figured this out - it's not an issue with this code so I reinstate my
ack, sorry for the noise!