Diff below is a rework of the suspend/resume logic in ehci(4).

If you're not interested in the full story below, please make sure
it doesn't introduce any regression on your machine(s) and, in any
case report to me (with your dmesg!) thanks :)

Full story:

  So this diff changes the way we supsend/resume ehci(4) in various
  way.  It started as a simple rewrite of the ehci_reset() function
  to fix a race while working on the rework of the suspend/resume
  framework with deraadt@, then later on I discovered that some of
  the magic in there was wrong or no longer needed.
  
  First of all, it removes the logic introduced in r1.46 that places
  all the ports into suspend mode because it no longer makes sense
  as we *do* detach/reattach USB devices during a suspend/resume 
  cycle now.
  
  Then it does a proper reset of the host while suspending instead
  of halting it and keeping the value of USBCMD.  Now that we are
  simply shutting down any USB device during suspend I see no real
  advantage in keeping a complex suspend/resume logic.  Just stop
  the host as if it was a shutdown and set it up again when resuming.
  
  To be coherent, it also modifies the resume path to match what's
  done in ehci_init(), fixing some small differences (and potential
  bugs!).  I could have merged the two functions, but I don't want to
  have a too big diff for now.

I'd really like to know if this introduces any regression.  I'm not
sure it will fix any of the ehci(4) suspend problems I'm aware of
'cause I don't have such hardware, but I hope it will help :)

In case this diff doesn't help or if you have a problem when resuming,
I left an "#ifdef 0" block in the DVACT_RESUME. Try enabling it and tell
me if it changes something.

M.


Index: sys/dev/usb/ehci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.131
diff -u -p -r1.131 ehci.c
--- sys/dev/usb/ehci.c  19 Apr 2013 08:58:53 -0000      1.131
+++ sys/dev/usb/ehci.c  28 Apr 2013 12:32:43 -0000
@@ -353,22 +353,10 @@ ehci_init(struct ehci_softc *sc)
 
        sc->sc_bus.usbrev = USBREV_2_0;
 
-       /* Reset the controller */
        DPRINTF(("%s: resetting\n", sc->sc_bus.bdev.dv_xname));
