Subject: Re: maxtor sata quirk
To: None <tech-kern@netbsd.org>
From: Manuel Bouyer <bouyer@antioche.eu.org>
List: tech-kern
Date: 03/07/2007 22:37:54
--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Mar 05, 2007 at 01:56:02PM -0500, George Georgalis wrote:
> I seem to have come across a quirk for Maxtor SATA drives:
> 
> wd0 at atabus2 drive 0: <MAXTOR STM3200820AS>
> wd0: drive supports 16-sector PIO transfers, LBA48 addressing
> wd0: 186 GB, 387621 cyl, 16 head, 63 sec, 512 bytes/sect x 390721968 sectors
> wd0: 32-bit data port
> wd0: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133)
> wd0(viaide1:0:0): using PIO mode 4, Ultra-DMA mode 6 (Ultra/133) (using DMA)
> wd1 at atabus3 drive 0: <MAXTOR STM3200820AS>
> wd1: drive supports 16-sector PIO transfers, LBA48 addressing
> wd1: 186 GB, 387621 cyl, 16 head, 63 sec, 512 bytes/sect x 390721968 sectors
> wd1: 32-bit data port
> wd1: drive supports PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133)
> wd1(viaide1:1:0): using PIO mode 4, Ultra-DMA mode 6 (Ultra/133) (using DMA)
> 
> the center command below (only) fails on two new drives.
> 
>     dd if=/dev/rwd1a of=/dev/null skip=268435391 count=1
>     dd if=/dev/rwd1a of=/dev/null skip=268435392 count=1
>     dd if=/dev/rwd1a of=/dev/null skip=268435393 count=1
> 
> with kernel messages:
> 
> wd1a: error reading fsbn 268435392 of 268435392-268435519 (wd1 bn 268435455; cn 266305 tn 0 sn 15), retrying
> wd1: (id not found)
> 
> In hex that's fffffc0 which is a little different than the Seagate
> LBA 32/48 quirk that comes up now and then. I'm looking for
> (hopefully) an existing quirk table that I can add my drive id to.
> 
> If anyone knows where to go from here, please let me know.

Could you try the attached patch against current ? This should fix the
issue for the most common cases, and hopefully it won't break any working
setups.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--huq684BweRXVnRxX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: wd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ata/wd.c,v
retrieving revision 1.337
diff -u -r1.337 wd.c
--- wd.c	4 Mar 2007 06:01:44 -0000	1.337
+++ wd.c	7 Mar 2007 21:35:03 -0000
@@ -107,7 +107,7 @@
 
 #include <prop/proplib.h>
 
-#define	LBA48_THRESHOLD		(0xfffffff)	/* 128GB / DEV_BSIZE */
+#define	LBA48_THRESHOLD		(0x10000000)	/* 128GB / DEV_BSIZE */
 
 #define	WDIORETRIES_SINGLE 4	/* number of retries before single-sector */
 #define	WDIORETRIES	5	/* number of retries before giving up */
@@ -233,29 +233,6 @@
 	  WD_QUIRK_SPLIT_MOD15_WRITE },
 	{ "ST380023AS",
 	  WD_QUIRK_SPLIT_MOD15_WRITE },
-
-	/*
-	 * These seagate drives seems to have issue addressing sector 0xfffffff
-	 * (aka LBA48_THRESHOLD) in LBA mode. The workaround is to force
-	 * LBA48
-	 * Note that we can't just change the code to always use LBA48 for
-	 * sector 0xfffffff, because this would break valid and working
-	 * setups using LBA48 drives on non-LBA48-capable controllers
-	 * (and it's hard to get a list of such controllers)
-	 */
-	{ "ST3160021A*",
-	  WD_QUIRK_FORCE_LBA48 },
-	{ "ST3160811A*",
-	  WD_QUIRK_FORCE_LBA48 },
-	{ "ST3160812A*",
-	  WD_QUIRK_FORCE_LBA48 },
-	{ "ST3160023A*",
-	  WD_QUIRK_FORCE_LBA48 },
-	{ "ST3160827A*",
-	  WD_QUIRK_FORCE_LBA48 },
-	/* Attempt to catch all seagate drives larger than 200GB */
-	{ "ST3[2-9][0-9][0-9][0-9][0-9][0-9][A-Z]*",
-	  WD_QUIRK_FORCE_LBA48 },
 	{ NULL,
 	  0 }
 };
@@ -382,6 +359,12 @@
 		    ((u_int64_t) wd->sc_params.__reserved6[10] << 32) |
 		    ((u_int64_t) wd->sc_params.__reserved6[9]  << 16) |
 		    ((u_int64_t) wd->sc_params.__reserved6[8]  << 0);
+		if (wd->sc_capacity <= LBA48_THRESHOLD &&
+		    (wd->sc_quirks & WD_QUIRK_FORCE_LBA48) == 0 &&
+		    (wd->sc_flags & WDF_LBA) != 0) {
+			/* no need to use LBA48, LBA will do it */
+			wd->sc_flags &= ~WDF_LBA48;
+		}
 	} else if ((wd->sc_flags & WDF_LBA) != 0) {
 		aprint_verbose(" LBA addressing\n");
 		wd->sc_capacity =
@@ -737,9 +720,7 @@
 		wd->sc_wdc_bio.flags = ATA_SINGLE;
 	else
 		wd->sc_wdc_bio.flags = 0;
-	if (wd->sc_flags & WDF_LBA48 &&
-	    (wd->sc_wdc_bio.blkno > LBA48_THRESHOLD ||
-	    (wd->sc_quirks & WD_QUIRK_FORCE_LBA48) != 0))
+	if (wd->sc_flags & WDF_LBA48)
 		wd->sc_wdc_bio.flags |= ATA_LBA48;
 	if (wd->sc_flags & WDF_LBA)
 		wd->sc_wdc_bio.flags |= ATA_LBA;
@@ -1599,9 +1580,7 @@
 	wd->sc_bp = NULL;
 	wd->sc_wdc_bio.blkno = blkno;
 	wd->sc_wdc_bio.flags = ATA_POLL;
-	if (wd->sc_flags & WDF_LBA48 &&
-	    (blkno > LBA48_THRESHOLD ||
-    	    (wd->sc_quirks & WD_QUIRK_FORCE_LBA48) != 0))
+	if (wd->sc_flags & WDF_LBA48)
 		wd->sc_wdc_bio.flags |= ATA_LBA48;
 	if (wd->sc_flags & WDF_LBA)
 		wd->sc_wdc_bio.flags |= ATA_LBA;

--huq684BweRXVnRxX--