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

[YONETANI Tomokazu <qhwt+dfly@xxxxxxxxxx>]

On Mon, Nov 29, 2004 at 09:32:21PM +0900, YONETANI Tomokazu wrote:
> On Mon, Nov 29, 2004 at 07:42:58AM +0100, Jeroen Ruigrok/asmodai wrote:
> > -On [20041128 14:32], YONETANI Tomokazu (qhwt+dfly@xxxxxxxxxx) wrote:
> > >I hope so(I'm talking without knowing the ATA spec, do you have a reference
> > >to a documentation I can read?)
> > 
> > http://www.t13.org/
> 
> Thanks! I'll be silent(at least for this issue) for a while in order to
> read them through.

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.

diff -u ata-7/ata-all.c ata/ata-all.c
--- ata-7/ata-all.c	2004-11-28 21:59:02.000000000 +0900
+++ ata/ata-all.c	2004-11-30 01:51:32.000000000 +0900
@@ -1189,7 +1189,7 @@
     crit_enter();
     atadev->channel->flags |= ATA_INTERRUPT_INTERLOCK;
     ATA_OUTB(atadev->channel->r_io, ATA_DRIVE, ATA_D_IBM | atadev->unit);
-    DELAY(10);
+    DELAY(1);
     ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_IDS | ATA_A_4BIT);
     crit_exit();
 
@@ -1269,7 +1269,7 @@
      */
     atadev->channel->active |= flags & (ATA_WAIT_INTR | ATA_WAIT_READY);
     ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
-    DELAY(10);
+    DELAY(1);
 
     /*
      * ATA_IMMEDIATE means that the caller is going to do more setup, do
diff -u ata-7/ata-disk.c ata/ata-disk.c
--- ata-7/ata-disk.c	2004-11-28 21:59:02.000000000 +0900
+++ ata/ata-disk.c	2004-11-30 01:52:21.000000000 +0900
@@ -265,6 +265,7 @@
     if (flush) {
 	if (ata_command(atadev, ATA_C_FLUSHCACHE, 0, 0, 0, ATA_WAIT_READY))
 	    ata_prtdev(atadev, "flushing cache on detach failed\n");
+	ata_clear_interlock(atadev);
     }
     if (adp->flags & AD_F_RAID_SUBDISK)
 	ata_raiddisk_detach(adp);
@@ -295,6 +296,7 @@
     ATA_SLEEPLOCK_CH(adp->device->channel, ATA_CONTROL);
     if (ata_command(adp->device, ATA_C_FLUSHCACHE, 0, 0, 0, ATA_WAIT_READY))
 	ata_prtdev(adp->device, "flushing cache on close failed\n");
+    ata_clear_interlock(adp->device);
     ATA_UNLOCK_CH(adp->device->channel);
     splx(s);
     return 0;
@@ -571,10 +573,10 @@
 	     * clear the interrupt interlock and reenable interrupts.  It
 	     * is possible that this will generate an immediate interrupt.
 	     */
+	    ata_clear_interlock(adp->device);
 	    ata_dmastart(adp->device, request->data, request->bytecount,
 			request->flags & ADR_F_READ);
 	    ad_start_timeout(request);
-	    ata_clear_interlock(adp->device);
 	    return ATA_OP_CONTINUES;
 	}
 
@@ -843,9 +845,9 @@
 	    ad_invalidatequeue(adp, NULL);
 	    return ATA_OP_FINISHED;
 	}
+	ata_clear_interlock(adp->device);
 	ata_dmastart(adp->device, request->data, request->bytecount,
 		    request->flags & ADR_F_READ);
-	ata_clear_interlock(adp->device);
 	return ATA_OP_CONTINUES;
     }
     return ATA_OP_FINISHED;
@@ -881,6 +883,7 @@
 	if (ata_command(adp->device, ATA_C_NOP,
 			0, 0, ATA_C_F_FLUSHQUEUE, ATA_WAIT_READY))
 	    ata_prtdev(adp->device, "flush queue failed\n");
+	ata_clear_interlock(adp->device);
 	adp->outstanding = 0;
     }
 }
@@ -993,6 +996,7 @@
 		    ata_umode(adp->device->param));
     else
 	ata_dmainit(atadev, ata_pmode(adp->device->param), -1, -1);
+    ata_clear_interlock(atadev);
 }
 
 void