DragonFly kernel List (threaded) for 2004-11
[
Date Prev][
Date Next]
[
Thread Prev][
Thread Next]
[
Date Index][
Thread Index]
Re: ATA Patch #6
:Ok, I found four things while tweaking the driver
:1) talking to myself repeatedly on a public forum makes me look very stupid :)
Ah, but you are making so much progress!
:2) adding DELAY() in ata_command() unconditionally was critical on performance.
I can believe that... but by how much? I don't think that delay is
optional or, at least, the interrupt code may have to add the delay
itself. For the moment I would prefer to be conservative.
There are two unconditional delays. One at the beginning of
ata_command() (in the 'The 10uS delay here could probably be reduced
to 1 uS' section), and one at the end (in the 'Start the command
rolling...' section).
The performance difference shouldn't be terrible, though, what are
you seeing?
:3) you removed the conditional from the following code in ata_command()
: and made it always control interrupt from the device:
: /* disable interrupt from device */
: if (atadev->channel->flags & ATA_QUEUED)
: ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_IDS | ATA_A_4BIT);
:
: but it led to timeout or lock-up on two of my DragonFly machines.
: if I put the conditional back in, the timeout won't happen.
This is kinda central to the changes. I added the interrupt interlock
but in order to prevent an interrupt from freezing the system due to
livelock (because the interrupt just returns if the interlock is set and
doesn't try to clear anything) that means the interrupt bit must be
unconditionally disabled until the command has been completely set up.
If there is an issue here with the interrupt disablement I think
it may be the key to solving the lockup issue. The ATA spec is
fairly clear on how ATA_A_IDS is supposed to work so I thought it was
safe to manipulate.
:4) in ata_command(), calls to crit_enter() and crit_exit() don't correspond.
:
Oops. Yes, I see them. Here's a new patch (we'll call it #7) with
the critical section handling fixed. Though I think the mismatch
only occurs if an error/warning is also reported to the console.
I am presuming that you have tags turned off. tags don't work (and
never did).
-Matt
Matthew Dillon
<dillon@xxxxxxxxxxxxx>
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 27 Nov 2004 20:32:02 -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
@@ -601,6 +602,7 @@
ata_intr(void *data)
{
struct ata_channel *ch = (struct ata_channel *)data;
+
/*
* on PCI systems we might share an interrupt line with another
* device or our twin ATA channel, so call ch->intr_func to figure
@@ -609,14 +611,86 @@
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;
+ /*
+ * If we are interrupting mainline code that is in the middle of setting
+ * up we have a problem. Note that this case is checked before we figure
+ * out if the interrupt is real. The ATA interrupt should be disabled
+ * while ATA_INTERURPT_INTERLOCK is set so the interrupt must either not
+ * be for us, or no longer asserted if it was queued just prior to mainline
+ * code disabling it. Just return.
+ */
+ if (ch->flags & ATA_INTERRUPT_INTERLOCK) {
+ ata_printf(ch, -1, "warning, ignoring interrupt with interlock "
+ "set, active=%02x s=%02x\n",
+ ch->active, ch->status);
+ return;
}
- /* clear interrupt and get status */
+ /*
+ * 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;
+ }
+ }
+ }
+
+ /*
+ * 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)
@@ -832,7 +906,7 @@
* BUSY is released.
*/
DELAY(50000);
- ATA_OUTB(ch->r_altio, ATA_ALTSTAT, ATA_A_4BIT);
+ ATA_OUTB(ch->r_altio, ATA_ALTSTAT, ATA_A_IDS | ATA_A_4BIT);
if (stat0 & ATA_S_BUSY)
mask &= ~0x01;
@@ -990,13 +1064,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,67 +1096,115 @@
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);
+}
+
+/*
+ * Issue an ATA command and potentially wait for completion. If ATA_IMMEDIATE
+ * is used and no error occured, the caller must complete setup/operation and
+ * call ata_clear_interlock() on the channel if the caller desires an
+ * interrupt.
+ */
int
ata_command(struct ata_device *atadev, u_int8_t command,
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 */
+ /*
+ * Select the device, set ATA_INTERRUPT_INTERLOCK, and disable the
+ * device interrupt.
+ *
+ * The 10uS delay here could probably be reduced to 1 uS
+ */
+ crit_enter();
+ atadev->channel->flags |= ATA_INTERRUPT_INTERLOCK;
ATA_OUTB(atadev->channel->r_io, ATA_DRIVE, ATA_D_IBM | atadev->unit);
+ DELAY(10);
+ ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_IDS | ATA_A_4BIT);
+ crit_exit();
- /* disable interrupt from device */
- if (atadev->channel->flags & ATA_QUEUED)
- ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_IDS | ATA_A_4BIT);
-
- /* ready to issue command ? */
+ /*
+ * Wait for BUSY to clear
+ */
if (ata_wait(atadev, 0) < 0) {
ata_prtdev(atadev, "timeout sending command=%02x s=%02x e=%02x\n",
command, atadev->channel->status, atadev->channel->error);
return -1;
}
- /* only use 48bit addressing if needed because of the overhead */
+ /*
+ * only use 48bit addressing if needed because of the overhead
+ */
if ((lba > 268435455 || count > 256) && atadev->param &&
atadev->param->support.address48) {
ATA_OUTB(atadev->channel->r_io, ATA_FEATURE, (feature>>8) & 0xff);
@@ -1084,7 +1219,9 @@
ATA_OUTB(atadev->channel->r_io, ATA_CYL_MSB, (lba>>16) & 0xff);
ATA_OUTB(atadev->channel->r_io, ATA_DRIVE, ATA_D_LBA | atadev->unit);
- /* translate command into 48bit version */
+ /*
+ * translate command into 48bit version
+ */
switch (command) {
case ATA_C_READ:
command = ATA_C_READ48; break;
@@ -1108,8 +1245,7 @@
ata_prtdev(atadev, "can't translate cmd to 48bit version\n");
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,33 +1259,43 @@
ATA_D_IBM | ATA_D_LBA | atadev->unit | ((lba>>24) &0xf));
}
+ /*
+ * 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);
+
+ /*
+ * ATA_IMMEDIATE means that the caller is going to do more setup, do
+ * not reenable interrupts or clear the interlock.
+ *
+ * ATA_WAIT_INTR means that the caller wants to wait for the completion
+ * interrupt, reenable interrupts and clear the interlock.
+ *
+ * ATA_WAIT_READY is used during fixed initialization sequences and
+ * interrupts should be left disabled.
+ */
switch (flags & ATA_WAIT_MASK) {
case ATA_IMMEDIATE:
- 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);
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);
-
+ crit_enter();
+ atadev->channel->flags &= ~ATA_INTERRUPT_INTERLOCK;
+ 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;
}
+ crit_exit();
break;
-
case ATA_WAIT_READY:
- atadev->channel->active |= ATA_WAIT_READY;
- ATA_OUTB(atadev->channel->r_io, ATA_CMD, command);
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);
@@ -1161,6 +1307,25 @@
return error;
}
+/*
+ * ata_command() physically disables the ATA interrupt and sets the
+ * ATA_INTERRUPT_INTERLOCK bit to prevent ata_intr() handler from touching
+ * the ATA I/O registers.
+ *
+ * This routine reenables the ATA interrupt and clears the interlock, and
+ * is called after the command has been completely queued including any
+ * required DMA setup/start. Anyone who calls ata_command() with ATA_IMMEDIATE
+ * who is not explicitly polling for the result must call this routine.
+ */
+void
+ata_clear_interlock(struct ata_device *atadev)
+{
+ crit_enter();
+ atadev->channel->flags &= ~ATA_INTERRUPT_INTERLOCK;
+ ATA_OUTB(atadev->channel->r_altio, ATA_ALTSTAT, ATA_A_4BIT);
+ crit_exit();
+}
+
static void
ata_enclosure_start(struct ata_device *atadev)
{
Index: ata-all.h
===================================================================
RCS file: /cvs/src/sys/dev/disk/ata/ata-all.h,v
retrieving revision 1.7
diff -u -r1.7 ata-all.h
--- ata-all.h 18 Feb 2004 02:47:38 -0000 1.7
+++ ata-all.h 27 Nov 2004 20:25:36 -0000
@@ -215,6 +215,7 @@
#define ATA_ATAPI_DMA_RO 0x04
#define ATA_QUEUED 0x08
#define ATA_DMA_ACTIVE 0x10
+#define ATA_INTERRUPT_INTERLOCK 0x20
struct ata_device device[2]; /* devices on this channel */
#define MASTER 0x00
@@ -268,6 +269,7 @@
int ata_reinit(struct ata_channel *);
int ata_wait(struct ata_device *, u_int8_t);
int ata_command(struct ata_device *, u_int8_t, u_int64_t, u_int16_t, u_int8_t, int);
+void ata_clear_interlock(struct ata_device *);
void ata_enclosure_leds(struct ata_device *, u_int8_t);
void ata_enclosure_print(struct ata_device *);
int ata_printf(struct ata_channel *, int, const char *, ...) __printflike(3, 4);
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 27 Nov 2004 20:25:36 -0000
@@ -54,6 +54,7 @@
#include "ata-raid.h"
#include <sys/proc.h>
#include <sys/buf2.h>
+#include <sys/thread2.h>
/* device structures */
static d_open_t adopen;
@@ -84,6 +85,7 @@
static void ad_requeue(struct ata_channel *, struct ad_request *);
static void ad_invalidatequeue(struct ad_softc *, struct ad_request *);
static int ad_tagsupported(struct ad_softc *);
+static void ad_start_timeout(struct ad_request *);
static void ad_timeout(struct ad_request *);
static void ad_free(struct ad_request *);
static int ad_version(u_int16_t);
@@ -462,6 +464,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 */
@@ -471,15 +476,6 @@
lba = request->blockaddr + (request->donecount / DEV_BSIZE);
if (request->donecount == 0) {
-
- /* start timeout for this transfer */
- if (dumping) {
- callout_stop(&request->callout);
- } else {
- callout_reset(&request->callout, 10 * hz,
- (void *)ad_timeout, request);
- }
-
/* setup transfer parameters */
count = howmany(request->bytecount, DEV_BSIZE);
max_count = adp->device->param->support.address48 ? 65536 : 256;
@@ -503,7 +499,18 @@
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. For DMA transfers the device may
+ * or may not set ATA_S_DRQ. If it DOES set ATA_S_DRQ it might also
+ * clear ATA_S_BUSY. Either bit will be set while the operation is
+ * in progress but which one (or both) depends on the target device.
+ *
+ * If the device does play with DRQ it may set it immediately or it
+ * may wait until it actually needs the data (which could be several
+ * milliseconds), so we don't try to wait for it.
+ */
request->flags &= ~ADR_F_DMA_USED;
if (adp->device->mode >= ATA_DMA &&
!ata_dmasetup(adp->device, request->data, request->bytecount)) {
@@ -526,13 +533,14 @@
}
adp->outstanding++;
+ ad_start_timeout(request);
+
/* if ATA bus RELEASE check for SERVICE */
if (adp->flags & AD_F_TAG_ENABLED &&
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,31 +548,42 @@
ata_prtdev(adp->device, "error executing command");
goto transfer_failed;
}
-#if 0
+
/*
- * wait for data transfer phase
+ * For a DMA transfer the device will set either BUSY or
+ * DRQ to 1, or will set both bits to 1, or will set BUSY to
+ * 0 and DRQ to 1, or will do one of the above and then do
+ * another of the above. If an error occurs the device may
+ * clear both BUSY and DRQ even before we get to here.
*
- * well this should be here acording to specs, but older
- * promise controllers doesn't like it, they lockup!
+ * It is said that some much older devices may even clear
+ * BUSY *BEFORE* they set DRQ. The only way to deal with
+ * this is to not check the status register until an interrupt
+ * occurs, which is problematic at best since the interrupt
+ * might be shared.
+ *
+ * Basically its a mess.
*/
- 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 */
+ /*
+ * Physically start DMA rolling and setup our timeout, then
+ * clear the interrupt interlock and reenable interrupts. It
+ * is possible that this will generate an immediate interrupt.
+ */
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;
}
- /* does this drive support multi sector transfers ? */
+ /*
+ * Fall through to PIO, initial command. Use a multi-sector transfer
+ * if it is supported.
+ */
if (request->currentsize > DEV_BSIZE)
cmd = request->flags&ADR_F_READ ? ATA_C_READ_MUL : ATA_C_WRITE_MUL;
-
- /* just plain old single sector transfer */
else
cmd = request->flags&ADR_F_READ ? ATA_C_READ : ATA_C_WRITE;
@@ -573,13 +592,20 @@
goto transfer_failed;
}
}
+
+ /*
+ * PIO initial command or continuance
+ */
/* calculate this transfer length */
request->currentsize = min(request->bytecount, adp->transfersize);
/* if this is a PIO read operation, return and wait for interrupt */
- if (request->flags & ADR_F_READ)
+ if (request->flags & ADR_F_READ) {
+ ad_start_timeout(request);
+ ata_clear_interlock(adp->device);
return ATA_OP_CONTINUES;
+ }
/* ready to write PIO data ? */
if (ata_wait(adp->device, (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) < 0) {
@@ -596,17 +622,18 @@
ATA_OUTSL(adp->device->channel->r_io, ATA_DATA,
(void *)((uintptr_t)request->data + request->donecount),
request->currentsize / sizeof(int32_t));
+ ad_start_timeout(request);
+ ata_clear_interlock(adp->device);
return ATA_OP_CONTINUES;
transfer_failed:
- callout_stop(&request->callout);
ad_invalidatequeue(adp, request);
printf(" - resetting\n");
/* if retries still permit, reinject this request */
- if (request->retries++ < AD_MAX_RETRIES)
+ if (request->retries++ < AD_MAX_RETRIES) {
ad_requeue(adp->device->channel, request);
- else {
+ } else {
/* retries all used up, return error */
request->bp->b_error = EIO;
request->bp->b_flags |= B_ERROR;
@@ -625,20 +652,24 @@
struct ad_softc *adp = request->softc;
int dma_stat = 0;
+ /* disarm timeout for this transfer */
+ callout_stop(&request->callout);
+
/* finish DMA transfer */
if (request->flags & ADR_F_DMA_USED)
dma_stat = ata_dmadone(adp->device);
/* do we have a corrected soft error ? */
- if (adp->device->channel->status & ATA_S_CORR)
+ if (adp->device->channel->status & ATA_S_CORR) {
diskerr(request->bp, request->bp->b_dev,
"soft error (ECC corrected)", LOG_PRINTF,
request->blockaddr + (request->donecount / DEV_BSIZE),
&adp->disk.d_label);
+ }
/* did any real errors happen ? */
if ((adp->device->channel->status & ATA_S_ERROR) ||
- (request->flags & ADR_F_DMA_USED && dma_stat & ATA_BMSTAT_ERROR)) {
+ ((request->flags & ADR_F_DMA_USED) && (dma_stat & ATA_BMSTAT_ERROR))) {
adp->device->channel->error =
ATA_INB(adp->device->channel->r_io, ATA_ERROR);
diskerr(request->bp, request->bp->b_dev,
@@ -650,7 +681,6 @@
/* if this is a UDMA CRC error, reinject request */
if (request->flags & ADR_F_DMA_USED &&
adp->device->channel->error & ATA_E_ICRC) {
- callout_stop(&request->callout);
ad_invalidatequeue(adp, request);
if (request->retries++ < AD_MAX_RETRIES)
@@ -665,7 +695,6 @@
/* if using DMA, try once again in PIO mode */
if (request->flags & ADR_F_DMA_USED) {
- callout_stop(&request->callout);
ad_invalidatequeue(adp, request);
ata_dmainit(adp->device, ata_pmode(adp->device->param), -1, -1);
request->flags |= ADR_F_FORCE_PIO;
@@ -694,8 +723,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,
@@ -712,8 +740,7 @@
if (request->flags & ADR_F_ERROR) {
request->bp->b_error = EIO;
request->bp->b_flags |= B_ERROR;
- }
- else {
+ } else {
request->bytecount -= request->currentsize;
request->donecount += request->currentsize;
if (request->bytecount > 0) {
@@ -722,9 +749,6 @@
}
}
- /* disarm timeout for this transfer */
- callout_stop(&request->callout);
-
request->bp->b_resid = request->bytecount;
devstat_end_transaction_buf(&adp->stats, request->bp);
@@ -821,6 +845,7 @@
}
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;
@@ -895,15 +920,38 @@
return 0;
}
+static
+void
+ad_start_timeout(struct ad_request *request)
+{
+ if (dumping == 0)
+ callout_reset(&request->callout, 10 * hz, (void *)ad_timeout, request);
+}
+
static void
ad_timeout(struct ad_request *request)
{
struct ad_softc *adp = request->softc;
+ u_int8_t status;
+ /*
+ * Handle races against ad_interrupt.
+ */
+ crit_enter();
+ if (request != adp->device->channel->running) {
+ printf("ad_timeout: request %p was ripped out from under us\n", request);
+ crit_exit();
+ return;
+ }
adp->device->channel->running = NULL;
- ata_prtdev(adp->device, "%s command timeout tag=%d serv=%d - resetting\n",
+ status = ATA_INB(adp->device->channel->r_io, ATA_STATUS);
+ crit_exit();
+ ata_prtdev(adp->device, "%s command timeout tag=%d serv=%d status %02x"
+ " dmastatus %02x - resetting\n",
(request->flags & ADR_F_READ) ? "READ" : "WRITE",
- request->tag, request->serv);
+ request->tag, request->serv, status,
+ (adp->device->mode >= ATA_DMA ?
+ ata_dmastatus(adp->device->channel) : 0xff));
if (request->flags & ADR_F_DMA_USED) {
ata_dmadone(adp->device);
Index: ata-dma.c
===================================================================
RCS file: /cvs/src/sys/dev/disk/ata/ata-dma.c,v
retrieving revision 1.25
diff -u -r1.25 ata-dma.c
--- ata-dma.c 1 Sep 2004 14:13:55 -0000 1.25
+++ ata-dma.c 27 Nov 2004 20:25:36 -0000
@@ -1336,13 +1336,18 @@
}
}
+/*
+ * Create the dma table for an ATA transfer. ATA_DMA_EOT must be set on
+ * the last entry. Setting it after the last entry rather then on the
+ * last entry may screw up EOT timing.
+ */
int
ata_dmasetup(struct ata_device *atadev, caddr_t data, int32_t count)
{
struct ata_channel *ch = atadev->channel;
struct ata_dmastate *ds = &atadev->dmastate;
- u_int32_t dma_count, dma_base;
- int i = 0;
+ u_int32_t dma_count;
+ int i;
if (((uintptr_t)data & ch->alignment) || (count & ch->alignment)) {
ata_prtdev(atadev, "non aligned DMA transfer attempted\n");
@@ -1353,27 +1358,20 @@
ata_prtdev(atadev, "zero length DMA transfer attempted\n");
return -1;
}
-
- dma_base = vtophys(data);
- dma_count = imin(count, (PAGE_SIZE - ((uintptr_t)data & PAGE_MASK)));
- data += dma_count;
- count -= dma_count;
-
- while (count) {
- ds->dmatab[i].base = dma_base;
- ds->dmatab[i].count = (dma_count & 0xffff);
- i++;
- if (i >= ATA_DMA_ENTRIES) {
- ata_prtdev(atadev, "too many segments in DMA table\n");
- return -1;
- }
- dma_base = vtophys(data);
- dma_count = imin(count, PAGE_SIZE);
- data += imin(count, PAGE_SIZE);
- count -= imin(count, PAGE_SIZE);
+
+ for (i = 0; count && i < ATA_DMA_ENTRIES; ++i) {
+ dma_count = imin(count, (PAGE_SIZE - ((uintptr_t)data & PAGE_MASK)));
+ ds->dmatab[i].base = vtophys(data);
+ ds->dmatab[i].count = dma_count & 0xffff;
+
+ data += dma_count;
+ count -= dma_count;
+ }
+ if (count) {
+ ata_prtdev(atadev, "too many segments in DMA table\n");
+ return -1;
}
- ds->dmatab[i].base = dma_base;
- ds->dmatab[i].count = (dma_count & 0xffff) | ATA_DMA_EOT;
+ ds->dmatab[i-1].count |= ATA_DMA_EOT;
return 0;
}
Index: atapi-all.c
===================================================================
RCS file: /cvs/src/sys/dev/disk/ata/atapi-all.c,v
retrieving revision 1.14
diff -u -r1.14 atapi-all.c
--- atapi-all.c 18 Sep 2004 18:33:38 -0000 1.14
+++ atapi-all.c 27 Nov 2004 20:25:36 -0000
@@ -319,8 +319,10 @@
request->flags & ATPR_F_READ);
/* command interrupt device ? just return */
- if (atadev->param->drq_type == ATAPI_DRQT_INTR)
+ if (atadev->param->drq_type == ATAPI_DRQT_INTR) {
+ ata_clear_interlock(atadev);
return ATA_OP_CONTINUES;
+ }
/* ready to write ATAPI command */
timout = 5000; /* might be less for fast devices */
@@ -346,6 +348,7 @@
/* send actual command */
ATA_OUTSW(atadev->channel->r_io, ATA_DATA, (int16_t *)request->ccb,
request->ccbsize / sizeof(int16_t));
+ ata_clear_interlock(atadev);
return ATA_OP_CONTINUES;
}
[
Date Prev][
Date Next]
[
Thread Prev][
Thread Next]
[
Date Index][
Thread Index]
