On 23/05/14 16:00, Russell King - ARM Linux wrote:
On Fri, May 23, 2014 at 02:57:49PM +0100, Daniel Thompson wrote:
+void eoi_fiq(int fiq) +{
- struct irq_data *irq_data = irq_get_irq_data(fiq + fiq_start);
- struct irq_chip *chip = irq_data_get_irq_chip(irq_data);
- if (chip->irq_eoi)
chip->irq_eoi(irq_data);
+}
So what if the IRQ chip takes a spinlock? You can't take spinlocks in FIQ context... Who checks for that kind of stuff - just hoping that the IRQ driver doesn't take a spinlock sounds real fragile.
I'll certainly do a better code review before pushing on. I certainly did overlook a spinlock in the GIC code which (I think) is actually reachable on a Tegra platform. I'll have to do something about that.
I've spent a bit of time this week seeing whether lock dep can be provoked into triggering if it sees a spin lock in this code but that isn't turning out very well (it needs CONFIG_LOCKDEP_DEBUG, relies on lockdep believing the FIQ handling state isn't a hardirq and still doesn't work properly).
Fortunately architectures using EOI appear to be pretty rare (I count five in drivers/irqchip/ and three in arch/arm/) so traditional code review and yelling might be sufficient.
Daniel.