Re: Call for testing of ehci(4), ohci(4), slhci(4), uhci(4) data toggle patch

[Yorick Hardy <yhardy%NetBSD.org@localhost>]

On 2010-04-27, Yorick Hardy wrote:
> On 2010-04-27, Yorick Hardy wrote:
> > On 2010-04-27, Matthias Drochner wrote:
> > > 
> > > yorickhardy%gmail.com@localhost said:
> > > > Can you try the patch below?
> > > 
> > > I didn't test yet, but wouldn't it look better to avoid the
> > > array which will be used only sparsely, and use some
> > > linked list or so instead? The per-device bitfield could be
> > > put into something device-specific, perhaps close to
> > > the default control pipe.
> > 
> > Yes, I had thought to do that. I was looking for an easy
> > solution to make sure that all problems are resolved first.
> > 
> > Perhaps the best place is in struct usbd_device?
> > Storing the toggle status here may be misleading
> > since for example uhci and ehci update the toggle
> > after transferring a number of packets, not
> > necessarily for each one, and ohci would only update
> > the toggle status on closing a pipe (assuming the
> > last patch is correct).
> 
> Perhaps something like this?

The OpenBSD solution seems better.
I am currently using the patch below.

-- 
Kind regards,

Yorick Hardy

Index: sys/dev/ic/sl811hs.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/sl811hs.c,v
retrieving revision 1.25
diff -u -r1.25 sl811hs.c
--- sys/dev/ic/sl811hs.c        25 Nov 2009 14:28:50 -0000      1.25
+++ sys/dev/ic/sl811hs.c        14 May 2010 22:56:36 -0000
@@ -1041,7 +1041,7 @@
        DLOG(D_TRACE, "slhci_open(addr=%d,ep=%d,rootaddr=%d)",
                dev->address, ed->bEndpointAddress, t->rootaddr, 0);
 
-       spipe->pflags = 0;
+       spipe->pflags = (pipe->endpoint->savedtoggle) ? PF_TOGGLE : 0;
        spipe->frame = 0;
        spipe->lastframe = 0;
        spipe->xfer = NULL;
@@ -1279,6 +1279,7 @@
            pnames(spipe->ptype), spipe, spipe->xfer, 0);
 
        slhci_lock_call(sc, &slhci_close_pipe, spipe, NULL);
+       pipe->endpoint->savedtoggle = (spipe->pflags & PF_TOGGLE) ? 1 : 0;
 }
 
 void
Index: sys/dev/usb/ehci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ehci.c,v
retrieving revision 1.166
diff -u -r1.166 ehci.c
--- sys/dev/usb/ehci.c  24 Feb 2010 22:38:09 -0000      1.166
+++ sys/dev/usb/ehci.c  14 May 2010 22:57:17 -0000
@@ -1549,7 +1549,7 @@
        if (sc->sc_dying)
                return (USBD_IOERROR);
 
-       epipe->nexttoggle = 0;
+       epipe->nexttoggle = pipe->endpoint->savedtoggle;
 
        if (addr == sc->sc_addr) {
                switch (ed->bEndpointAddress) {
@@ -2823,6 +2823,7 @@
        ehci_rem_qh(sc, sqh, head);
        splx(s);
        ehci_free_sqh(sc, epipe->sqh);
+       pipe->endpoint->savedtoggle = epipe->nexttoggle;
 }
 
 /*
@@ -2846,9 +2847,11 @@
        ehci_soft_qtd_t *sqtd;
        ehci_physaddr_t cur;
        u_int32_t qhstatus;
+       u_int32_t nstatus;
        int s;
        int hit;
        int wake;
+       int fixdt;
 
        DPRINTF(("ehci_abort_xfer: xfer=%p pipe=%p\n", xfer, epipe));
 
@@ -2902,11 +2905,18 @@
            sqh->offs + offsetof(ehci_qh_t, qh_qtd.qtd_status),
            sizeof(sqh->qh.qh_qtd.qtd_status),
            BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
-       for (sqtd = exfer->sqtdstart; ; sqtd = sqtd->nextqtd) {
+       for (sqtd = exfer->sqtdstart, fixdt = 1; ; sqtd = sqtd->nextqtd) {
                usb_syncmem(&sqtd->dma,
                    sqtd->offs + offsetof(ehci_qtd_t, qtd_status),
                    sizeof(sqtd->qtd.qtd_status),
                    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
+               nstatus = le32toh(sqtd->qtd.qtd_status);
+               if (fixdt && (nstatus & EHCI_QTD_ACTIVE)
+                         && ((sqtd != exfer->sqtdend->nextqtd)
+                              || EHCI_QTD_GET_BYTES(nstatus))) {
+                       epipe->nexttoggle = (EHCI_QTD_GET_TOGGLE(nstatus)) ? 1 
: 0;
+                       fixdt = 0;
+               }
                sqtd->qtd.qtd_status |= htole32(EHCI_QTD_HALTED);
                usb_syncmem(&sqtd->dma,
                    sqtd->offs + offsetof(ehci_qtd_t, qtd_status),
Index: sys/dev/usb/ohci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ohci.c,v
retrieving revision 1.206
diff -u -r1.206 ohci.c
--- sys/dev/usb/ohci.c  24 Feb 2010 22:38:09 -0000      1.206
+++ sys/dev/usb/ohci.c  14 May 2010 22:57:22 -0000
@@ -2119,6 +2119,10 @@
                        fmt |
                        OHCI_ED_SET_MAXP(UGETW(ed->wMaxPacketSize)));
                sed->ed.ed_headp = sed->ed.ed_tailp = HTOO32(tdphys);
+               if (pipe->endpoint->savedtoggle) {
+                       sed->ed.ed_headp |= HTOO32(OHCI_TOGGLECARRY);
+                       sed->ed.ed_tailp |= HTOO32(OHCI_TOGGLECARRY);
+               }
                usb_syncmem(&sed->dma, sed->offs, sizeof(sed->ed),
                    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
 
@@ -2206,6 +2210,10 @@
        /* Make sure the host controller is not touching this ED */
        usb_delay_ms(&sc->sc_bus, 1);
        splx(s);
+       if (opipe->sed->ed.ed_headp & HTOO32(OHCI_TOGGLECARRY))
+               pipe->endpoint->savedtoggle = 1;
+       else
+               pipe->endpoint->savedtoggle = 0;
        ohci_free_sed(sc, opipe->sed);
 }
 
