Re: [PATCH #upstraem-fixes] ata_piix: detect and clear spurious IRQs

[Jeff Garzik <jeff@garzik.org>]

Alan Cox wrote:
> On Fri, 21 Nov 2008 13:13:06 +0900
> Tejun Heo <tj@kernel.org> wrote:
> 
>> The DMA_IRQ bit in the bmdma status register is always set when IDEIRQ
>> is asserted allowing spurious IRQ detection.  Detect spurious IRQs and
>> clear them.  This protects ata_piix against nobody-cared which gets
>> reported not so rarely.
> 
> Various controllers have the ability to report the IRQ more reliably in
> similar fashion, should this not be part of ata_sff_interrupt with an
> optional ops->irq_pending call ?

Though I'm in general agreement with this sub-thread of opinions, I do 
note that, in the past, I have purposefully avoided an ->irq_pending.

I always felt the alternative -- writing a small irq handler function 
specifically for that controller -- was a much more flexible and 
powerful method of producing the desired result.

A separate interrupt function can, for example, perform an MMIO read to 
check irq-pending, outside of a spinlock.  ->irq_pending callback is a 
bit more constraining.

In terms of implementation, we could probably collapse all the 
non-controller-specific behavior into a single function call or macro, 
performed inside the custom interrupt handling routine.

	Jeff