Hello, Albert Lee wrote: > Patch 5/9: > > This patch changes polling codes to use freeze()/thaw() for irq off/on. > ata_qc_set_polling() is also removed since now unused. > > The reason for freeze()/thaw(): some ATAPI devices raises INTRQ even if nIEN = 1. > Using the host adapter irq mask mechanism, if available, is more reliable than using the device nIEN bit. > > Considerations: > 1. the semantic of freeze()/thaw() maybe more than irq off/on? It usually is irq off/on. > 2. HSM, the new user of freeze()/thaw(), will call freeze()/thaw() more frequently than EH. > Can current implemention of freeze()/thaw() handle it? Yeah, I don't see any reason why it can't be used that way but please audit each freeze/thaw implementation. > @@ -5442,7 +5442,7 @@ unsigned int ata_qc_issue_prot(struct at > > case ATA_PROT_PIO: > if (qc->tf.flags & ATA_TFLAG_POLLING) > - ata_qc_set_polling(qc); > + ap->ops->freeze(ap); > > ata_tf_to_host(ap, &qc->tf); Wouldn't ata_tf_load() change the ctl value to qc->tf.ctl when issuing the command? I'm not really sure about this change because most controller which have dedicated IRQ mask bit also have dedicated IRQ pending bit (and probably IRQ clear bit too). With those bits, IRQ during polling is not a big problem (sata_sil for example). So, the change doesn't really help the controllers which actually have problems dealing with devices which raise IRQ regardless of ATA_NIEN. I don't see any easy way out other than manipulating host IRQ on/off as IDE does. If we choose to go that way, we can use them in ata_bmdma_freeze/thaw (they should be named ata_sff_freeze/thaw really) and use this patch on top of it. Maybe it's better to drop ->freeze() and ->thaw() altogether and use ->irq_on() and ->irq_off() instead. Their names are more intuitive IMHO but that's minor. Jeff, Alan, what do you think about adopting IDE's irq off/on mechanism. It's ugly and may cause some problems if the IRQ is shared but then again it has been used for a long time without too much problem and occasional lost IRQ is much better than dead machine due to screaming IRQs followed by nobody cared. 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
