Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys The loongon2f+cs5526+jmicron PATA->SATA bridge cause an ...



details:   https://anonhg.NetBSD.org/src/rev/c656258deb59
branches:  trunk
changeset: 768795:c656258deb59
user:      bouyer <bouyer%NetBSD.org@localhost>
date:      Sat Aug 27 17:05:57 2011 +0000

description:
The loongon2f+cs5526+jmicron PATA->SATA bridge cause an interresting issue:
1) because the CS5536 is not associated with a x86 CPU, interrupts are not
  ack'ed as it expects so interrupts cannot configured as edge-triggered
  (as is expected for a PCIIDE in compat mode)
2) the PATA->SATA bridge ignores the WDC_IDS (interrupt disable bit) so
  the PATA IRQ line gets asserted when resetting or running some polled
  commands. It also wrongly asserts IRQ when the (nonexistent) slave
  device is selected
2) wouldn't be an issue with edge-triggered interrupt because we would
   get a spurious interrupt and continue operation, a new interrupt only shows
   up when the PATA IRQ line goes low and high again. But because of 1),
   we get an unclearable interrupt instead, and the system loops on the
   interrupt handler.

To workaround this, introduce a WDC_NO_IDS compile option which runs
all polled commands (including reset) at splbio() and without sleeps,
so that the controller's interrupt is effectively disabled and
won't be reenabled before the interrupt can be cleared.

The conditions triggering this problem are speficic enough to handle
this via a compile-time option; no need for a run-time (e.g. a
config(9), device property or callback to disable interrupts) solution.

diffstat:

 sys/arch/evbmips/conf/LOONGSON |   5 +-
 sys/conf/files                 |   3 +-
 sys/dev/ata/ata_wdc.c          |  10 ++++-
 sys/dev/ic/wdc.c               |  70 +++++++++++++++++++++++++++++++----------
 4 files changed, 65 insertions(+), 23 deletions(-)

diffs (251 lines):

diff -r a071d2b43630 -r c656258deb59 sys/arch/evbmips/conf/LOONGSON
--- a/sys/arch/evbmips/conf/LOONGSON    Sat Aug 27 16:54:14 2011 +0000
+++ b/sys/arch/evbmips/conf/LOONGSON    Sat Aug 27 17:05:57 2011 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: LOONGSON,v 1.1 2011/08/27 13:42:44 bouyer Exp $
+# $NetBSD: LOONGSON,v 1.2 2011/08/27 17:05:57 bouyer Exp $
 #
 # LOONGSON machine description file
 # 
@@ -22,7 +22,7 @@
 
 options        INCLUDE_CONFIG_FILE     # embed config file in kernel binary
 
-#ident                 "GDIUM-$Revision: 1.1 $"
+#ident                 "GDIUM-$Revision: 1.2 $"
 
 maxusers       16
 
@@ -179,6 +179,7 @@
 
 pciide*        at pci? dev ? function ? flags 0x0000   # GENERIC pciide driver
 viaide*        at pci? dev ? function ?        # VIA/AMD/Nvidia IDE controllers
+options        WDC_NO_IDS #workaround CS5536+JMH330 interrupt disable bug
 
 # ATA (IDE) bus support
 atabus* at ata?
diff -r a071d2b43630 -r c656258deb59 sys/conf/files
--- a/sys/conf/files    Sat Aug 27 16:54:14 2011 +0000
+++ b/sys/conf/files    Sat Aug 27 17:05:57 2011 +0000
@@ -1,4 +1,4 @@
-#      $NetBSD: files,v 1.1025 2011/08/26 19:07:13 jmcneill Exp $
+#      $NetBSD: files,v 1.1026 2011/08/27 17:05:57 bouyer Exp $
 #      @(#)files.newconf       7.5 (Berkeley) 5/10/93
 
 version        20100430
@@ -946,6 +946,7 @@
 device wdc: ata, wdc_common
 
 defflag        opt_ata.h       ATADEBUG
+defflag        opt_wdc.h       WDC_NO_IDS
 
 device atabus: atapi,ata_hl
 attach atabus at ata
