Fix a nasty bug in ata_intr, parens are a good thing if used right.

Make the error recovery code a little more obvious.

Inform the user if UDMA66 mode couldn't be selected due to a
non ATA66 compliant 80pin cable.

Minor cosmetics.

3 files changed, 46 insertions, 30 deletions

diff --git a/sys/dev/ata/ata-all.c b/sys/dev/ata/ata-all.c
index 41e802fa170b..552439b10bce 100644
--- a/sys/dev/ata/ata-all.c
+++ b/sys/dev/ata/ata-all.c
@@ -1105,7 +1105,7 @@ ata_intr(void *data)
 #endif
     default:
 	if (scp->flags & ATA_DMA_ACTIVE) {
-	    if (!(dmastat = ata_dmastatus(scp)) & ATA_BMSTAT_INTERRUPT)
+	    if (!((dmastat = ata_dmastatus(scp)) & ATA_BMSTAT_INTERRUPT))
 		return;
 	    else
 		outb(scp->bmaddr+ATA_BMSTAT_PORT, dmastat|ATA_BMSTAT_INTERRUPT);
@@ -1513,9 +1513,9 @@ ata_umode(struct ata_params *ap)
 {
     if (ap->atavalid & ATA_FLAG_88) {
 	if (ap->udmamodes & 0x10)
-	    return (ap->cblid ? 4 : 2);
+	    return 4;
 	if (ap->udmamodes & 0x08)
-	    return (ap->cblid ? 3 : 2);
+	    return 3;
 	if (ap->udmamodes & 0x04)
 	    return 2;
 	if (ap->udmamodes & 0x02)
diff --git a/sys/dev/ata/ata-disk.c b/sys/dev/ata/ata-disk.c
index fa27dcc9416b..a1cb3aa01425 100644
--- a/sys/dev/ata/ata-disk.c
+++ b/sys/dev/ata/ata-disk.c
@@ -413,7 +413,7 @@ ad_transfer(struct ad_request *request)
 	if (ata_command(adp->controller, adp->unit, cmd, 
 			cylinder, head, sector, count, 0, ATA_IMMEDIATE)) {
 	    printf("ad%d: error executing command\n", adp->lun);
-	    return;
+	    goto transfer_failed;
 	}
 
 	/* if this is a DMA transfer, start it, return and wait for interrupt */
@@ -432,9 +432,11 @@ ad_transfer(struct ad_request *request)
 
     /* ready to write PIO data ? */
     if (ata_wait(adp->controller, adp->unit, 
-		 (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) < 0)
+		 (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) < 0) {
 	printf("ad%d: timeout waiting for DRQ", adp->lun);
-    
+	goto transfer_failed;
+    }
+
     /* output the data */
     if (adp->controller->flags & ATA_USE_16BIT)
 	outsw(adp->controller->ioaddr + ATA_DATA,
@@ -444,6 +446,16 @@ ad_transfer(struct ad_request *request)
 	outsl(adp->controller->ioaddr + ATA_DATA,
 	      (void *)((uintptr_t)request->data + request->donecount),
 	      request->currentsize / sizeof(int32_t));
+    return;
+
+transfer_failed:
+    untimeout((timeout_t *)ad_timeout, request, request->timeout_handle);
+    request->bp->bio_error = EIO;
+    request->bp->bio_flags |= BIO_ERROR;
+    request->bp->bio_resid = request->bytecount;
+    devstat_end_transaction_bio(&adp->stats, request->bp);
+    biodone(request->bp);
+    free(request, M_AD);
 }
 
 int32_t
@@ -457,8 +469,10 @@ ad_interrupt(struct ad_request *request)
 	dma_stat = ata_dmadone(adp->controller);
 
     /* get drive status */
-    if (ata_wait(adp->controller, adp->unit, 0) < 0)
-	 printf("ad%d: timeout waiting for status", adp->lun);
+    if (ata_wait(adp->controller, adp->unit, 0) < 0) {
+	printf("ad%d: timeout waiting for status", adp->lun);
+	request->flags |= ADR_F_ERROR;
+    }
 
     /* do we have a corrected soft error ? */
     if (adp->controller->status & ATA_S_CORR)
@@ -467,7 +481,6 @@ ad_interrupt(struct ad_request *request)
     /* did any real errors happen ? */
     if ((adp->controller->status & ATA_S_ERROR) ||
 	((request->flags & ADR_F_DMA_USED) && (dma_stat & ATA_BMSTAT_ERROR))) {
-oops:
 	printf("ad%d: %s %s ERROR blk# %d", adp->lun,
 	       (adp->controller->error & ATA_E_ICRC) ? "UDMA ICRC" : "HARD",
 	       (request->flags & ADR_F_READ) ? "READ" : "WRITE",
@@ -518,20 +531,20 @@ oops:
 
 	if (ata_wait(adp->controller, adp->unit, 
 		     (ATA_S_READY | ATA_S_DSC | ATA_S_DRQ)) != 0) {
-	    printf("ad%d: read error detected late", adp->lun);
-	    goto oops;	 
+	    printf("ad%d: read error detected (too) late", adp->lun);
+	    request->flags |= ADR_F_ERROR;
+	}
+	else {
+	    /* data ready, read in */
+	    if (adp->controller->flags & ATA_USE_16BIT)
+		insw(adp->controller->ioaddr + ATA_DATA,
+		     (void *)((uintptr_t)request->data + request->donecount), 
+		     request->currentsize / sizeof(int16_t));
+	    else
+		insl(adp->controller->ioaddr + ATA_DATA,
+		     (void *)((uintptr_t)request->data + request->donecount), 
+		     request->currentsize / sizeof(int32_t));
 	}
-
-	/* data ready, read in */
-	if (adp->controller->flags & ATA_USE_16BIT)
-	    insw(adp->controller->ioaddr + ATA_DATA,
-		 (void *)((uintptr_t)request->data + request->donecount), 
-		 request->currentsize / sizeof(int16_t));
-	else
-	    insl(adp->controller->ioaddr + ATA_DATA,
-		 (void *)((uintptr_t)request->data + request->donecount), 
-		 request->currentsize / sizeof(int32_t));
-
     }
 
     /* finish up transfer */
@@ -548,13 +561,12 @@ oops:
 	}
     }
 
-    request->bp->bio_resid = request->bytecount;
-    devstat_end_transaction_bio(&adp->stats, request->bp);
-    biodone(request->bp);
-
     /* disarm timeout for this transfer */
     untimeout((timeout_t *)ad_timeout, request, request->timeout_handle);
 
+    request->bp->bio_resid = request->bytecount;
+    devstat_end_transaction_bio(&adp->stats, request->bp);
+    biodone(request->bp);
     free(request, M_AD);
     return ATA_OP_FINISHED;
 }
diff --git a/sys/dev/ata/ata-dma.c b/sys/dev/ata/ata-dma.c
index cd586ca4650d..694705726e75 100644
--- a/sys/dev/ata/ata-dma.c
+++ b/sys/dev/ata/ata-dma.c
@@ -93,6 +93,11 @@ ata_dmainit(struct ata_softc *scp, int32_t device,
 	}
 	scp->dmatab[ATA_DEV(device)] = dmatab;
     }
