From owner-freebsd-current@FreeBSD.ORG Sun Aug 5 00:06:35 2007 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DEA4716A473 for ; Sun, 5 Aug 2007 00:06:35 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (adsl-75-1-14-242.dsl.scrm01.sbcglobal.net [75.1.14.242]) by mx1.freebsd.org (Postfix) with ESMTP id 8238513C48A for ; Sun, 5 Aug 2007 00:04:16 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.13.3/8.13.3) with ESMTP id l75049Ej016209 for ; Sat, 4 Aug 2007 17:04:13 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <200708050004.l75049Ej016209@gw.catspoiler.org> Date: Sat, 4 Aug 2007 17:04:09 -0700 (PDT) From: Don Lewis To: current@FreeBSD.org MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii Cc: Subject: CFT - patch to fix ehci hang on shutdown X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Aug 2007 00:06:36 -0000 I've got an AMD 64 X2 machine that uses the recent NVIDIA GeForce 7050 / nForce 630a chipset that hangs on shutdown. It hangs with both UP and SMP kernels, and with both FreeBSD 6.2-STABLE and 7.0-CURRENT. The problem appears to be that a delay is needed between the the call EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET); in ehci_shutdown(), and the call cparams = EREAD4(sc, EHCI_HCCPARAMS); in ehci_pci_givecontroller(). There are three instances of a code sequence that does a controller reset in ehci.c, one of which, in ehci_init(), inserts some delays and periodically polls the controller to look for the completion of the reset. It seems to make sense to encapsulate the latter version of the sequence in a separate function, and then call that function from ehci_reset(), ehci_detach(), and ehci_shutdown(). I implemented this in the attached patch, and it fixes shutdown problem for me on both 7.0-CURRENT and 6.2-STABLE (with some minor tweaks for the latter). I've tested this patch on this system with both i386 and amd64 kernels, and I also tested it on my Pentium-M laptop. The shutdown problems are gone and everything else looks normal, but I don't have any USB 2.0 peripherals to do further testing. I'd appreciate any testing that can be done in the next serveral days before I ask re@ for approval to commit this to -CURRENT. Index: sys/dev/usb/ehci.c =================================================================== RCS file: /home/ncvs/src/sys/dev/usb/ehci.c,v retrieving revision 1.55 diff -u -r1.55 ehci.c --- sys/dev/usb/ehci.c 20 Jun 2007 05:10:52 -0000 1.55 +++ sys/dev/usb/ehci.c 4 Aug 2007 21:05:46 -0000 @@ -311,6 +311,25 @@ ehci_device_isoc_done, }; +static usbd_status +ehci_hcreset(ehci_softc_t *sc) +{ + u_int32_t hcr; + u_int i; + + EOWRITE4(sc, EHCI_USBCMD, 0); /* Halt controller */ + usb_delay_ms(&sc->sc_bus, 1); + EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET); + for (i = 0; i < 100; i++) { + usb_delay_ms(&sc->sc_bus, 1); + hcr = EOREAD4(sc, EHCI_USBCMD) & EHCI_CMD_HCRESET; + if (!hcr) + return (USBD_NORMAL_COMPLETION); + } + printf("%s: reset timeout\n", device_get_nameunit(sc->sc_bus.bdev)); + return (USBD_IOERROR); +} + usbd_status ehci_init(ehci_softc_t *sc) { @@ -365,20 +384,9 @@ /* Reset the controller */ DPRINTF(("%s: resetting\n", device_get_nameunit(sc->sc_bus.bdev))); - EOWRITE4(sc, EHCI_USBCMD, 0); /* Halt controller */ - usb_delay_ms(&sc->sc_bus, 1); - EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET); - for (i = 0; i < 100; i++) { - usb_delay_ms(&sc->sc_bus, 1); - hcr = EOREAD4(sc, EHCI_USBCMD) & EHCI_CMD_HCRESET; - if (!hcr) - break; - } - if (hcr) { - printf("%s: reset timeout\n", - device_get_nameunit(sc->sc_bus.bdev)); - return (USBD_IOERROR); - } + err = ehci_hcreset(sc); + if (err != USBD_NORMAL_COMPLETION) + return (err); /* frame list size at default, read back what we got and use that */ switch (EHCI_CMD_FLS(EOREAD4(sc, EHCI_USBCMD))) { @@ -927,8 +935,7 @@ sc->sc_dying = 1; EOWRITE4(sc, EHCI_USBINTR, sc->sc_eintrs); - EOWRITE4(sc, EHCI_USBCMD, 0); - EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET); + (void) ehci_hcreset(sc); callout_stop(&sc->sc_tmo_intrlist); callout_stop(&sc->sc_tmo_pcd); @@ -1090,8 +1097,7 @@ ehci_softc_t *sc = v; DPRINTF(("ehci_shutdown: stopping the HC\n")); - EOWRITE4(sc, EHCI_USBCMD, 0); /* Halt controller */ - EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET); + (void) ehci_hcreset(sc); } usbd_status