Hi, On Tue, Mar 11, 2014 at 04:51:00PM +0100, Hans de Goede wrote: > SUN4I_IRQ_VECTOR_REG containing 0 can mean one of 2 things: > 1) irq 0 pending > 2) no more irqs pending > > So we must loop always atleast once to make irq 0 work, otherwise irq 0 > will never get serviced and we end up with a hard hang because > sun4i_handle_irq gets re-entered constantly. > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > drivers/irqchip/irq-sun4i.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c > index a5438d8..3761bf1 100644 > --- a/drivers/irqchip/irq-sun4i.c > +++ b/drivers/irqchip/irq-sun4i.c > @@ -140,10 +140,16 @@ static asmlinkage void __exception_irq_entry sun4i_handle_irq(struct pt_regs *re > { > u32 irq, hwirq; > > + /* > + * hwirq == 0 can mean one of 2 things: > + * 1) irq 0 pending > + * 2) no more irqs pending 3) spurious interrupt. > + * So loop always atleast once to make irq 0 work. > + */ > hwirq = readl(sun4i_irq_base + SUN4I_IRQ_VECTOR_REG) >> 2; > - while (hwirq != 0) { > + do { I'd at least lookup in the pending register to see if the interrupt 0 was actually triggered. Otherwise, you could end up with spurious handler calls on the interrupt 0. > irq = irq_find_mapping(sun4i_irq_domain, hwirq); > handle_IRQ(irq, regs); > hwirq = readl(sun4i_irq_base + SUN4I_IRQ_VECTOR_REG) >> 2; And you end up with the same issue if there's a first != 0 interrupt, and then the interrupt 0. What about something like: while (1) { hwirq = readl(sun4i_irq_base + SUN4I_IRQ_VECTOR_REG) >> 2; if (!hwirq) if (!(readl(sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0)) & BIT(0))) break; irq = irq_find_mapping(sun4i_irq_domain, hwirq); handle_IRQ(irq, regs); } -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature