kern/41926: [PATCH] probe failed for SATA drives in wdc_sataprobe()

[fun%naobsd.org@localhost]

>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:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 23 13:40:00 +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();