Re: ATA Patch #7 (Re: ATA Patch #6)

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:Here's another patch to be applied on top of ATA Patch #7(attached).
:I added ata_clear_interlock() in ata_dmastart() because it needs
:device interrupt enabled anyway. Here I'm assuming that the critical
:section can be nested(looking at thread2.h it can).
:Command-start delay can be changed via hw.ata.command_delay(default: 10).
:Negative value is ignored and silently reset to 0 (can we let sysctl(8)
:to reject bogus values?).

    You can use SYSCTL_PROC to control allowed values, but it may be
    easier just to leave it SYSCTL_INT and check at DELAY time, e.g.:

    delay = blahdelay;
    if (delay < 1)
	delay = 1;
    if (delay > 1000)
	delay = 1000;
    DELAY(delay);

    We are going to have to clean up the ata_clear_interlock() calls...
    we do not want to reenable the interlock in situations where no 
    interrupt is required to complete the command because otherwise
    if an interrupt does occur the system can livelock because we no
    longer unconditionally read the status register if we dont think the
    interrupt is valid.

    e.g. there are a bunch of places where you added ata_clear_interlock()
    where you shouldn't have.  Most of those need to be backed out.  We
    *ONLY* clear the interlock and enable the interrupt if the operation
    is one which requires an interrupt to complete.

    --

    Here is a posit... if writing to ALTSTATUS (which controls IDS and 
    other bits) is illegal during DMAC, is reading STATUS or ALTSTATUS
    also illegal during DMAC?  The spec seems to indicate that it is
    not illegal but I'm beginning to think that it may be.

    If it is then we need to add more code to the interrupt handler, though
    I'm not sure exactly what we would be able to do.. I guess we could
    check the DMA controller to see if it indicates that DMA has completed,
    but that doesn't help us with the ATA_S_ERROR case.

    --

    I've synchronized with your patch-7-take-2 so lets form a new basis
    relative to that.  Could you take another pass at the interlock calls
    you added and remove the ones for situations where no completion
    interrupt is expected?


						-Matt