| From: | YONETANI Tomokazu <qhwt+dfly@xxxxxxxxxx> |
| Date: | Tue, 30 Nov 2004 22:34:09 +0900 |
On Mon, Nov 29, 2004 at 10:40:36AM -0800, Matthew Dillon wrote: > > :Ok, it may be too early to speak, but if I understand the documents > :correctly, the device control register should not be accessed when > :DMACK is assert -- that is, you can't disable device interrupt after > :ata_dmastart(), right? Here's a small modification to ATA Patch #7 > :that just worked(but slightly worse numbers from dd command compared > :to unpatched kernel) on Dynabook SS3500. > :- reduced DELAY()'s you added down to 1 > :- move up ata_clear_interlock() before ata_dmastart() > :- add missing ata_clear_interlock() after calls to ata_command() > : with ATA_WAIT_READY flag. > > Ooh. I think you are onto something. That makes a lot of sense. > > You need to add one thing, and that is to put a critical section > around the clearing of the interlock and the DMA start so no interrupt > can occur inbetween the two. > > Also, the command delay is mandatory... it's either in the specs > (somewhere), or it had to be done some time in the past to support > slow devices. The selection DELAY (the first one) below can be reduced > to 1us, but the command-start DELAY (the second one) cannot. > > Maybe we should make the command-start delay programmable with a > sysctl. 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?).
Attachment:
ata-7-take2.patch.gz
Description: application/gunzip