+    if (udmamode > 2 && !ATA_PARAM(scp, device)->cblid) {
+	ata_printf(scp, device,
+		   "DMA limitted to UDMA33, non-ATA66 compliant cable\n");
+	udmamode = 2;
+    }
 
     switch (scp->chiptype) {
 
@@ -250,8 +255,8 @@ ata_dmainit(struct ata_softc *scp, int32_t device,
 			   "%s setting up UDMA2 mode on Aladdin chip\n",
 			   (error) ? "failed" : "success");
 	    if (!error) {
-		word54 |= 0x5555;
-		word54 |= (0x0a << (16 + (scp->unit << 3) + (device << 2)));
+		word54 &= ~(0x000f000f << (devno << 2));
+		word54 |= (0x000a0005 << (devno << 2));
 		pci_write_config(parent, 0x54, word54, 4);
 		pci_write_config(parent, 0x53, 
 				 pci_read_config(parent, 0x53, 1) | 0x03, 1);
@@ -335,8 +340,7 @@ via_82c686:
     case 0x05861106:	/* VIA 82C586 */
 via_82c586:
 	/* UDMA2 mode only on 82C586 > rev1, 82C596, AMD 756 */
-	if ((udmamode >= 2 && scp->chiptype == 0x05861106 &&
-	     pci_read_config(scp->dev, 0x08, 1) >= 0x01) ||
+	if ((udmamode >= 2 && ata_find_dev(parent, 0x05861106, 0x02)) ||
 	    (udmamode >= 2 && scp->chiptype == 0x05961106) ||
 	    (udmamode >= 2 && scp->chiptype == 0x74091022)) {
 	    error = ata_command(scp, device, ATA_C_SETFEATURES, 0, 0, 0,