Jeff, replies to your comments here and I'll post another reply to post new patch.
Jeff Garzik wrote: > Tejun Heo wrote: > >> For example, ATA_PROT_NODATA is processed as follows. > > > You mean ATA_PROT_ATAPI_NODATA, I presume. > Yeap. >> Driver Controller >> >> ------------------------------------------------------------------------- >> 1. issue ATA_CMD_PACKET >> 2. Turns on BSY and sends H2D regs >> 3. atapi_packet_task poll for BSY >> 4. D2H regs FIS (!BSY, DRQ) >> 5. packet_task sends CDB >> 6. Turns on BSY and sends DATA FIS > > > but first PIO Setup FIS > Thanks for the correction. > >> 7. D2H regs FIS (!BSY, DRDY, INTR) >> 8. intr handler completes the cmd >> >> In step #3, we're waiting for BSY to clear - command is active and >> NIEN is clear. After step #4 but before #5, command is active, BSY >> clear and DRQ set, if an interrupt from the other port occurs here, it >> will incorrectly fail this qc causing EH to kick in for sense data. > > > Agreed, that is a problem w/ libata's ATAPI HSM. Two problems actually: > > (a) [what you found] at that point in the HSM, unexpected interrupts can > incorrectly complete a command > (b) [a 'todo' item'] at that point in the HSM, there may actually be an > expected interrupt, before which the CDB should not be sent, per > ATA/ATAPI-4 This is deprecated since ATA/ATAPI-4. Will we see this on SATA ATAPI devices? Maybe for PATA ATAPI's? > > >> The problem is that the current interrupt handler is unaware of the >> HSM state where NIEN and BSY are clear but interrupts are not >> expected. So, the problem is that the interrupt handler doesn't know >> enough about HSM state to be able to determine that the interrupt is >> not its. > > > Agreed. > > >> The flag I've mentioned is to tell just that to the >> interrupt handler. > > >> Otherwise, I think we can implement something like ap->ata_state and >> fully record where the HSM is, which would also be useful to implement >> interrupt-driven PIO. What do you think? > > > I tend to prefer ap->ata_state method, since I definitely would like to > implement interrupt-driven PIO. But if the patch is too ugly, maybe a > bit flag would be OK. I just went the flag way for the following reason. As the larger chunk (PIO state machine) is not in place yet, implementing small part would look out of place and it's gonna need be modified anyway later when actual state machine is implemented. I've added a FIXME: comment after the flag to signify that this should be merged into state machine. > > Long term, PIO w/ interrupts is probably the best thing to do under > SATA. That prevents problems with PATA->SATA emulation that lead to > screaming interrupts. For SATA, it seems like polling is a bad idea in > all cases [save suspend/resume edge cases]. > > SATA technology is inherently more suited to an interrupt-driven > architecture anyway. Polling makes less sense, and needlessly increases > PCI bus traffic. I liked the simplicity of polling and, as PIO is not a very hot path, thought it would be okay, but I agree that implementing full interrupt driven HSM is a better thing to do. Thanks. -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
