On 02/05/13(Thu) 09:41, Martin Pieuchot wrote:
> On 01/05/13(Wed) 17:44, Ted Unangst wrote:
> > On Sun, Apr 28, 2013 at 15:44, Martin Pieuchot wrote:
> > > Diff below is a rework of the suspend/resume logic in ehci(4).
> > > 
> > 
> > > 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.
> > 
> > Got around to testing this. Now everything works. It still prints
> > echi_idone about 100 times after resume, but it doesn't print it
> > forever.
> > 
> > I'd say the diff works, but only with the reset in the resume case as
> > well.
> 
> Ok so, here's an updated diff with the reset enable in the resume case.

Updated diff after recent ehci(4) changes.

I'm still looking for non-regression tests.

Index: dev/usb/ehci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.133
diff -u -p -r1.133 ehci.c
--- dev/usb/ehci.c      30 May 2013 16:15:02 -0000      1.133
+++ dev/usb/ehci.c      3 Jun 2013 13:49:17 -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))) {
@@ -1033,6 +1021,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 */
@@ -1045,7 +1035,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) {
@@ -1056,95 +1046,74 @@ ehci_activate(struct device *self, int a
                rv = config_activate_children(self, act);
                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:
                rv = config_activate_children(self, act);
-               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);
+               ehci_reset(sc);
+
+               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);
 
@@ -1160,17 +1129,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: 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
--- dev/usb/ehcivar.h   15 Apr 2013 09:23:01 -0000      1.25
+++ dev/usb/ehcivar.h   3 Jun 2013 13:49:17 -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: arch/beagle/dev/omehci.c
===================================================================
RCS file: /cvs/src/sys/arch/beagle/dev/omehci.c,v
retrieving revision 1.13
diff -u -p -r1.13 omehci.c
--- arch/beagle/dev/omehci.c    14 May 2013 12:01:17 -0000      1.13
+++ arch/beagle/dev/omehci.c    3 Jun 2013 13:49:17 -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