Messages in this thread | | | Date | Tue, 29 Apr 2008 15:08:46 +0200 | From | Uwe Kleine-König <> | Subject | Re: [PATCH] let setup_irq reenable a shared irq |
| |
Hello,
Thomas Gleixner wrote: > On Mon, 28 Apr 2008, Uwe Kleine-König wrote: > > > Oh no. There is lots of code in drivers, which does: > > > > > > disable_irq(); > > > do_some_protected_stuff(); > > > enable_irq(); > > > > > > So when the second driver is loaded on another CPU it would see the > > > IRQ_DISABLED bit set and unconditionally reenable the interrupt. > > > > > > This unprotects the protected operation and definitely triggers the > > > WARN_ON() in enable_irq() where we check for desc->depth == 0. > > mmpf. > > > > It's not nice to use disable_irq()/enable_irq() in a driver, is it? > > Well, it's not nice, but it's there (in rather large quantities) Ah, and now I finally understood desc->depth ...
> Subject: genirq: reenable a nobody cared disabled irq when a new driver arrives > From: Thomas Gleixner <tglx@linutronix.de> > Date: Mon, 28 Apr 2008 17:01:56 +0200 > > Uwe Kleine-Koenig has some strange hardware where one of the shared > interrupts can be asserted during boot before the appropriate driver > loads. Requesting the shared irq line from another driver results in a > spurious interrupt storm which finally disables the interrupt line. > > I have seen similar behaviour on resume before (the hardware does not > work anymore so I can not verify) and this spurious irq issue is > raised on a regular base in bugreports. > > Change the spurious disable logic to increment the disable depth and > mark the interrupt with an extra flag which allows us to reenable the > interrupt when a new driver arrives which requests the same irq > line. In the worst case this will disable the irq again via the > spurious trap, but there is a decent chance that the new driver is the > one which can handle the already asserted interrupt and makes the box > usable again. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Acked-by: Ingo Molnar <mingo@elte.hu> > --- > include/linux/irq.h | 1 + > kernel/irq/manage.c | 49 ++++++++++++++++++++++++++++++++----------------- > kernel/irq/spurious.c | 4 ++-- > 3 files changed, 35 insertions(+), 19 deletions(-) > > Index: linux-2.6/include/linux/irq.h > =================================================================== > --- linux-2.6.orig/include/linux/irq.h > +++ linux-2.6/include/linux/irq.h > @@ -61,6 +61,7 @@ typedef void (*irq_flow_handler_t)(unsig > #define IRQ_WAKEUP 0x00100000 /* IRQ triggers system wakeup */ > #define IRQ_MOVE_PENDING 0x00200000 /* need to re-target IRQ destination */ > #define IRQ_NO_BALANCING 0x00400000 /* IRQ is excluded from balancing */ > +#define IRQ_SPURIOUS_DISABLED 0x00400000 /* IRQ was disabled by the spurious trap */ Is it intended that IRQ_NO_BALANCING == IRQ_SPURIOUS_DISABLED?
Other than that
Tested-and-Acked-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Best regards and thanks Uwe
-- Uwe Kleine-König, Software Engineer Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |