4 files changed, 118 insertions, 169 deletions

diff --git a/sys/dev/ata/ata-all.c b/sys/dev/ata/ata-all.c
index c70e01fcb3c6..85bf3335da70 100644
--- a/sys/dev/ata/ata-all.c
+++ b/sys/dev/ata/ata-all.c
@@ -60,7 +60,7 @@ __FBSDID("$FreeBSD$");
 static  d_ioctl_t       ata_ioctl;
 static struct cdevsw ata_cdevsw = {
 	.d_version =    D_VERSION,
-	.d_flags =      D_NEEDGIANT, /* we need this as newbus isn't safe */
+	.d_flags =      D_NEEDGIANT, /* we need this as newbus isn't mpsafe */
 	.d_ioctl =      ata_ioctl,
 	.d_name =       "ata",
 };
@@ -68,7 +68,7 @@ static struct cdevsw ata_cdevsw = {
 /* prototypes */
 static void ata_interrupt(void *);
 static void ata_boot_attach(void);
-device_t ata_add_child(device_t parent, struct ata_device *atadev, int unit);
+static device_t ata_add_child(device_t parent, struct ata_device *atadev, int unit);
 
 /* global vars */
 MALLOC_DEFINE(M_ATA, "ATA generic", "ATA driver generic layer");
@@ -80,7 +80,6 @@ int ata_wc = 1;
 
 /* local vars */
 static struct intr_config_hook *ata_delayed_attach = NULL;
-static struct root_hold_token *ata_root_hold_token;
 static int ata_dma = 1;
 static int atapi_dma = 1;
 
@@ -144,12 +143,9 @@ ata_attach(device_t dev)
 	return error;
     }
 
-    /* do not attach devices if we are in early boot */
-    if (ata_delayed_attach)
-	return 0;
-
-    /* probe and attach devices on this channel */
-    ata_identify(dev);
+    /* probe and attach devices on this channel unless we are in early boot */
+    if (!ata_delayed_attach)
+	ata_identify(dev);
     return 0;
 }
 
@@ -188,6 +184,7 @@ ata_reinit(device_t dev)
     device_t *children;
     int nchildren, i;
 
+    /* check that we have a vaild channel to reinit */
     if (!ch || !ch->r_irq)
 	return ENXIO;
 
@@ -198,7 +195,7 @@ ata_reinit(device_t dev)
     while (ATA_LOCKING(dev, ATA_LF_LOCK) != ch->unit)
 	tsleep(&dev, PRIBIO, "atarini", 1);
 
-    /* grap the channel lock */
+    /* unconditionally grap the channel lock */
     mtx_lock(&ch->state_mtx);
     ch->state = ATA_STALL_QUEUE;
     mtx_unlock(&ch->state_mtx);
