DragonFly kernel List (threaded) for 2004-11
Re: ATA Patch #7 (Re: ATA Patch #6)
: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;
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?