NetBSD Problem Report #41926

From fun@naobsd.org  Sun Aug 23 13:37:20 2009
Return-Path: <fun@naobsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 7870163C2D5
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 23 Aug 2009 13:37:20 +0000 (UTC)
Message-Id: <200908231217.n7NCHZXY015245@mail.naobsd.org>
Date: Sun, 23 Aug 2009 21:17:35 +0900 (JST)
From: fun@naobsd.org
Reply-To: fun@naobsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: [PATCH] probe failed for SATA drives in wdc_sataprobe()
X-Send-Pr-Version: 3.95

>Number:         41926
>Category:       kern
>Synopsis:       [PATCH] probe failed for SATA drives in wdc_sataprobe()
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 23 13:40:00 +0000 2009
>Closed-Date:    Tue Oct 06 13:47:14 +0000 2009
>Last-Modified:  Sun Oct 18 16:40:02 +0000 2009
>Originator:     FUKAUMI Naoki
>Release:        NetBSD 5.0_STABLE
>Organization:
	FUKAUMI Naoki
>Environment:
Architecture: x86_64 / arm
Machine: amd64 / evbarm
>Description:
	I found some SATA drives are not properly probed with some SATA
	controllers. I have two machines, one has
	> viaide1: NVIDIA nForce4 Serial ATA Controller (rev. 0xf3)
	with
	> cd0 at atapibus0 drive 0: <Optiarc DVD RW AD-7200S, , 1.21> cdrom removable
	and another has
	> mvsata0 at gt0 offset 0x80000-0x87fff irq 29: Marvell Serial-ATA Host Controller (SATAHC)
	with
	> wd0 at atabus1 drive 0: <ITGO Powered by www.polotek.cn>
	(Marvell MV88F5182 SoC with 4GB SDHC Card via SD-SATA converter)

	without patch, I get following error
	> viaide1 channel 0: reset failed for drive 0
	and these SATA drives are not attached.

	another SATA drives are properly attached without patch.

>How-To-Repeat:
	see above.

>Fix:
	patch attached.

	I replaced bus_space_{read,write}_2() with *_1(). I think it
	should be OK...