@@ -212,7 +209,12 @@ ata_reinit(device_t dev)
 	for (i = 0; i < nchildren; i++) {
 	    if (children[i] && device_is_attached(children[i]))
 		if (ATA_REINIT(children[i])) {
-		    if (ch->running->dev == children[i]) {
+		    /*
+		     * if we have a running request and its device matches
+		     * this child we need to inform the request that the 
+		     * device is gone and remove it from ch->running
+		     */
+		    if (ch->running && ch->running->dev == children[i]) {
 			device_printf(ch->running->dev,
 				      "FAILURE - device detached\n");
 			ch->running->dev = NULL;
@@ -225,7 +227,7 @@ ata_reinit(device_t dev)
 	mtx_unlock(&Giant);     /* newbus suckage dealt with, release Giant */
     }
 
-    /* catch running request if any */
+    /* catch request in ch->running if we havn't already */
     ata_catch_inflight(ch);
 
     /* we're done release the channel for new work */
@@ -247,10 +249,11 @@ ata_suspend(device_t dev)
 {
     struct ata_channel *ch;
 
+    /* check for valid device */
     if (!dev || !(ch = device_get_softc(dev)))
 	return ENXIO;
 
-    /* wait for the channel to be IDLE before when enter suspend mode */
+    /* wait for the channel to be IDLE before entering suspend mode */
     while (1) {
 	mtx_lock(&ch->state_mtx);
 	if (ch->state == ATA_IDLE) {
@@ -271,10 +274,11 @@ ata_resume(device_t dev)
     struct ata_channel *ch;
     int error;
 
+    /* check for valid device */
     if (!dev || !(ch = device_get_softc(dev)))
 	return ENXIO;
 
-    /* reinit the devices, we dont know what mode/state they have */
+    /* reinit the devices, we dont know what mode/state they are in */
     error = ata_reinit(dev);
 
     /* kick off requests on the queue */
@@ -304,18 +308,15 @@ ata_interrupt(void *data)
 	}
 
 	/* check for the right state */
-	if (ch->state == ATA_ACTIVE || ch->state == ATA_STALL_QUEUE) {
-	    request->flags |= ATA_R_INTR_SEEN;
-	}
-	else {
+	if (ch->state != ATA_ACTIVE && ch->state != ATA_STALL_QUEUE) {
 	    device_printf(request->dev,
 			  "interrupt state=%d unexpected\n", ch->state);
 	    break;
 	}
 
 	/*
-	 * we have the HW locks, so start the tranaction for this request
-	 * if it finishes immediately we dont need to wait for interrupt
+	 * we have the HW locks, so end the tranaction for this request
+	 * if it finishes immediately otherwise wait for next interrupt
 	 */
 	if (ch->hw.end_transaction(request) == ATA_OP_FINISHED) {
 	    ch->running = NULL;
@@ -326,9 +327,6 @@ ata_interrupt(void *data)
 	    ata_finish(request);
 	    return;
 	}
-	else {
-	    request->flags &= ~ATA_R_INTR_SEEN;
-	}
     } while (0);
     mtx_unlock(&ch->state_mtx);
 }
@@ -559,13 +557,13 @@ ata_boot_attach(void)
 	    ata_identify(ch->dev);
 	}
     }
-    root_mount_rel(ata_root_hold_token);
 }
 
+
 /*
  * misc support functions
  */
-device_t
+static device_t
 ata_add_child(device_t parent, struct ata_device *atadev, int unit)
 {
     struct ata_channel *ch = device_get_softc(parent);
@@ -815,7 +813,6 @@ ata_module_event_handler(module_t mod, int what, void *arg)
 		return EIO;
 	    }
 	    ata_delayed_attach->ich_func = (void*)ata_boot_attach;
-	    ata_root_hold_token = root_mount_hold("ATA");
 	    if (config_intrhook_establish(ata_delayed_attach) != 0) {
 		printf("ata: config_intrhook_establish failed\n");
 		free(ata_delayed_attach, M_TEMP);
diff --git a/sys/dev/ata/ata-all.h b/sys/dev/ata/ata-all.h
index 10b18d6bcbf4..c8aeb3c88f3c 100644
--- a/sys/dev/ata/ata-all.h
+++ b/sys/dev/ata/ata-all.h
@@ -277,12 +277,10 @@ struct ata_request {
 #define         ATA_R_CONTROL           0x00000001
 #define         ATA_R_READ              0x00000002
 #define         ATA_R_WRITE             0x00000004
-#define         ATA_R_DMA               0x00000008
-
-#define         ATA_R_ATAPI             0x00000010
+#define         ATA_R_ATAPI             0x00000008
+#define         ATA_R_DMA               0x00000010
 #define         ATA_R_QUIET             0x00000020
-#define         ATA_R_INTR_SEEN         0x00000040
-#define         ATA_R_TIMEOUT           0x00000080
+#define         ATA_R_TIMEOUT           0x00000040
 
 #define         ATA_R_ORDERED           0x00000100
 #define         ATA_R_AT_HEAD           0x00000200
@@ -468,6 +466,7 @@ int ata_atapicmd(struct ata_device *atadev, u_int8_t *ccb, caddr_t data, int cou
 void ata_queue_request(struct ata_request *request);
 void ata_start(device_t dev);
 void ata_finish(struct ata_request *request);
+void ata_timeout(struct ata_request *);
 void ata_catch_inflight(struct ata_channel *ch);
 void ata_fail_requests(struct ata_channel *ch, device_t dev);
 char *ata_cmd2str(struct ata_request *request);
diff --git a/sys/dev/ata/ata-lowlevel.c b/sys/dev/ata/ata-lowlevel.c
index 717184d625bf..b6bf9e1c8495 100644
--- a/sys/dev/ata/ata-lowlevel.c
+++ b/sys/dev/ata/ata-lowlevel.c
@@ -162,7 +162,7 @@ ata_generic_hw(struct ata_channel *ch)
     ch->hw.command = ata_generic_command;
 }
 
-/* must be called with ATA channel locked */
+/* must be called with ATA channel locked and state_mtx held */
 static int
 ata_begin_transaction(struct ata_request *request)
 {
@@ -192,7 +192,7 @@ ata_begin_transaction(struct ata_request *request)
 		device_printf(request->dev, "error issueing %s command\n",
 			   ata_cmd2str(request));
 		request->result = EIO;
-		break;
+		goto begin_finished;
 	    }
 
 	    /* device reset doesn't interrupt */
@@ -204,7 +204,7 @@ ata_begin_transaction(struct ata_request *request)
 		} while (request->status & ATA_S_BUSY && timeout--);
 		if (request->status & ATA_S_ERROR)
 		    request->error = ATA_IDX_INB(ch, ATA_ERROR);
-		break;
+		goto begin_finished;
 	    }
 
 	    /* if write command output the data */
@@ -213,12 +213,12 @@ ata_begin_transaction(struct ata_request *request)
 			     (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) < 0) {
 		    device_printf(request->dev,"timeout waiting for write DRQ");
 		    request->result = EIO;
-		    break;
+		    goto begin_finished;
 		}
 		ata_pio_write(request, request->transfersize);
 	    }
 	}
