[PATCH 6/6] ARM: gic: use handle_fasteoi_irq for SPIs
ccross at google.com
Sat Apr 30 12:42:47 EDT 2011
On Sat, Apr 30, 2011 at 2:54 AM, Thomas Gleixner <tglx at linutronix.de> wrote:
> On Fri, 29 Apr 2011, Colin Cross wrote:
>> > Cc: Abhijeet Dharmapurikar <adharmap at codeaurora.org>
>> > Cc: Russell King - ARM Linux <linux at arm.linux.org.uk>
>> > Acked-by: Catalin Marinas <catalin.marinas at arm.com>
>> > Signed-off-by: Will Deacon <will.deacon at arm.com>
>> After further testing, I'm having a problem with this patch, although
>> I think its more of a core issue than the fault of this patch. One of
>> my interrupts is getting stuck with the PENDING flag set, and
>> preventing suspend. The flow that triggers this is:
>> level triggered irq
>> schedule_work (because it takes a sleeping i2c transaction to
>> deassert the irq pin)
> If you'd use a threaded irq handler the IRQ_ONESHOT mechanism would
> handle that problem for you. It masks the irq line before calling the
> handler and unmask happens after the threaded handler has run.
> disable_irq_nosync from an interrupt handler plus scheduling work is
> the historic "threaded" interrupt handler mechanism. It's kinda murky
> nowadays due to the lazy irq disable mechanism.
Yes, and in the specific case that reproduces this problem for me a
threaded handler would be appropriate. However, there are cases where
disable irq can legitimately be called from an interrupt handler. One
example I was given is a debounce timer - the interrupt handler
disables the irq and then sets a timer.
>> same irq
>> mark irq pending
>> work function
>> causes level triggered irq to go low
>> check_irq_resend (returns immediately)
>> At this point, the irq is unmasked, but not being asserted, and marked
>> as pending. check_irq_resend doesn't clear the pending flag for level
>> triggered interrupts, so the pending flag will stay set until the next
>> time the interrupt occurs.
> Yes, that should be cleared unconditionally in check_irq_resend.
>> Should handle_fasteoi_irq mask the interrupt before eoi if it was
>> disabled by the interrupt handler? Otherwise, every level triggered
>> interrupt that is not deasserted by the interrupt handler will
>> interrupt the cpu twice. There is still the case where a driver
> No, we should stop doing the disable_irq_nosync from handlers and use
> threaded interrupts instead.
As in the example above, I don't think threaded interrupts cover all
the cases where disable_irq_nosync is used in an interrupt handler,
although 99% of the time they do. handle_level_irq already has this
optimization - it doesn't unmask the irq line if the handler returns
with the irq disabled. I think the patch I posted would need to check
for a level triggered interrupt, masking the line on an edge triggered
interrupt could lose the next interrupt.
>> disables the irq, the interrupt goes high, then goes low again before
>> enable_irq is called.
> So what you're saying is:
> irq ____ _______
> |________________| |_____________________
> | |
> ISR/mask/eoi enable_irq/unmask
> So after the unmask the asserted new interrupt is not delivered?
> That's not a software problem :)
Of course its not delivered, the problem is that it is marked pending,
check_wakeup_irqs returns true and prevents suspend. Always clearing
the pending flag in check_if_resend fixes suspend after calling
enable_irq, but suspend will still fail if attempted between when the
irq line goes low and enable_irq is called (which could be never if
the driver has been disabled for some reason).
More information about the linux-arm-kernel