ATA/ATAPI devices with PIO mode < 3 are probed incorrectly

I have an ATAPI tape drive (Exabyte Eagle TR-4i) that was giving a lot of errors during writes. I tracked the problem down to the fact that the drive only supports PIO mode 2 but was being set up by the kernel as PIO mode 3. In wdc.c and atapiscsi.c there are statements like "All ATAPI devices are at least PIO mode 3" and corresponding code to force mode 3. I can't find anything in the specs to say that ATAPI devices must be mode 3, and I personally have at least two PIO mode 2 ATAPI devices (one being the tape drive mentioned above, another being a Sanyo CD drive). The probe is also unable to detect the PIO mode of any ATA device that doesn't support at least mode 3, because it only looks at the IDENTIFY DEVICE word for "advanced" PIO modes, i.e. those >= 3. The result is that any device with a mode < 3 ends up getting configured as mode 0 if it's ATA or mode 3 if it's ATAPI. The 3 diffs below fix it, except that on some chipsets pciide.c will, for reasons unknown to me, set the mode to 0 if it's < 3. Can anybody tell me why that is done?

These diffs are against 3.2-stable, but they apply fine to -current.

• wdc.c.3.2 Fri Dec 13 12:34:11 2002 +++ wdc.c Sat Dec 14 12:38:01 2002
  @@ -1235,67 +1235,126 @@
  } } else #endif - /* An ATAPI device is at last PIO mode 3 */ - if (drvp->drive_flags & DRIVE_ATAPI) - drvp->PIO_mode = 3; - WDCDEBUG_PRINT(("wdc_probe_caps: wdc_cap %d cf_flags %d\n", wdc->cap, cf_flags), DEBUG_PROBE);
  • /*
  • * It's not in the specs, but it seems that some drive
  • * returns 0xffff in atap_extensions when this field is invalid
  • */
  • if (params->atap_extensions != 0xffff &&
  • (params->atap_extensions & WDC_EXT_MODES)) {
  • valid_mode_found = 0; + valid_mode_found = 0; + for (i = 7; i >= 0; i--) { /*
  • * XXX some drives report something wrong here (they claim to
  • * support PIO mode 8 !). As mode is coded on 3 bits in
  • * SET FEATURE, limit it to 7 (so limit i to 4).
  • * If higther mode than 7 is found, abort. + * If atap_piomode_supp is valid, try to use it first. + * It can only contain mode 3 or higher. */
  • for (i = 7; i >= 0; i--) {
  • if ((params->atap_piomode_supp & (1 << i)) == 0)
  • continue;
  • if (i > 4)
  • return; -
  • valid_mode_found = 1; -
  • if ((wdc->cap & WDC_CAPABILITY_MODE) == 0) {
  • drvp->PIO_cap = i + 3;
  • continue;
  • } -
  • /*
  • * See if mode is accepted.
  • * If the controller can't set its PIO mode,
  • * assume the BIOS set it up correctly + if (i > 2) { + /* + * It's against the specs, but it seems that some drive + * returns 0xffff in atap_extensions when this field is + * invalid. + * + * Some drives report something wrong in + * atap_piomode_supp (they claim to support PIO + * mode 8!). As mode is coded on 3 bits in SET FEATURE, + * limit it to 7 (bits 5-7 are invalid). */
  • if (ata_set_mode(drvp, 0x08 | (i + 3),
  • at_poll) != CMD_OK)
  • continue; - + if (params->atap_extensions != 0xffff && + (params->atap_extensions & WDC_EXT_MODES) && + (params->atap_piomode_supp & 0xe0) == 0) { + if (params->atap_piomode_supp + & (1 << (i - 3))) { + valid_mode_found = 1; + WDCDEBUG_PRINT( + ("wdc_probe_caps: %s:" + " atap_piomode_supp" + " contains mode %d\n", + drvp->drive_name, i), + DEBUG_PROBE); + } + } + } + /* + * If there's no valid mode in atap_piomode_supp then try + * atap_oldpiotiming instead. A device may set WDC_EXT_MODES + * and still legitimately have no bits set in atap_piomode_supp + * if it does not support mode 3 or higher. + */ + if (!valid_mode_found && i <= 2) { /*
  • * If controller's driver can't set its PIO mode,
  • * set the highest one the controller supports + * ATA-1 and ATA-2 want word 51 to contain the cycle + * time in ns. ATA-3 is ambiguous about whether it + * wants it to contain the cycle time or the mode. + * ATA-4 wants it to contain the mode; 0, 1, or 2. */
  • if (wdc->PIO_cap >= i + 3) {
  • drvp->PIO_mode = i + 3;
  • drvp->PIO_cap = i + 3;
  • break; + if (params->atap_oldpiotiming == i) { + valid_mode_found = 1; + WDCDEBUG_PRINT(("wdc_probe_caps: %s:" + " atap_oldpiotiming contains" + " mode %d\n", drvp->drive_name, i), + DEBUG_PROBE); + /* + * A device with a cycle time of 180ns or less is at + * least PIO mode 3 and should be reporting that in + * atap_piomode_supp, so ignore it here. + */ + } else if (params->atap_oldpiotiming > 180) { + if ((params->atap_oldpiotiming + <= 240 && i == 2) || + (params->atap_oldpiotiming + <= 480 && i == 1) || + (params->atap_oldpiotiming + <= 600 && i == 0)) { + valid_mode_found = 1; + WDCDEBUG_PRINT(("wdc_probe_caps: %s:" + " atap_oldpiotiming contains a" + " cycle time of %dns (mode %d)\n", + drvp->drive_name, + params->atap_oldpiotiming, i), + DEBUG_PROBE); + } } }
  • if (!valid_mode_found) {
  • /*
  • * We didn't find a valid PIO mode.
  • * Assume the values returned for DMA are buggy too
  • */
  • return; + if (valid_mode_found) { + if (wdc->cap & WDC_CAPABILITY_MODE) { + if (wdc->PIO_cap >= i) { + /* + * SET FEATURES 0x08 is only for + * PIO modes > 2 + */ + if (i > 2 && + ata_set_mode(drvp, 0x08 | i, + at_poll) != CMD_OK) { + WDCDEBUG_PRINT(( + "wdc_probe_caps: %s:" + " failed to set PIO" + " mode %d\n", + drvp->drive_name, i), + DEBUG_PROBE); + continue; + } + } else { + WDCDEBUG_PRINT(("wdc_probe_caps: %s:" + " controller can't support PIO" + " mode %d\n", drvp->drive_name, i), + DEBUG_PROBE); + continue; + } + } + drvp->PIO_mode = i; + drvp->PIO_cap = i; + drvp->drive_flags |= DRIVE_MODE; + WDCDEBUG_PRINT(("wdc_probe_caps: %s: using PIO mode" + " %d\n", drvp->drive_name, i), DEBUG_PROBE); + break; }
  • drvp->drive_flags |= DRIVE_MODE; + } + if (!valid_mode_found) { + /* + * We didn't find a valid PIO mode. + * Assume the values returned for DMA are buggy too + */ + return; + }

+        if (params->atap_extensions != 0xffff &&
+            (params->atap_extensions & WDC_EXT_MODES)) {
 		/* Some controllers don't support ATAPI DMA */
 		if ((drvp->drive_flags & DRIVE_ATAPI) &&
 		    (wdc->cap & WDC_CAPABILITY_NO_ATAPI_DMA))

--- atapiscsi.c.3.2	Tue May 21 01:42:03 2002
+++ atapiscsi.c	Sat Dec 14 13:59:41 2002


@@ -1447,13 +1447,15 @@


 	}

 	case ATAPI_PIOMODE_STATE:
-piomode:
 		/* Don't try to set mode if controller can't be adjusted */
 		if ((chp->wdc->cap & WDC_CAPABILITY_MODE) == 0)
 			goto ready;
 		/* Also don't try if the drive didn't report its mode */
 		if ((drvp->drive_flags & DRIVE_MODE) == 0)
 			goto ready;
+		/* SET FEATURES 0x08 is only for modes > 2 */
+		if (drvp->PIO_mode <= 2)
+			goto ready;
 		wdccommand(chp, drvp->drive, SET_FEATURES, 0, 0, 0,
 		    0x08 | drvp->PIO_mode, WDSF_SET_MODE);
 		drvp->state = ATAPI_PIOMODE_WAIT_STATE;


@@ -1464,13 +1466,12 @@


 		if (chp->wdc->cap & WDC_CAPABILITY_IRQACK)
 			chp->wdc->irqack(chp);
 		if (chp->ch_status & WDCS_ERR) {
-			if (drvp->PIO_mode < 3) {
-				drvp->PIO_mode = 3;
-				goto piomode;
-			}
 			/*
 			 * All ATAPI drives are supposed to support
 			 * PIO mode 3 or greater.
+			 *
+			 * XXX The above statement is false.  This has been
+			 * working fine in most cases, so it is left here.
 			 */
 			drvp->PIO_mode = 3;
 			chp->wdc->set_modes(chp);

--- ata_wdc.c.3.2	Sat Dec 14 13:32:18 2002
+++ ata_wdc.c	Fri Dec 13 14:35:33 2002


@@ -607,6 +607,9 @@


 		/* Also don't try if the drive didn't report its mode */
 		if ((drvp->drive_flags & DRIVE_MODE) == 0)
 			goto geometry;
+		/* SET FEATURES 0x08 is only for modes > 2 */
+		if (drvp->PIO_mode <= 2)
+			goto geometry;
 		wdccommand(chp, drvp->drive, SET_FEATURES, 0, 0, 0,
 		    0x08 | drvp->PIO_mode, WDSF_SET_MODE);
Do you need help?
In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software:
Learn more about Pantek's Technical Support offerings
Place an order for 24/7/365 Technical Support here
Learn more about Pantek, who we are, and what we do
Chat with a Live Operator
Related openbsd.org Links
advocacy
announce
bugs
ipv6
misc
ports
ports-changes
security-announce
smp
source-changes
tech
Request more information about Pantek
 		drvp->state = PIOMODE_WAIT;