Reboot on "shutdown -r" hangs after final "uptime ..." string

[Don Lewis]

On 11 Aug, Don Lewis wrote:
> On 11 Aug, To: bruce at cran.org.uk wrote:
>> On 12 Aug, bruce at cran.org.uk wrote:
> 
>> Something that you might want to also print is the value of
>> 	EOREAD4(sc, EHCI_USBSTS)
>> immediately before the
>> 	EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET);
>> call in ehci_hcreset().  Bit 12 of this register (EHCI_STS_HCH) should
>> be a zero before the write to do the reset is performed.
> 
> Correction, this bit should be a one before doing the reset.
> 
>> Maybe
>> ehci_hcreset() needs to wait for that to happen (as is done in
>> ehci_init()), rather than just waiting a fixed amount of time.
> 
> I misread the code in ehci_init(), which is waiting for a transition in
> the opposite direction.  I don't know why, because the EHCI spec says
> that the 1->0 transition is immediate.

A revised version of the patch is below.  The only change is the
addition of a second wait loop in ehci_hcreset(), which would be
necessary according the the EHCI spec if the controller could ever
require more than 1ms to stop.

This patch might fix the problem on your machine if you are observing a
zero for bit 12 (EHCI_STS_HCH) of the EHCI_USBSTS register in
ehci_hcreset().

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	12 Aug 2007 03:55:54 -0000
@@ -311,6 +311,39 @@
 	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 */
+	for (i = 0; i < 100; i++) {
+		usb_delay_ms(&sc->sc_bus, 1);
+		hcr = EOREAD4(sc, EHCI_USBSTS) & EHCI_STS_HCH;
+		if (hcr)
+			break;
+	}
+	if (!hcr)
+		/*
+                 * Fall through and try reset anyway even though
+                 * Table 2-9 in the EHCI spec says this will result
+                 * in undefined behavior.
+                 */
+		printf("%s: stop timeout\n",
+		       device_get_nameunit(sc->sc_bus.bdev));
+
+	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 +398,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 +949,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 +1111,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


If this doesn't help, you could test the SMI hypothesis by disabling the
SMI on OS Ownership Enable bit before clearing the EHCI_LEGSUP_OSOWNED
bit.  Something like the following should disable the SMI enable bit,
but I don't know what side effects it might have.  It could even melt
your computer ;-)  In any case, it's not a proper fix.

static void
ehci_pci_givecontroller(device_t self)
{
        ehci_softc_t *sc = device_get_softc(self);
        u_int32_t cparams, eec, legsup;
        int eecp;

        cparams = EREAD4(sc, EHCI_HCCPARAMS);
        for (eecp = EHCI_HCC_EECP(cparams); eecp != 0;
            eecp = EHCI_EECP_NEXT(eec)) {
                eec = pci_read_config(self, eecp, 4);
                if (EHCI_EECP_ID(eec) != EHCI_EC_LEGSUP)
                        continue;
/* NEW */       legsup = pci_read_config(self,eecp+EHCI_LEGSUP_USBLEGCTLSTS,4);
/* NEW */       legsup = legsup & ~0x200;
/* NEW */       pci_write_config(self, eecp+EHCI_LEGSUP_USBLEGCTLSTS, legsup,4);
                legsup = eec;
                pci_write_config(self, eecp, legsup & ~EHCI_LEGSUP_OSOWNED, 4);
        }
}