restarting a UHCI controller after "host controller halted"

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

Hi,
background: I have a home-made USB device which gets attached as a USB modem:
ehci2: handing over low speed device on port 3 to uhci6
uhub9: port 3, device disappeared after reset
umodem0 at uhub8 port 1 configuration 1 interface 0
umodem0: LIP6/SoC USB Thermo, rev 2.00/0.71, addr 2, iclass 2/2
umodem0: data interface 1, has no CM over data, has no break
umodem0: status change notification available
ucom0 at umodem0

Once in a while, when openning /dev/ttyU0 (the application open/close
it several times per minutes), I get:
uhci6: host controller process error
uhci6: host controller halted
When this happens the device's firmware reports a PID error when connected
to a Intel EHCI, but no error when connected to a VIA EHCI. Also, the
UHCI docs refers to a PID error for a possible cause of "host controller
process error". Because of this I think the issue is on the host
side, and more specifically I suspect there's a race between CPU and
controller when updating the descriptor lists (I need to look at this
more closely).

When a "host controller halted" is reported, the controller is dead, and
a reboot is required to get it working again. It's annoying by itself,
even if it's currently triggered by a software error.
With the attached patch, the controller is reset and restarted, and
stay functionnal:
uhci6: host controller process error
uhci6: host controller halted
umodem0: at uhub8 port 1 (addr 2) disconnected
ucom0: detached
umodem0: detached
ehci2: handing over low speed device on port 3 to uhci6
uhub9: port 3, device disappeared after reset
umodem0 at uhub8 port 1 configuration 1 interface 0
umodem0: LIP6/SoC USB Thermo, rev 2.00/0.71, addr 2, iclass 2/2
umodem0: data interface 1, has no CM over data, has no break
umodem0: status change notification available
ucom0 at umodem0

would anyone object if I commit it ? It has been tested on both a Intel
EHCI and add-on VIA EHCI.

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

Index: uhci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uhci.c,v
retrieving revision 1.219
diff -u -p -u -r1.219 uhci.c
--- uhci.c      2 Jun 2008 01:02:21 -0000       1.219
+++ uhci.c      2 Jun 2008 20:49:20 -0000
@@ -1259,20 +1259,38 @@ uhci_intr1(uhci_softc_t *sc)
                       device_xname(sc->sc_dev));
        }
        if (status & UHCI_STS_HCH) {
-               /* no acknowledge needed */
                if (!sc->sc_dying) {
                        printf("%s: host controller halted\n",
                            device_xname(sc->sc_dev));
 #ifdef UHCI_DEBUG
                        uhci_dump_all(sc);
 #endif
+                       ack |= UHCI_STS_HCH;
                }
-               sc->sc_dying = 1;
        }
 
        if (!ack)
-               return (0);     /* nothing to acknowledge */
-       UWRITE2(sc, UHCI_STS, ack); /* acknowledge the ints */
+               return (0); /* nothing to acknowledge */
+       UWRITE2(sc, UHCI_STS, ack & ~UHCI_STS_HCH); /* acknowledge the ints */
+       if (ack & UHCI_STS_HCH) {
+               sc->sc_bus.use_polling++;
+               uhci_run(sc, 0);
+               UWRITE2(sc, UHCI_INTR, 0);
+               uhci_globalreset(sc);
+               uhci_reset(sc);
+               UWRITE2(sc, UHCI_INTR, UHCI_INTR_TOCRCIE | UHCI_INTR_RIE |
+                   UHCI_INTR_IOCE | UHCI_INTR_SPIE);
+               UHCICMD(sc, UHCI_CMD_MAXP); /* Assume 64 byte packets at frame 
end */
+               UWRITE4(sc, UHCI_FLBASEADDR, DMAADDR(&sc->sc_dma, 0));
+               uhci_run(sc, 1);
+               sc->sc_bus.use_polling--;
+               if (UREAD2(sc, UHCI_STS) & UHCI_STS_HCH) {
+                       printf("%s: host controller couldn't be restarted\n",
+                           device_xname(sc->sc_dev));
+                       sc->sc_dying = 1;
+                       return (0);
+               }
+       }
 
        sc->sc_bus.no_intrs++;
        usb_schedsoftintr(&sc->sc_bus);