-       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",
-                   sc->sc_bus.bdev.dv_xname);
-               return (USBD_IOERROR);
-       }
+       err = ehci_reset(sc);
+       if (err)
+               return (err);
 
        /* frame list size at default, read back what we got and use that */
        switch (EHCI_CMD_FLS(EOREAD4(sc, EHCI_USBCMD))) {
@@ -1032,6 +1020,8 @@ ehci_detach(struct ehci_softc *sc, int f
 
        timeout_del(&sc->sc_tmo_intrlist);
 
+       ehci_reset(sc);
+
        usb_delay_ms(&sc->sc_bus, 300); /* XXX let stray task complete */
 
        /* XXX free other data structures XXX */
@@ -1044,7 +1034,7 @@ int
 ehci_activate(struct device *self, int act)
 {
        struct ehci_softc *sc = (struct ehci_softc *)self;
-       u_int32_t cmd, hcr;
+       u_int32_t cmd, hcr, cparams;
        int i, rv = 0;
 
        switch (act) {
@@ -1054,94 +1044,75 @@ ehci_activate(struct device *self, int a
        case DVACT_SUSPEND:
                sc->sc_bus.use_polling++;
 
-               for (i = 1; i <= sc->sc_noport; i++) {
-                       cmd = EOREAD4(sc, EHCI_PORTSC(i));
-                       if ((cmd & (EHCI_PS_PO|EHCI_PS_PE)) == EHCI_PS_PE)
-                               EOWRITE4(sc, EHCI_PORTSC(i),
-                                   cmd | EHCI_PS_SUSP);
-               }
-
-               sc->sc_cmd = EOREAD4(sc, EHCI_USBCMD);
-               cmd = sc->sc_cmd & ~(EHCI_CMD_ASE | EHCI_CMD_PSE);
+               /*
+                * First tell the host to stop processing Asynchronous
+                * and Periodic schedules.
+                */
+               cmd = EOREAD4(sc, EHCI_USBCMD) & ~(EHCI_CMD_ASE | EHCI_CMD_PSE);
                EOWRITE4(sc, EHCI_USBCMD, cmd);
-
                for (i = 0; i < 100; i++) {
+                       usb_delay_ms(&sc->sc_bus, 1);
                        hcr = EOREAD4(sc, EHCI_USBSTS) &
                            (EHCI_STS_ASS | EHCI_STS_PSS);
                        if (hcr == 0)
                                break;
-
-                       usb_delay_ms(&sc->sc_bus, 1);
                }
                if (hcr != 0)
-                       printf("%s: reset timeout\n",
+                       printf("%s: disable schedules timeout\n",
                            sc->sc_bus.bdev.dv_xname);
 
-               cmd &= ~EHCI_CMD_RS;
-               EOWRITE4(sc, EHCI_USBCMD, cmd);
-
-               for (i = 0; i < 100; i++) {
-                       hcr = EOREAD4(sc, EHCI_USBSTS) & EHCI_STS_HCH;
-                       if (hcr == EHCI_STS_HCH)
-                               break;
-
-                       usb_delay_ms(&sc->sc_bus, 1);
-               }
-               if (hcr != EHCI_STS_HCH)
-                       printf("%s: config timeout\n",
-                           sc->sc_bus.bdev.dv_xname);
+               /*
+                * Then reset the host as if it was a shutdown.
+                *
+                * All USB devices are disconnected/reconnected during
+                * a suspend/resume cycle so keep it simple.
+                */
+               ehci_reset(sc);
 
                sc->sc_bus.use_polling--;
                break;
        case DVACT_POWERDOWN:
-               ehci_shutdown(sc);
+               ehci_reset(sc);
                break;
        case DVACT_RESUME:
                sc->sc_bus.use_polling++;
 
-               /* restore things in case the bios sucks */
-               EOWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
+#if 0
+               ehci_reset(sc);
+#endif
+
+               cparams = EREAD4(sc, EHCI_HCCPARAMS);
+               /* MUST clear segment register if 64 bit capable. */
+               if (EHCI_HCC_64BIT(cparams))
+                       EWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
+
                EOWRITE4(sc, EHCI_PERIODICLISTBASE, DMAADDR(&sc->sc_fldma, 0));
                EOWRITE4(sc, EHCI_ASYNCLISTADDR,
                    sc->sc_async_head->physaddr | EHCI_LINK_QH);
-               EOWRITE4(sc, EHCI_USBINTR, sc->sc_eintrs);
 
-               hcr = 0;
-               for (i = 1; i <= sc->sc_noport; i++) {
-                       cmd = EOREAD4(sc, EHCI_PORTSC(i));
-                       if ((cmd & (EHCI_PS_PO|EHCI_PS_SUSP)) == EHCI_PS_SUSP) {
-                               EOWRITE4(sc, EHCI_PORTSC(i),
-                                   cmd | EHCI_PS_FPR);
-                               hcr = 1;
-                       }
-               }
-
-               if (hcr) {
-                       usb_delay_ms(&sc->sc_bus, USB_RESUME_WAIT);
-                       for (i = 1; i <= sc->sc_noport; i++) {
-                               cmd = EOREAD4(sc, EHCI_PORTSC(i));
-                               if ((cmd & (EHCI_PS_PO|EHCI_PS_SUSP)) ==
-                                   EHCI_PS_SUSP)
-                                       EOWRITE4(sc, EHCI_PORTSC(i),
-                                           cmd & ~EHCI_PS_FPR);
-                       }
-               }
-
-               EOWRITE4(sc, EHCI_USBCMD, sc->sc_cmd);
+               /* Turn on controller */
+               EOWRITE4(sc, EHCI_USBCMD,
+                   EHCI_CMD_ITC_2 | /* 2 microframes interrupt delay */
+                   (EOREAD4(sc, EHCI_USBCMD) & EHCI_CMD_FLS_M) |
+                   EHCI_CMD_ASE |
+                   EHCI_CMD_PSE |
+                   EHCI_CMD_RS);
 
                /* Take over port ownership */
                EOWRITE4(sc, EHCI_CONFIGFLAG, EHCI_CONF_CF);
-
                for (i = 0; i < 100; i++) {
+                       usb_delay_ms(&sc->sc_bus, 1);
                        hcr = EOREAD4(sc, EHCI_USBSTS) & EHCI_STS_HCH;
-                       if (hcr != EHCI_STS_HCH)
+                       if (!hcr)
                                break;
+               }
 
-                       usb_delay_ms(&sc->sc_bus, 1);
+               if (hcr) {
+                       printf("%s: run timeout\n", sc->sc_bus.bdev.dv_xname);
+                       /* XXX should we bail here? */
                }
-               if (hcr == EHCI_STS_HCH)
-                       printf("%s: config timeout\n",
-                           sc->sc_bus.bdev.dv_xname);
+
+               EOWRITE4(sc, EHCI_USBINTR, sc->sc_eintrs);
 
                usb_delay_ms(&sc->sc_bus, USB_RESUME_WAIT);
 
@@ -1157,17 +1128,38 @@ ehci_activate(struct device *self, int a
        return (rv);
 }
 
-/*
- * Shut down the controller when the system is going down.
- */
-void
-ehci_shutdown(void *v)
+usbd_status
+ehci_reset(struct ehci_softc *sc)
 {
-       struct ehci_softc *sc = v;
+       u_int32_t hcr;
+       int i;
 
-       DPRINTF(("ehci_shutdown: stopping the HC\n"));
+       DPRINTF(("%s: stopping the HC\n", __func__));
        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)
+               printf("%s: halt timeout\n", sc->sc_bus.bdev.dv_xname);
+
        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", sc->sc_bus.bdev.dv_xname);
+               return (USBD_IOERROR);
+       }
+
+       return (USBD_NORMAL_COMPLETION);
 }
 
 struct usbd_xfer *
Index: sys/dev/usb/ehcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/ehcivar.h,v
retrieving revision 1.25
diff -u -p -r1.25 ehcivar.h
--- sys/dev/usb/ehcivar.h       15 Apr 2013 09:23:01 -0000      1.25
+++ sys/dev/usb/ehcivar.h       27 Apr 2013 09:53:29 -0000
@@ -125,8 +125,6 @@ struct ehci_softc {
        char sc_vendor[16];             /* vendor string for root hub */
        int sc_id_vendor;               /* vendor ID for root hub */
 
-       u_int32_t sc_cmd;               /* shadow of cmd reg during suspend */
-
        struct usb_dma sc_fldma;
        ehci_link_t *sc_flist;
        u_int sc_flsize;
@@ -180,4 +178,5 @@ usbd_status ehci_init(struct ehci_softc 
 int            ehci_intr(void *);
 int            ehci_detach(struct ehci_softc *, int);
 int            ehci_activate(struct device *, int);
-void           ehci_shutdown(void *);
+usbd_status    ehci_reset(struct ehci_softc *);
+
Index: sys/arch/beagle/dev/omehci.c
===================================================================
RCS file: /cvs/src/sys/arch/beagle/dev/omehci.c,v
retrieving revision 1.12
diff -u -p -r1.12 omehci.c
--- sys/arch/beagle/dev/omehci.c        20 Apr 2013 17:43:53 -0000      1.12
+++ sys/arch/beagle/dev/omehci.c        27 Apr 2013 09:37:09 -0000
@@ -179,7 +179,7 @@ omehci_activate(struct device *self, int
                sc->sc.sc_bus.use_polling--;
                break;
        case DVACT_POWERDOWN:
-               ehci_shutdown(sc);
+               ehci_reset(sc->sc);
                break;
        }
        return 0;

Reply via email to