Hello, Albert. Just a few comments. Albert Lee wrote: > + /* prevent ata_interrupt() from reading > + * status when transfering data for safty > + */ > + if (in_wq) > + spin_lock_irqsave(ap->lock, flags); > + Can't we just remove in_wq and run the HSM the same way whether we're invoking it from the interrupt handler or work queue? The worker can grab and release the lock outside of the HSM function and HSM can just assume that it's running with lock held. Am I missing something? > @@ -5227,6 +5254,9 @@ irqreturn_t ata_interrupt (int irq, void > if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) && > (qc->flags & ATA_QCFLAG_ACTIVE)) > handled |= ata_host_intr(ap, qc); > + else > + /* ack possibly unexpected irq */ > + ata_chk_status(ap); I'm not so sure whether poking TF Status even when no command is flight is a good idea. The IRQ can be shared with a fairly busy device (e.g. gigabit ethernet) and we'll be reading the status register a lot for nothing. I think we shouldn't do anything as before if no command is in flight. Also, please split it into two separate patches. Both are pretty significant changes and I think each deserves separate patch. 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