diff -r a071d2b43630 -r c656258deb59 sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c     Sat Aug 27 16:54:14 2011 +0000
+++ b/sys/dev/ata/ata_wdc.c     Sat Aug 27 17:05:57 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_wdc.c,v 1.93 2010/03/28 20:46:18 snj Exp $ */
+/*     $NetBSD: ata_wdc.c,v 1.94 2011/08/27 17:05:57 bouyer Exp $      */
 
 /*
  * Copyright (c) 1998, 2001, 2003 Manuel Bouyer.
@@ -54,9 +54,10 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.93 2010/03/28 20:46:18 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.94 2011/08/27 17:05:57 bouyer Exp $");
 
 #include "opt_ata.h"
+#include "opt_wdc.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -180,6 +181,11 @@
        struct ata_drive_datas *drvp = &chp->ch_drive[xfer->c_drive];
        int wait_flags = (xfer->c_flags & C_POLL) ? AT_POLL : 0;
        const char *errstring;
+#ifdef WDC_NO_IDS
+       wait_flags = AT_POLL;
+#else
+#error "NEED WDC_NO_IDS"
+#endif
 
        ATADEBUG_PRINT(("wdc_ata_bio_start %s:%d:%d\n",
            device_xname(atac->atac_dev), chp->ch_channel, xfer->c_drive),
diff -r a071d2b43630 -r c656258deb59 sys/dev/ic/wdc.c
--- a/sys/dev/ic/wdc.c  Sat Aug 27 16:54:14 2011 +0000
+++ b/sys/dev/ic/wdc.c  Sat Aug 27 17:05:57 2011 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: wdc.c,v 1.262 2011/08/13 16:02:48 jakllsch Exp $ */
+/*     $NetBSD: wdc.c,v 1.263 2011/08/27 17:05:58 bouyer Exp $ */
 
 /*
  * Copyright (c) 1998, 2001, 2003 Manuel Bouyer.  All rights reserved.
@@ -58,9 +58,10 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.262 2011/08/13 16:02:48 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wdc.c,v 1.263 2011/08/27 17:05:58 bouyer Exp $");
 
 #include "opt_ata.h"
+#include "opt_wdc.h"
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -298,8 +299,22 @@
                return;
        }
 
+       s = splbio();
        /* for ATA/OLD drives, wait for DRDY, 3s timeout */
        for (i = 0; i < mstohz(3000); i++) {
+               /*
+                * select drive 1 first, so that master is selected on
+                * exit from the loop
+                */
+               if (chp->ch_drive[1].drive_flags & (DRIVE_ATA|DRIVE_OLD)) {
+                       if (wdc->select)
+                               wdc->select(chp,1);
+                       bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh],
+                           0, WDSD_IBM | 0x10);
+                       delay(10);      /* 400ns delay */
+                       st1 = bus_space_read_1(wdr->cmd_iot,
+                           wdr->cmd_iohs[wd_status], 0);
+               }
                if (chp->ch_drive[0].drive_flags & (DRIVE_ATA|DRIVE_OLD)) {
                        if (wdc->select)
                                wdc->select(chp,0);
@@ -310,15 +325,6 @@
                            wdr->cmd_iohs[wd_status], 0);
                }
 
-               if (chp->ch_drive[1].drive_flags & (DRIVE_ATA|DRIVE_OLD)) {
-                       if (wdc->select)
-                               wdc->select(chp,1);
-                       bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh],
-                           0, WDSD_IBM | 0x10);
-                       delay(10);      /* 400ns delay */
-                       st1 = bus_space_read_1(wdr->cmd_iot,
-                           wdr->cmd_iohs[wd_status], 0);
-               }
 
                if (((chp->ch_drive[0].drive_flags & (DRIVE_ATA|DRIVE_OLD))
                        == 0 ||
@@ -327,9 +333,16 @@
                        == 0 ||
                    (st1 & WDCS_DRDY)))
                        break;
+#ifdef WDC_NO_IDS
+               /* cannot tsleep here (can't enable IPL_BIO interrups),
+                * delay instead
+                */
+               delay(1000000 / hz);
+#else
+#error "NEED WDC_NO_IDS"
                tsleep(&params, PRIBIO, "atadrdy", 1);
+#endif
        }