-	return ATA_OP_CONTINUES;
+	goto begin_continue;
 
     /* ATA DMA data transfer commands */
     case ATA_R_DMA:
@@ -227,7 +227,7 @@ ata_begin_transaction(struct ata_request *request)
 			  request->flags & ATA_R_READ)) {
 	    device_printf(request->dev, "setting up DMA failed\n");
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* issue command */
@@ -237,16 +237,16 @@ ata_begin_transaction(struct ata_request *request)
 	    device_printf(request->dev, "error issueing %s command\n",
 		       ata_cmd2str(request));
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* start DMA engine */
 	if (ch->dma->start(ch)) {
 	    device_printf(request->dev, "error starting DMA\n");
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
-	return ATA_OP_CONTINUES;
+	goto begin_continue;
 
     /* ATAPI PIO commands */
     case ATA_R_ATAPI:
@@ -256,7 +256,7 @@ ata_begin_transaction(struct ata_request *request)
 	    DELAY(10);
 	    if (!(ATA_IDX_INB(ch, ATA_ALTSTAT) & ATA_S_DSC))
 		request->result = EBUSY;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* start ATAPI operation */
@@ -264,12 +264,12 @@ ata_begin_transaction(struct ata_request *request)
 			   request->transfersize << 8, 0, 0)) {
 	    device_printf(request->dev, "error issuing ATA PACKET command\n");
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* command interrupt device ? just return and wait for interrupt */
 	if ((atadev->param.config & ATA_DRQ_MASK) == ATA_DRQ_INTR)
-	    return ATA_OP_CONTINUES;
+	    goto begin_continue;
 
 	/* wait for ready to write ATAPI command block */
 	{
@@ -286,7 +286,7 @@ ata_begin_transaction(struct ata_request *request)
 	    if (timeout <= 0) {
 		device_printf(request->dev,"timeout waiting for ATAPI ready\n");
 		request->result = EIO;
-		break;
+		goto begin_finished;
 	    }
 	}
 
@@ -298,7 +298,7 @@ ata_begin_transaction(struct ata_request *request)
 			   (int16_t *)request->u.atapi.ccb,
 			   (atadev->param.config & ATA_PROTO_MASK) ==
 			   ATA_PROTO_ATAPI_12 ? 6 : 8);
-	return ATA_OP_CONTINUES;
+	goto begin_continue;
 
     case ATA_R_ATAPI|ATA_R_DMA:
 	/* is this just a POLL DSC command ? */
@@ -307,7 +307,7 @@ ata_begin_transaction(struct ata_request *request)
 	    DELAY(10);
 	    if (!(ATA_IDX_INB(ch, ATA_ALTSTAT) & ATA_S_DSC))
 		request->result = EBUSY;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* check sanity, setup SG list and DMA engine */
