Hello, Jeff. Jeff Garzik wrote: > Alan Cox wrote: >> On Fri, 21 Nov 2008 13:13:06 +0900 >> Tejun Heo <tj@xxxxxxxxxx> 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. If we call ->irq_pending() only when no qc is in flight, I don't think there will be any noticeable performance penalty when the IRQ pending register is per-port. If pending status of all ports can be determined by single read, having a separate handler is a good idea. It all depends on how controllers actually implement them. All BMDMA controllers I know about are sata_sil (already has private irq handler) and ata_piix (this patch). Alan, how do other controllers do it? > 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. ->irq_clear() is tightly bound to ->irq_pending(). Drivers which don't support ->irq_pending() probably wouldn't support or need ->irq_clear() either, but still, I can't think of one good location. What's on your mind? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
