ATA driver patch #5 to test
Here's another one.
-Matt
Index: ata-all.c
===================================================================
RCS file: /cvs/src/sys/dev/disk/ata/ata-all.c,v
retrieving revision 1.21
diff -u -r1.21 ata-all.c
--- ata-all.c 17 Aug 2004 20:59:39 -0000 1.21
+++ ata-all.c 15 Nov 2004 22:36:43 -0000
@@ -26,7 +26,7 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
* $FreeBSD: src/sys/dev/ata/ata-all.c,v 1.50.2.45 2003/03/12 14:47:12 sos Exp $
- * $DragonFly: src/sys/dev/disk/ata/ata-all.c,v 1.21 2004/08/17 20:59:39 dillon Exp $
+ * $DragonFly$
*/
#include "opt_ata.h"
@@ -53,6 +53,7 @@
#include <machine/bus.h>
#include <machine/clock.h>
#include <sys/rman.h>
+#include <sys/thread2.h>
#ifdef __alpha__
#include <machine/md_var.h>
#endif
@@ -609,14 +610,72 @@
if (ch->intr_func && ch->intr_func(ch))
return;
- /* if drive is busy it didn't interrupt */
- if (ATA_INB(ch->r_altio, ATA_ALTSTAT) & ATA_S_BUSY) {
- DELAY(100);
- if (!(ATA_INB(ch->r_altio, ATA_ALTSTAT) & ATA_S_DRQ))
- return;
+ /*
+ * Figure out if the interrupt is real. False interrupts can occur in a
+ * number of situations including:
+ *
+ * - The device asserts the interrupt prior to clearing BUSY or setting
+ * DRQ.
+ *
+ * - The device generated an interrupt before we were able to disable
+ * it and the interrupt thread was already queued.
+ *
+ * - Other devices share this interrupt vector.
+ *
+ * - Upon queueing the device command the device may delay before
+ * setting BUSY (this is handled by ata_command(), not here).
+ *
+ * To deal with the remaining races if ATA_S_BUSY is set we must wait
+ * a bit and check it again to see if it is still set.
+ *
+ * Having to spin in a delay loop is really bad but there is no way
+ * around it. The ATA spec is just plain broken.
+ */
+
+ if (ch->flags & ATA_DMA_ACTIVE) {
+ /*
+ * In the DMA case the operation is not complete until both BUSY and
+ * DRQ are clear. If either is found to be set we delay and recheck.
+ * If either is still set the interrupt was bogus and we return.
+ *
+ * The ATA spec says that DMA data may be transfered with BUSY=1 and
+ * DRQ=0, or BUSY=0 and DRQ=1.
+ */
+ if (ATA_INB(ch->r_altio, ATA_ALTSTAT) & (ATA_S_BUSY | ATA_S_DRQ)) {
+ DELAY(100);
+ if (ATA_INB(ch->r_altio, ATA_ALTSTAT) & (ATA_S_BUSY | ATA_S_DRQ))
+ return;
+ }
+ } else {
+ /*
+ * In the PIO case if the device intends to transfer data it will
+ * set DRQ but may or may not clear BUSY. If the device does not
+ * intend to transfer data (for example, an error occurs or there is
+ * no data to transfer) then the device will clear both BUSY and DRQ.
+ *
+ * What this means is we still need to do the delay+recheck due to
+ * the races mentioned above, but in the second test the interrupt
+ * is only false if BUSY is set and DRQ is clear.
+ *
+ * It may be possible to reduce the 100uS delay below to 10uS.
+ */
+ if (ATA_INB(ch->r_altio, ATA_ALTSTAT) & ATA_S_BUSY) {
+ DELAY(100);
+ if ((ATA_INB(ch->r_altio, ATA_ALTSTAT) & (ATA_S_BUSY | ATA_S_DRQ))
+ == ATA_S_BUSY
+ ) {
+ return;
+ }
+ }
}
- /* clear interrupt and get status */
+ /*
+ * As per ATA spec we must throw away data read above and re-read from
+ * the main status register to get good data and to clear the interrupt.
+ * Otherwise the other bits may have still been changing state when we
+ * determined that BUSY and DRQ were golden (the old device was writing to
+ * the status register at the same time we were reading from it problem).
+ */
ch->status = ATA_INB(ch->r_io, ATA_STATUS);
if (ch->status & ATA_S_ERROR)
@@ -990,13 +1049,26 @@
return ATA_OP_FINISHED;
}
+/*
+ * Wait for BUSY to clear and then for all bits specified by the mask
+ * to be set. The ATA spec says that we have to re-read the status register
+ * after we detect that BUSY has cleared to deal with garbage that may occur
+ * when the device is writing to the status register simultanious with our
+ * reading from it.
+ */
int
ata_wait(struct ata_device *atadev, u_int8_t mask)
{
- int timeout = 0;
-
- DELAY(1);
- while (timeout < 5000000) { /* timeout 5 secs */
+ sysclock_t last;
+ int timeout;
+
+ /*
+ * 5-second timeout
+ */
+ timeout = 5;
+ last = cputimer_count();
+
+ while (timeout) {
atadev->channel->status = ATA_INB(atadev->channel->r_io, ATA_STATUS);
/* if drive fails status, reselect the drive just to be sure */
@@ -1009,38 +1081,70 @@
return -1;
}
- /* are we done ? */
- if (!(atadev->channel->status & ATA_S_BUSY))
- break;
-
- if (timeout > 1000) {
- timeout += 1000;
- DELAY(1000);
+ /*
+ * When BUSY clears we have to re-read the status register to get
+ * non-junky data in case we raced the device writing to the status
+ * register.
+ */
+ if ((atadev->channel->status & ATA_S_BUSY) == 0) {
+ atadev->channel->status = ATA_INB(atadev->channel->r_io, ATA_STATUS);
+ if ((atadev->channel->status & ATA_S_BUSY) == 0)
+ break;
}
- else {
- timeout += 10;
- DELAY(10);
+
+ /*
+ * It is unclear whether we need to delay here or whether we can
+ * just poll status as fast as possible. DELAY a little.
+ */
+ DELAY(5);
+ if (cputimer_count() - last > cputimer_freq) {
+ --timeout;
+ last += cputimer_freq;
}
}
+
+ /*
+ * If status indicates error save the error register for future use,
+ * fail on timeout.
+ */
if (atadev->channel->status & ATA_S_ERROR)
atadev->channel->error = ATA_INB(atadev->channel->r_io, ATA_ERROR);
- if (timeout >= 5000000)
+
+ if (timeout == 0)
return -1;
- if (!mask)
+
+ /*
+ * If we are not waiting for any bits we were just waiting for BUSY to
+ * clear and return the error status.
+ */
+ if (mask == 0)
return (atadev->channel->status & ATA_S_ERROR);
-
- /* Wait 50 msec for bits wanted. */
- timeout = 5000;
- while (timeout--) {
- atadev->channel->status = ATA_INB(atadev->channel->r_io, ATA_STATUS);
- if ((atadev->channel->status & mask) == mask) {
- if (atadev->channel->status & ATA_S_ERROR)
- atadev->channel->error=ATA_INB(atadev->channel->r_io,ATA_ERROR);
- return (atadev->channel->status & ATA_S_ERROR);
- }
- DELAY (10);
- }
- return -1;
+
+ /*
+ * After BUSY has cleared we wait up to 100ms for the desired bits.
+ * The bits should theoretically have already been loaded in. If
+ * we already have the data we do not have to reload since we already did
+ * that above.
+ */
+ if ((atadev->channel->status & mask) != mask) {
+ timeout = 100;
+ last = cputimer_count();
+ while (timeout) {
+ atadev->channel->status = ATA_INB(atadev->channel->r_io, ATA_STATUS);
+ if ((atadev->channel->status & mask) == mask)
+ break;
+ if (cputimer_count() - last > cputimer_freq / (1000 / 50)) {
+ --timeout;
+ last += cputimer_freq / (1000 / 50);
+ }
+ DELAY(5);
+ }
+ if (timeout == 0)
+ return(-1);
+ }
+ if (atadev->channel->status & ATA_S_ERROR)
+ atadev->channel->error = ATA_INB(atadev->channel->r_io,ATA_ERROR);
+ return (atadev->channel->status & ATA_S_ERROR);
}
int
@@ -1048,15 +1152,16 @@
u_int64_t lba, u_int16_t count, u_int8_t feature, int flags)
{
int error = 0;
+
#ifdef ATA_DEBUG
ata_prtdev(atadev, "ata_command: addr=%04lx, cmd=%02x, "
"lba=%lld, count=%d, feature=%d, flags=%02x\n",
rman_get_start(atadev->channel->r_io),
command, lba, count, feature, flags);
#endif
-
/* select device */
ATA_OUTB(atadev->channel->r_io, ATA_DRIVE, ATA_D_IBM | atadev->unit);
+ DELAY(10); /* XXX */
/* disable interrupt from device */
if (atadev->channel->flags & ATA_QUEUED)
@@ -1069,6 +1174,15 @@
return -1;
}
+ /*
+ * Disabling the interrupt may not be enough if it is shared. It can
+ * take ATA_S_BUSY up to 10uS to become true after a new command has been
+ * queued, we have to guarentee that we do not falsely assume command
+ * completion when the command hasn't even started yet. We also may need
+ * to interlock with a later tsleep() call.
+ */
+ crit_enter();
+
/* only use 48bit addressing if needed because of the overhead */
if ((lba > 268435455 || count > 256) && atadev->param &&
atadev->param->support.address48) {
@@ -1106,10 +1220,10 @@
command = ATA_C_FLUSHCACHE48; break;
default:
ata_prtdev(atadev, "can't translate cmd to 48bit version\n");
+ crit_exit();
return -1;
}
- }
- else {
+ } else {
ATA_OUTB(atadev->channel->r_io, ATA_FEATURE, feature);
ATA_OUTB(atadev->channel->r_io, ATA_COUNT, count);
ATA_OUTB(atadev->channel->r_io, ATA_SECTOR, lba & 0xff);
@@ -1123,41 +1237,52 @@
ATA_D_IBM | ATA_D_LBA | atadev->unit | ((lba>>24) &0xf));
}
- switch (flags & ATA_WAIT_MASK) {
- case ATA_IMMEDIATE:
- ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
+ /*
+ * Start the command rolling and wait at least 10uS for the device to
+ * bring up BUSY. Nasty, unfortunately, but at least the delay should
+ * run in parallel with the target device's processing of the command.
+ *
+ * NOTE: The ATA spec says that the status register is invalid for 400ns
+ * after writing the command byte.
+ */
+ atadev->channel->active |= flags & (ATA_WAIT_INTR | ATA_WAIT_READY);
+ ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
+ DELAY(10);
+ /*
+ * Reenable the ATA interrupt unless we are intending to wait for
+ * completion synchronously. Note: tsleep is interlocked with our
+ * critical section, otherwise completion may occur before the timeout
+ * is started.
+ */
+ if ((flags & ATA_WAIT_MASK) != ATA_WAIT_READY) {
/* enable interrupt */
if (atadev->channel->flags & ATA_QUEUED)
ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_4BIT);
- break;
+ }
+ switch (flags & ATA_WAIT_MASK) {
+ case ATA_IMMEDIATE:
+ break;
case ATA_WAIT_INTR:
- atadev->channel->active |= ATA_WAIT_INTR;
- ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
-
- /* enable interrupt */
- if (atadev->channel->flags & ATA_QUEUED)
- ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_4BIT);
-
if (tsleep((caddr_t)atadev->channel, 0, "atacmd", 10 * hz)) {
ata_prtdev(atadev, "timeout waiting for interrupt\n");
atadev->channel->active &= ~ATA_WAIT_INTR;
error = -1;
}
break;
-
case ATA_WAIT_READY:
- atadev->channel->active |= ATA_WAIT_READY;
- ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
+ crit_exit();
if (ata_wait(atadev, ATA_S_READY) < 0) {
ata_prtdev(atadev, "timeout waiting for cmd=%02x s=%02x e=%02x\n",
command, atadev->channel->status,atadev->channel->error);
error = -1;
}
atadev->channel->active &= ~ATA_WAIT_READY;
+ crit_enter();
break;
}
+ crit_exit();
return error;
}
Index: ata-disk.c
===================================================================
RCS file: /cvs/src/sys/dev/disk/ata/ata-disk.c,v
retrieving revision 1.23
diff -u -r1.23 ata-disk.c
--- ata-disk.c 23 Sep 2004 11:50:03 -0000 1.23
+++ ata-disk.c 15 Nov 2004 22:05:48 -0000
@@ -462,6 +462,9 @@
u_int64_t lba;
u_int32_t count, max_count;
u_int8_t cmd;
+#if 0
+ u_int8_t status;
+#endif
int flags = ATA_IMMEDIATE;
/* get request params */
@@ -503,7 +506,20 @@
devstat_start_transaction(&adp->stats);
- /* does this drive & transfer work with DMA ? */
+ /*
+ * DMA OPERATION. A DMA based ATA command issues the command and
+ * then sets up and initiates DMA. Once the command is issued the
+ * device will set ATA_S_BUSY. When the device is ready to transfer
+ * actual data it will clear ATA_S_BUSY and set ATA_S_DRQ. However,
+ * because DRQ represents actual data ready to roll this can take
+ * a while (e.g. for a read the disk has to get to the data). We
+ * don't want to spin that long.
+ *
+ * It is unclear whether DMA can simply be started without waiting
+ * for ATA_S_BUSY to clear but that seems to be what everyone else
+ * does. It might be prudent to wait for at least ATA_S_READY but
+ * we don't (yet) do that.
+ */
request->flags &= ~ADR_F_DMA_USED;
if (adp->device->mode >= ATA_DMA &&
!ata_dmasetup(adp->device, request->data, request->bytecount)) {
@@ -531,8 +547,7 @@
ATA_INB(adp->device->channel->r_io, ATA_IREASON) &
ATA_I_RELEASE)
return ad_service(adp, 1);
- }
- else {
+ } else {
cmd = (request->flags & ADR_F_READ) ?
ATA_C_READ_DMA : ATA_C_WRITE_DMA;
@@ -540,18 +555,6 @@
ata_prtdev(adp->device, "error executing command");
goto transfer_failed;
}
-#if 0
- /*
- * wait for data transfer phase
- *
- * well this should be here acording to specs, but older
- * promise controllers doesn't like it, they lockup!
- */
- if (ata_wait(adp->device, ATA_S_READY | ATA_S_DRQ)) {
- ata_prtdev(adp->device, "timeout waiting for data phase\n");
- goto transfer_failed;
- }
-#endif
}
/* start transfer, return and wait for interrupt */
@@ -694,8 +697,7 @@
if (ata_wait(adp->device, (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) != 0) {
ata_prtdev(adp->device, "read error detected (too) late");
request->flags |= ADR_F_ERROR;
- }
- else {
+ } else {
/* data ready, read in */
if (adp->device->channel->flags & ATA_USE_16BIT)
ATA_INSW(adp->device->channel->r_io, ATA_DATA,
@@ -899,11 +901,14 @@
ad_timeout(struct ad_request *request)
{
struct ad_softc *adp = request->softc;
+ u_int8_t status;
+ status = ATA_INB(adp->device->channel->r_io, ATA_STATUS);
adp->device->channel->running = NULL;
- ata_prtdev(adp->device, "%s command timeout tag=%d serv=%d - resetting\n",
+ ata_prtdev(adp->device, "%s command timeout tag=%d serv=%d status %02x"
+ " - resetting\n",
(request->flags & ADR_F_READ) ? "READ" : "WRITE",
- request->tag, request->serv);
+ request->tag, request->serv, status);
if (request->flags & ADR_F_DMA_USED) {
ata_dmadone(adp->device);