-       s = splbio();
        if ((st0 & WDCS_DRDY) == 0)
                chp->ch_drive[0].drive_flags &= ~(DRIVE_ATA|DRIVE_OLD);
        if ((st1 & WDCS_DRDY) == 0)
@@ -689,16 +702,22 @@
        DELAY(2000);
        (void) bus_space_read_1(wdr->cmd_iot, wdr->cmd_iohs[wd_error], 0);
        bus_space_write_1(wdr->ctl_iot, wdr->ctl_ioh, wd_aux_ctlr, WDCTL_4BIT);
+#ifdef WDC_NO_IDS
+       ret_value = __wdcwait_reset(chp, ret_value, RESET_POLL);
+#else
        splx(s);
-
        ret_value = __wdcwait_reset(chp, ret_value, poll);
+       s = splbio();
+#endif
        ATADEBUG_PRINT(("%s:%d: after reset, ret_value=0x%d\n",
            device_xname(chp->ch_atac->atac_dev), chp->ch_channel,
            ret_value), DEBUG_PROBE);
 
        /* if reset failed, there's nothing here */
-       if (ret_value == 0)
+       if (ret_value == 0) {
+               splx(s);
                return 0;
+       }
 
        /*
         * Test presence of drives. First test register signatures looking
@@ -732,7 +751,6 @@
                 * sc & sn are supposted to be 0x1 for ATAPI but in some cases
                 * we get wrong values here, so ignore it.
                 */
-               s = splbio();
                if (cl == 0x14 && ch == 0xeb) {
                        chp->ch_drive[drive].drive_flags |= DRIVE_ATAPI;
                } else {
@@ -740,8 +758,18 @@
                        if ((wdc->cap & WDC_CAPABILITY_PREATA) != 0)
                                chp->ch_drive[drive].drive_flags |= DRIVE_OLD;
                }
-               splx(s);
        }
+       /*
+        * Select an existing drive before lowering spl, some WDC_NO_IDS
+        * devices incorrectly assert IRQ on nonexistent slave
+        */
+       if (ret_value & 0x01) {
+               bus_space_write_1(wdr->cmd_iot, wdr->cmd_iohs[wd_sdh], 0,
+                   WDSD_IBM);
+               (void)bus_space_read_1(wdr->cmd_iot,
+                   wdr->cmd_iohs[wd_status], 0);
+       }
+       splx(s);
        return (ret_value);
 }
 
@@ -908,7 +936,6 @@
 #if NATA_DMA || NATA_PIOBM
        struct wdc_softc *wdc = CHAN_TO_WDC(chp);
 #endif
-
        TAILQ_INIT(&reset_xfer);
 
        chp->ch_flags &= ~ATACH_IRQ_WAIT;
@@ -1006,6 +1033,9 @@
        struct wdc_regs *wdr = &wdc->regs[chp->ch_channel];
        int drv_mask1, drv_mask2;
 
+#ifdef WDC_NO_IDS
+       poll = RESET_POLL;
+#endif
        wdc->reset(chp, poll);
 
        drv_mask1 = (chp->ch_drive[0].drive_flags & DRIVE) ? 0x01:0x00;
@@ -1066,7 +1096,7 @@
        u_int8_t sc0 = 0, sn0 = 0, cl0 = 0, ch0 = 0;
        u_int8_t sc1 = 0, sn1 = 0, cl1 = 0, ch1 = 0;
 #endif
-
+       KASSERT(poll == 1);
        if (poll)
                nloop = WDCNDELAY_RST;
        else
@@ -1475,12 +1505,16 @@
                drive_flags = chp->ch_drive[xfer->c_drive].drive_flags;
        }
 
+#ifdef WDC_NO_IDS
+       wflags = AT_POLL;
+#else
        if ((ata_c->flags & (AT_WAIT | AT_POLL)) == (AT_WAIT | AT_POLL)) {
                /* both wait and poll, we can tsleep here */
                wflags = AT_WAIT | AT_POLL;
        } else {
                wflags = AT_POLL;
        }
+#endif
 
  again:
        ATADEBUG_PRINT(("__wdccommand_intr %s:%d:%d\n",



Home | Main Index | Thread Index | Old Index