@@ -315,14 +315,14 @@ ata_begin_transaction(struct ata_request *request)
 			  request->flags & ATA_R_READ)) {
 	    device_printf(request->dev, "setting up DMA failed\n");
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* start ATAPI operation */
 	if (ch->hw.command(atadev, ATA_PACKET_CMD, 0, 0, ATA_F_DMA)) {
 	    device_printf(request->dev, "error issuing ATAPI packet command\n");
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
 
 	/* wait for ready to write ATAPI command block */
@@ -340,7 +340,7 @@ ata_begin_transaction(struct ata_request *request)
 	    if (timeout <= 0) {
 		device_printf(request->dev,"timeout waiting for ATAPI ready\n");
 		request->result = EIO;
-		break;
+		goto begin_finished;
 	    }
 	}
 
@@ -356,17 +356,25 @@ ata_begin_transaction(struct ata_request *request)
 	/* start DMA engine */
 	if (ch->dma->start(ch)) {
 	    request->result = EIO;
-	    break;
+	    goto begin_finished;
 	}
-	return ATA_OP_CONTINUES;
+	goto begin_continue;
     }
+    /* NOT REACHED */
+    printf("ata_begin_transaction OOPS!!!\n");
 
-    /* request finish here */
+begin_finished:
     if (ch->dma && ch->dma->flags & ATA_DMA_LOADED)
 	ch->dma->unload(ch);
     return ATA_OP_FINISHED;
+
+begin_continue:
+    callout_reset(&request->callout, request->timeout * hz,
+		  (timeout_t*)ata_timeout, request);
+    return ATA_OP_CONTINUES;
 }
 
+/* must be called with ATA channel locked and state_mtx held */
 static int
 ata_end_transaction(struct ata_request *request)
 {
@@ -386,8 +394,7 @@ ata_end_transaction(struct ata_request *request)
 
 	/* on timeouts we have no data or anything so just return */
 	if (request->flags & ATA_R_TIMEOUT)
-	    //return ATA_OP_FINISHED;
-	    break;
+	    goto end_finished;
 
 	/* on control commands read back registers to the request struct */
 	if (request->flags & ATA_R_CONTROL) {
@@ -418,8 +425,7 @@ ata_end_transaction(struct ata_request *request)
 	/* if we got an error we are done with the HW */
 	if (request->status & ATA_S_ERROR) {
 	    request->error = ATA_IDX_INB(ch, ATA_ERROR);
-	    //return ATA_OP_FINISHED;
-	    break;
+	    goto end_finished;
 	}
 	
 	/* are we moving data ? */
@@ -431,8 +437,7 @@ ata_end_transaction(struct ata_request *request)
 			     (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) < 0) {
 		    device_printf(request->dev, "timeout waiting for read DRQ");
 		    request->result = EIO;
-		    //return ATA_OP_FINISHED;
-		    break;
+		    goto end_finished;
 		}
 		ata_pio_read(request, request->transfersize);
 	    }
@@ -457,23 +462,21 @@ ata_end_transaction(struct ata_request *request)
 			device_printf(request->dev,
 				      "timeout waiting for write DRQ");
 			request->status = ATA_IDX_INB(ch, ATA_STATUS);
-			//return ATA_OP_FINISHED;
-			break;
+			goto end_finished;
 		    }
 
 		    /* output data and return waiting for new interrupt */
 		    ata_pio_write(request, request->transfersize);
-		    return ATA_OP_CONTINUES;
+		    goto end_continue;
 		}
 
 		/* if data read command, return & wait for interrupt */
 		if (request->flags & ATA_R_READ)
-		    return ATA_OP_CONTINUES;
+		    goto end_continue;
 	    }
 	}
 	/* done with HW */
-	//return ATA_OP_FINISHED;
-	break;
+	goto end_finished;
 
     /* ATA DMA data transfer commands */
     case ATA_R_DMA:
@@ -494,8 +497,7 @@ ata_end_transaction(struct ata_request *request)
 	ch->dma->unload(ch);
 
 	/* done with HW */
-	//return ATA_OP_FINISHED;
-	break;
+	goto end_finished;
 
     /* ATAPI PIO commands */
     case ATA_R_ATAPI:
@@ -503,8 +505,7 @@ ata_end_transaction(struct ata_request *request)
 
 	/* on timeouts we have no data or anything so just return */
 	if (request->flags & ATA_R_TIMEOUT)