@@ -3193,6 +3201,10 @@
        for (j = 0; j < nslots; j++)
                --sc->sc_bws[(pos * nslots + j) % OHCI_NO_INTRS];
 
+       if (opipe->sed->ed.ed_headp & HTOO32(OHCI_TOGGLECARRY))
+               pipe->endpoint->savedtoggle = 1;
+       else
+               pipe->endpoint->savedtoggle = 0;
        ohci_free_std(sc, opipe->tail.td);
        ohci_free_sed(sc, opipe->sed);
 }
Index: sys/dev/usb/uhci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uhci.c,v
retrieving revision 1.232
diff -u -r1.232 uhci.c
--- sys/dev/usb/uhci.c  24 Feb 2010 22:38:09 -0000      1.232
+++ sys/dev/usb/uhci.c  14 May 2010 22:57:27 -0000
@@ -2075,8 +2075,10 @@
        struct uhci_pipe *upipe = (struct uhci_pipe *)xfer->pipe;
        uhci_softc_t *sc = upipe->pipe.device->bus->hci_private;
        uhci_soft_td_t *std;
+       u_int32_t dt;
        int s;
        int wake;
+       int fixdt;
 
        DPRINTFN(1,("uhci_abort_xfer: xfer=%p, status=%d\n", xfer, status));
 
@@ -2120,11 +2122,16 @@
        xfer->status = status;  /* make software ignore it */
        usb_uncallout(xfer->timeout_handle, uhci_timeout, ii);
        DPRINTFN(1,("uhci_abort_xfer: stop ii=%p\n", ii));
-       for (std = ii->stdstart; std != NULL; std = std->link.std) {
+       for (std = ii->stdstart, fixdt = 1; std != NULL; std = std->link.std) {
                usb_syncmem(&std->dma,
                    std->offs + offsetof(uhci_td_t, td_status),
                    sizeof(std->td.td_status),
                    BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD);
+               if (fixdt && (le32toh(std->td.td_status) & UHCI_TD_ACTIVE)) {
+                       dt = UHCI_TD_GET_DT(le32toh(std->td.td_token));
+                       upipe->nexttoggle = (dt) ? 1 : 0;
+                       fixdt = 0;
+               }
                std->td.td_status &= htole32(~(UHCI_TD_ACTIVE | UHCI_TD_IOC));
                usb_syncmem(&std->dma,
                    std->offs + offsetof(uhci_td_t, td_status),
@@ -2175,6 +2182,7 @@
        uhci_softc_t *sc = dev->bus->hci_private;
 
        uhci_free_sqh(sc, upipe->u.bulk.sqh);
+       pipe->endpoint->savedtoggle = upipe->nexttoggle;
 }
 
 usbd_status
@@ -2372,6 +2380,7 @@
                uhci_free_sqh(sc, upipe->u.intr.qhs[i]);
        free(upipe->u.intr.qhs, M_USBHC);
 
+       pipe->endpoint->savedtoggle = upipe->nexttoggle;
        /* XXX free other resources */
 }
 
@@ -2784,6 +2793,7 @@
        }
        splx(s);
 
+       pipe->endpoint->savedtoggle = upipe->nexttoggle;
        free(iso->stds, M_USBHC);
 }
 
@@ -3183,7 +3193,7 @@
                     ed->bEndpointAddress, sc->sc_addr));
 
        upipe->aborting = 0;
-       upipe->nexttoggle = 0;
+       upipe->nexttoggle = pipe->endpoint->savedtoggle;
 
        if (pipe->device->address == sc->sc_addr) {
                switch (ed->bEndpointAddress) {
Index: sys/dev/usb/usb_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.167
diff -u -r1.167 usb_subr.c
--- sys/dev/usb/usb_subr.c      12 Nov 2009 20:11:35 -0000      1.167
+++ sys/dev/usb/usb_subr.c      14 May 2010 22:57:31 -0000
@@ -486,6 +486,7 @@
                        }
                }
                ifc->endpoints[endpt].refcnt = 0;
+               ifc->endpoints[endpt].savedtoggle = 0;
                p += ed->bLength;
        }
 #undef ed
Index: sys/dev/usb/usbdivar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.91
diff -u -r1.91 usbdivar.h
--- sys/dev/usb/usbdivar.h      12 Nov 2009 20:11:35 -0000      1.91
+++ sys/dev/usb/usbdivar.h      14 May 2010 22:57:59 -0000
@@ -46,6 +46,7 @@
 struct usbd_endpoint {
        usb_endpoint_descriptor_t *edesc;
        int                     refcnt;
+       int                     savedtoggle;
 };
 
 struct usbd_bus_methods {