Index: sys/dev/ic/wdc.c
===================================================================
RCS file: /home/fun/cvsroot/NetBSD/src/sys/dev/ic/wdc.c,v
retrieving revision 1.255.4.1
diff -u -p -r1.255.4.1 wdc.c
--- sys/dev/ic/wdc.c	20 Nov 2008 02:45:36 -0000	1.255.4.1
+++ sys/dev/ic/wdc.c	17 Aug 2009 18:16:31 -0000
@@ -209,7 +209,7 @@ void
 wdc_sataprobe(struct ata_channel *chp)
 {
 	struct wdc_regs *wdr = CHAN_TO_WDC_REGS(chp);
-	uint16_t scnt, sn, cl, ch;
+	uint8_t st = 0, sc, sn, cl, ch;
 	int i, s;

 	/* XXX This should be done by other code. */
@@ -222,23 +222,34 @@ wdc_sataprobe(struct ata_channel *chp)
 	switch (sata_reset_interface(chp, wdr->sata_iot, wdr->sata_control,
 	    wdr->sata_status)) {
 	case SStatus_DET_DEV:
-		bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh], 0,
-		    WDSD_IBM);
-		delay(10);	/* 400ns delay */
-		scnt = bus_space_read_2(wdr->cmd_iot,
+		/* wait 5s for BSY to clear */
+		for (i = 0; i < WDC_PROBE_WAIT * hz; i++) {
+			bus_space_write_1(wdr->cmd_iot,
+			    wdr->cmd_iohs[wd_sdh], 0, WDSD_IBM);
+			delay(10);	/* 400ns delay */
+			st = bus_space_read_1(wdr->cmd_iot,
+			    wdr->cmd_iohs[wd_status], 0);
+			if ((st & WDCS_BSY) == 0)
+				break;
+			tsleep(&chp, PRIBIO, "sataprb", 1);
+		}
+		if (i == WDC_PROBE_WAIT * hz)
+			aprint_error_dev(chp->ch_atac->atac_dev,
+			    "BSY never cleared, status 0x%02x\n", st);
+		sc = bus_space_read_1(wdr->cmd_iot,
 		    wdr->cmd_iohs[wd_seccnt], 0);
-		sn = bus_space_read_2(wdr->cmd_iot,
+		sn = bus_space_read_1(wdr->cmd_iot,
 		    wdr->cmd_iohs[wd_sector], 0);
-		cl = bus_space_read_2(wdr->cmd_iot,
+		cl = bus_space_read_1(wdr->cmd_iot,
 		    wdr->cmd_iohs[wd_cyl_lo], 0);
-		ch = bus_space_read_2(wdr->cmd_iot,
+		ch = bus_space_read_1(wdr->cmd_iot,
 		    wdr->cmd_iohs[wd_cyl_hi], 0);
-		ATADEBUG_PRINT(("%s: port %d: scnt=0x%x sn=0x%x "
+		ATADEBUG_PRINT(("%s: port %d: sc=0x%x sn=0x%x "
 		    "cl=0x%x ch=0x%x\n",
 		    device_xname(chp->ch_atac->atac_dev), chp->ch_channel,
-		    scnt, sn, cl, ch), DEBUG_PROBE);
+		    sc, sn, cl, ch), DEBUG_PROBE);
 		/*
-		 * scnt and sn are supposed to be 0x1 for ATAPI, but in some
+		 * sc and sn are supposed to be 0x1 for ATAPI, but in some
 		 * cases we get wrong values here, so ignore it.
 		 */
 		s = splbio();

>Release-Note:

>Audit-Trail:
From: David Huang <khym@azeotrope.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/41926
Date: Fri, 18 Sep 2009 14:28:55 -0500

 Just wanted to mention that this patch makes my Optiarc DVD RW  
 AD-7240S probe properly; without it, it attaches as wd0 <ST506>. See http://mail-index.netbsd.org/current-users/2009/09/17/msg010586.html 
   and http://mail-index.netbsd.org/current-users/2009/09/18/msg010597.html

State-Changed-From-To: open->closed
State-Changed-By: sborrill@NetBSD.org
State-Changed-When: Tue, 06 Oct 2009 13:47:14 +0000
State-Changed-Why:
Committed patch to -current. Pullup to netbsd-5 will be requested.

Thanks for the patch!


From: Stephen Borrill <sborrill@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41926 CVS commit: src/sys/dev/ic
Date: Tue, 6 Oct 2009 13:45:01 +0000

 Module Name:	src
 Committed By:	sborrill
 Date:		Tue Oct  6 13:45:01 UTC 2009

 Modified Files:
 	src/sys/dev/ic: wdc.c

 Log Message:
 Commit patch from PR#41926. Confirmed to work by PR submitter on two
 controllers as well myself and another on viaide.

 Stops errors such as the following when probing SATA drives through
 controllers that offer the legacy pciide interface:
 viaide1 channel 0: reset failed for drive 0

 OK bouyer@


 To generate a diff of this commit:
 cvs rdiff -u -r1.257 -r1.258 src/sys/dev/ic/wdc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/41926 CVS commit: [netbsd-5] src/sys/dev/ic
Date: Sun, 18 Oct 2009 16:39:13 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Sun Oct 18 16:39:13 UTC 2009

 Modified Files:
 	src/sys/dev/ic [netbsd-5]: wdc.c

 Log Message:
 Pull up following revision(s) (requested by sborrill in ticket #1074):
 	sys/dev/ic/wdc.c: revision 1.258
 Commit patch from PR#41926. Confirmed to work by PR submitter on two
 controllers as well myself and another on viaide.
 Stops errors such as the following when probing SATA drives through
 controllers that offer the legacy pciide interface:
 viaide1 channel 0: reset failed for drive 0
 OK bouyer@


 To generate a diff of this commit:
 cvs rdiff -u -r1.255.4.1 -r1.255.4.2 src/sys/dev/ic/wdc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search