-	    //return ATA_OP_FINISHED;
-	    break;
+	    goto end_finished;
 
 	switch ((ATA_IDX_INB(ch, ATA_IREASON) & (ATA_I_CMD | ATA_I_IN)) |
 		(request->status & ATA_S_DRQ)) {
@@ -516,14 +517,13 @@ ata_end_transaction(struct ata_request *request)
 	    if (!(request->status & ATA_S_DRQ)) {
 		device_printf(request->dev, "command interrupt without DRQ\n");
 		request->status = ATA_S_ERROR;
-		//return ATA_OP_FINISHED;
-		break;
+		goto end_finished;
 	    }
 	    ATA_IDX_OUTSW_STRM(ch, ATA_DATA, (int16_t *)request->u.atapi.ccb,
 			       (atadev->param.config &
 				ATA_PROTO_MASK)== ATA_PROTO_ATAPI_12 ? 6 : 8);
 	    /* return wait for interrupt */
-	    return ATA_OP_CONTINUES;
+	    goto end_continue;
 
 	case ATAPI_P_WRITE:
 	    if (request->flags & ATA_R_READ) {
@@ -531,7 +531,7 @@ ata_end_transaction(struct ata_request *request)
 		device_printf(request->dev,
 			      "%s trying to write on read buffer\n",
 			   ata_cmd2str(request));
-		//return ATA_OP_FINISHED;
+		goto end_finished;
 		break;
 	    }
 	    ata_pio_write(request, length);
@@ -541,7 +541,7 @@ ata_end_transaction(struct ata_request *request)
 	    request->transfersize = min((request->bytecount-request->donecount),
 					request->transfersize);
 	    /* return wait for interrupt */
-	    return ATA_OP_CONTINUES;
+	    goto end_continue;
 
 	case ATAPI_P_READ:
 	    if (request->flags & ATA_R_WRITE) {
@@ -549,8 +549,7 @@ ata_end_transaction(struct ata_request *request)
 		device_printf(request->dev,
 			      "%s trying to read on write buffer\n",
 			   ata_cmd2str(request));
-		//return ATA_OP_FINISHED;
-		break;
+		goto end_finished;
 	    }
 	    ata_pio_read(request, length);
 	    request->donecount += length;
@@ -559,7 +558,7 @@ ata_end_transaction(struct ata_request *request)
 	    request->transfersize = min((request->bytecount-request->donecount),
 					request->transfersize);
 	    /* return wait for interrupt */
-	    return ATA_OP_CONTINUES;
+	    goto end_continue;
 
 	case ATAPI_P_DONEDRQ:
 	    device_printf(request->dev,
@@ -581,8 +580,7 @@ ata_end_transaction(struct ata_request *request)
 	case ATAPI_P_DONE:
 	    if (request->status & (ATA_S_ERROR | ATA_S_DWF))
 		request->error = ATA_IDX_INB(ch, ATA_ERROR);
-	    //return ATA_OP_FINISHED;
-	    break;
+	    goto end_finished;
 
 	default:
 	    device_printf(request->dev, "unknown transfer phase\n");
@@ -590,8 +588,7 @@ ata_end_transaction(struct ata_request *request)
 	}
 
 	/* done with HW */
-	//return ATA_OP_FINISHED;
-	break;
+	goto end_finished;
 
     /* ATAPI DMA commands */
     case ATA_R_ATAPI|ATA_R_DMA:
@@ -612,14 +609,17 @@ ata_end_transaction(struct ata_request *request)
 	ch->dma->unload(ch);
 
 	/* done with HW */
-	//return ATA_OP_FINISHED;
-	break;
+	goto end_finished;
     }
+    /* NOT REACHED */
+    printf("ata_end_transaction OOPS!!\n");
 
-    /* disable interrupt */
-    //ATA_IDX_OUTB(ch, ATA_CONTROL, ATA_A_4BIT | ATA_A_IDS);
-
+end_finished:
+    callout_stop(&request->callout);
     return ATA_OP_FINISHED;
+
+end_continue:
+    return ATA_OP_CONTINUES;
 }
 
 /* must be called with ATA channel locked */
@@ -658,7 +658,7 @@ ata_generic_reset(struct ata_channel *ch)
 		      mask, ostat0, ostat1);
 
     /* if nothing showed up there is no need to get any further */
-    /* SOS is that too strong?, we just might loose devices here XXX */
+    /* XXX SOS is that too strong?, we just might loose devices here */
     ch->devices = 0;
     if (!mask)
 	return;
@@ -685,7 +685,8 @@ ata_generic_reset(struct ata_channel *ch)
 		device_printf(ch->dev,
 			      "stat0=0x%02x err=0x%02x lsb=0x%02x msb=0x%02x\n",
 			      stat0, err, lsb, msb);
-	    if (stat0 == err && lsb == err && msb == err && timeout > 10)
+	    if (stat0 == err && lsb == err && msb == err &&
+		timeout > (stat0 & ATA_S_BUSY ? 100 : 10))
 		mask &= ~0x01;
 	    if (!(stat0 & ATA_S_BUSY)) {
 		if ((err & 0x7f) == ATA_E_ILI) {
@@ -714,7 +715,8 @@ ata_generic_reset(struct ata_channel *ch)
 		device_printf(ch->dev,
 			      "stat1=0x%02x err=0x%02x lsb=0x%02x msb=0x%02x\n",
 			      stat1, err, lsb, msb);
-	    if (stat1 == err && lsb == err && msb == err && timeout > 10)
+	    if (stat1 == err && lsb == err && msb == err &&
+		timeout > (stat1 & ATA_S_BUSY ? 100 : 10))
 		mask &= ~0x02;
 	    if (!(stat1 & ATA_S_BUSY)) {
 		if ((err & 0x7f) == ATA_E_ILI) {
diff --git a/sys/dev/ata/ata-queue.c b/sys/dev/ata/ata-queue.c
index f4bdce16f4ae..133f8ade75e6 100644
--- a/sys/dev/ata/ata-queue.c
+++ b/sys/dev/ata/ata-queue.c
@@ -47,7 +47,6 @@ __FBSDID("$FreeBSD$");
 
 /* prototypes */
 static void ata_completed(void *, int);
-static void ata_timeout(struct ata_request *);
 static void ata_sort_queue(struct ata_channel *ch, struct ata_request *request);
 static char *ata_skey2str(u_int8_t);
 
@@ -56,32 +55,28 @@ ata_queue_request(struct ata_request *request)
 {
     struct ata_channel *ch = device_get_softc(device_get_parent(request->dev));
 
-    /* mark request as virgin */
+    /* mark request as virgin (this might be a ATA_R_REQUEUE) */
     request->result = request->status = request->error = 0;
-    callout_init(&request->callout, 1);
+    callout_init_mtx(&request->callout, &ch->state_mtx, CALLOUT_RETURNUNLOCKED);
 
     if (!request->callback && !(request->flags & ATA_R_REQUEUE))
 	sema_init(&request->done, 0, "ATA request done");
 
     /* in ATA_STALL_QUEUE state we call HW directly (used only during reinit) */
     if ((ch->state & ATA_STALL_QUEUE) && (request->flags & ATA_R_CONTROL)) {
-
-	/* arm timeout */
-	if (!dumping)
-	    callout_reset(&request->callout, request->timeout * hz,
-			  (timeout_t*)ata_timeout, request);
-	/* kick HW into action */
+	mtx_lock(&ch->state_mtx);
 	ch->running = request;
 	if (ch->hw.begin_transaction(request) == ATA_OP_FINISHED) {
 	    ch->running = NULL;
-	    callout_drain(&request->callout);
 	    if (!request->callback) 
 		sema_destroy(&request->done);
+	    mtx_unlock(&ch->state_mtx);
 	    return;
 	}
+	mtx_unlock(&ch->state_mtx);
     }
+    /* otherwise put request on the locked queue at the specified location */
     else  {
-	/* put request on the locked queue at the specified location */
 	mtx_lock(&ch->queue_mtx);
 	if (request->flags & ATA_R_AT_HEAD)
 	    TAILQ_INSERT_HEAD(&ch->ata_queue, request, chain);
@@ -94,7 +89,7 @@ ata_queue_request(struct ata_request *request)
 	ata_start(ch->dev);
     }
 
-    /* if this is a requeued request callback/sleep has been setup */
+    /* if this is a requeued request callback/sleep we're done */
     if (request->flags & ATA_R_REQUEUE)
 	return;
 
@@ -184,17 +179,11 @@ ata_start(device_t dev)
 		mtx_unlock(&cptr->lock);
 	    }
 
-	    /* check we are int the right state and has no dependencies */
+	    /* check we are in the right state and has no dependencies */
 	    mtx_lock(&ch->state_mtx);
 	    if (ch->state == ATA_IDLE && !dependencies) {
 		ATA_DEBUG_RQ(request, "starting");
-#if 0
-		if (request->sequence_count)
-		    request = request->sequence[--request->sequence_count];
-		else
-#else
 		TAILQ_REMOVE(&ch->ata_queue, request, chain);
-#endif
 		ch->running = request;
 		ch->state = ATA_ACTIVE;
 
@@ -202,10 +191,6 @@ ata_start(device_t dev)
 		if (ch->freezepoint == request)
 		    ch->freezepoint = NULL;
 
-		/* arm timeout for this request */
-		if (!dumping)
-		    callout_reset(&request->callout, request->timeout * hz,
-				  (timeout_t*)ata_timeout, request);
 		if (ch->hw.begin_transaction(request) == ATA_OP_FINISHED) {
 		    ch->running = NULL;
 		    ch->state = ATA_IDLE;
@@ -227,16 +212,16 @@ ata_finish(struct ata_request *request)
 {
     struct ata_channel *ch = device_get_softc(device_get_parent(request->dev));
 
-    /* if in STALL_QUEUE state or request is ATA_R_DIRECT call complete now */
+    /*
+     * if in ATA_STALL_QUEUE state or request has ATA_R_DIRECT flags set
+     * we need to call ata_complete() directly here (no taskqueue involvement)
+     */
     if ((ch->state & ATA_STALL_QUEUE) || (request->flags & ATA_R_DIRECT)) {
 	ATA_DEBUG_RQ(request, "finish directly");
 	ata_completed(request, 0);
     }
     else {
-	/* reset timeout and put on the proper taskqueue for completition */
-	if (!dumping && !(request->flags & ATA_R_TIMEOUT))
-	    callout_reset(&request->callout, request->timeout * hz,
-			  (timeout_t*)ata_timeout, request);
+	/* put request on the proper taskqueue for completition */
 	if (request->bio && !(request->flags & (ATA_R_THREAD | ATA_R_TIMEOUT))){
 	    ATA_DEBUG_RQ(request, "finish bio_taskqueue");
 	    bio_taskqueue(request->bio, (bio_task_t *)ata_completed, request);
@@ -262,10 +247,19 @@ ata_completed(void *context, int dummy)
     /* if we had a timeout, reinit channel and deal with the falldown */
     if (request->flags & ATA_R_TIMEOUT) {
 	/*
-	 * if reinit succeeds, retries still permit and device didn't
-	 * get removed by the reinit, reinject request
+	 * if reinit succeeds and the device doesn't get detached and
+	 * there are retries left we reinject this request
 	 */
 	if (!ata_reinit(ch->dev) && request->dev && (request->retries-- > 0)) {
+	    if (!(request->flags & ATA_R_QUIET)) {
+		device_printf(request->dev,
+			      "TIMEOUT - %s retrying (%d retr%s left)",
+			      ata_cmd2str(request), request->retries,
+			      request->retries == 1 ? "y" : "ies");
+		if (!(request->flags & (ATA_R_ATAPI | ATA_R_CONTROL)))
+		    printf(" LBA=%llu", (unsigned long long)request->u.ata.lba);
+		printf("\n");
+	    }
 	    request->flags &= ~(ATA_R_TIMEOUT | ATA_R_DEBUG);
 	    request->flags |= (ATA_R_AT_HEAD | ATA_R_REQUEUE);
 	    ATA_DEBUG_RQ(request, "completed reinject");
@@ -273,7 +267,7 @@ ata_completed(void *context, int dummy)
 	    return;
 	}
 
-	/* nothing more to try so finish with error */
+	/* ran out of good intentions so finish with error */
 	if (!(request->flags & ATA_R_QUIET)) {
 	    if (request->dev) {
 		device_printf(request->dev,
@@ -288,10 +282,8 @@ ata_completed(void *context, int dummy)
 	    request->result = EIO;
     }
     else {
-	/* untimeout request now we have control back */
-	callout_drain(&request->callout);
-
 	/* if this is a soft ECC error warn about it */
+	/* XXX SOS we could do WARF here */
 	if ((request->status & (ATA_S_CORR | ATA_S_ERROR)) == ATA_S_CORR) {
 	    device_printf(request->dev,
 			  "WARNING - %s soft error (ECC corrected)",
@@ -301,7 +293,7 @@ ata_completed(void *context, int dummy)
 	    printf("\n");
 	}
 
-	/* if this is a UDMA CRC error, retry request */
+	/* if this is a UDMA CRC error we reinject if there are retries left */
 	if (request->flags & ATA_R_DMA && request->error & ATA_E_ICRC) {
 	    if (request->retries-- > 0) {
 		device_printf(request->dev,
@@ -447,49 +439,20 @@ ata_completed(void *context, int dummy)
     ata_start(ch->dev);
 }
 
-static void
+void
 ata_timeout(struct ata_request *request)
 {
     struct ata_channel *ch = device_get_softc(device_get_parent(request->dev));
 
-    mtx_lock(&ch->state_mtx);
-
     //request->flags |= ATA_R_DEBUG;
     ATA_DEBUG_RQ(request, "timeout");
 
-    /* if interrupt has been seen, shout and just rearm timeout */
-    if (request->flags & ATA_R_INTR_SEEN) {
-	device_printf(request->dev,
-		      "WARNING - %s interrupt was seen but timeout fired",
-		      ata_cmd2str(request));
-	if (!(request->flags & (ATA_R_ATAPI | ATA_R_CONTROL)))
-	    printf(" LBA=%llu", (unsigned long long)request->u.ata.lba);
-	printf("\n");
-
-	/* re-arm timeout */
-	if (!dumping)
-	    callout_reset(&request->callout, request->timeout * hz,
-			  (timeout_t*)ata_timeout, request);
-	mtx_unlock(&ch->state_mtx);
-	return;
-    }
-
     /*
-     * grap and hold the state lock so we wont loose the race with 
+     * set the state to TIMEOUT so we wont loose the race with 
      * an eventual interrupt arriving late
      */
     if (ch->state == ATA_ACTIVE || ch->state == ATA_STALL_QUEUE) {
 	request->flags |= ATA_R_TIMEOUT;
-	if (!(request->flags & ATA_R_QUIET) && request->retries > 0) {
-	    device_printf(request->dev,
-			  "TIMEOUT - %s retrying (%d retr%s left)",
-			  ata_cmd2str(request), request->retries,
-			  request->retries == 1 ? "y" : "ies");
-	    if (!(request->flags & (ATA_R_ATAPI | ATA_R_CONTROL)))
-		printf(" LBA=%llu", (unsigned long long)request->u.ata.lba);
-	    printf("\n");
-	}
-	ch->hw.end_transaction(request);
 	ch->state |= ATA_TIMEOUT;
 	mtx_unlock(&ch->state_mtx);
 	ATA_LOCKING(ch->dev, ATA_LF_UNLOCK);
@@ -507,12 +470,11 @@ ata_catch_inflight(struct ata_channel *ch)
     struct ata_request *request;
 
     mtx_lock(&ch->state_mtx);
-    request = ch->running;
+    if ((request = ch->running))
+        callout_stop(&request->callout);
     ch->running = NULL;
     mtx_unlock(&ch->state_mtx);
-
     if (request) {
-	callout_drain(&request->callout);
 	device_printf(request->dev,
 		      "WARNING - %s requeued due to channel reset",
 		      ata_cmd2str(request));
@@ -541,18 +503,6 @@ ata_fail_requests(struct ata_channel *ch, device_t dev)
 	}
     }
     mtx_unlock(&ch->queue_mtx);
-
-    mtx_lock(&ch->state_mtx);
-    request = ch->running;
-    ch->running = NULL;
-    mtx_unlock(&ch->state_mtx);
-
-    /* if we have a request "in flight" fail it as well */
-    if (request && (!dev || request->dev == dev)) {
-	callout_drain(&request->callout);
-	request->result = ENXIO;
-	ata_finish(request);
-    }
 }
 
 static u_int64_t
@@ -709,6 +659,7 @@ ata_cmd2str(struct ata_request *request)
 	case 0xa0: return ("PACKET_CMD");
 	case 0xa1: return ("ATAPI_IDENTIFY");
 	case 0xa2: return ("SERVICE");
+	case 0xc0: return ("CFA ERASE");
 	case 0xc4: return ("READ_MUL");
 	case 0xc5: return ("WRITE_MUL");
 	case 0xc6: return ("SET_MULTI");