Date: Wed, 10 Dec 2003 10:31:21 +0000 (GMT) From: Bruce M Simpson <bms@spc.org> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/60110: [PATCH] Fix isochronous USB pipe handling in -CURRENT Message-ID: <20031210103121.59ED41B@saboteur.dek.spc.org> Resent-Message-ID: <200312101040.hBAAeJkH004895@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 60110 >Category: kern >Synopsis: [PATCH] Fix isochronous USB pipe handling in -CURRENT >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Dec 10 02:40:19 PST 2003 >Closed-Date: >Last-Modified: >Originator: Bruce M Simpson >Release: FreeBSD 5.2-CURRENT i386 >Organization: >Environment: System: FreeBSD saboteur.dek.spc.org 5.2-CURRENT FreeBSD 5.2-CURRENT #0: Mon Dec 8 13:35:27 GMT 2003 bms@kimchi.dek.spc.org:/usr/src/sys/i386/compile/SABOTEUR i386 >Description: Submitted on behalf of Damien Bergamini (ueagle(4) maintainer). Fix USB isochronous pipe handling. These diffs are against 5.1-RELEASE. I have not personally tested these diffs, but have read over them and they appear to be sane. >How-To-Repeat: >Fix: --- ohci-5-1-RELEASE.patch begins here --- --- /sys/dev/usb/ohci.c Sat Oct 4 17:35:57 2003 +++ /sys/dev/usb/ohci.c Wed Oct 8 21:20:19 2003 @@ -1,10 +1,3 @@ -/* $NetBSD: ohci.c,v 1.125 2002/05/28 12:42:38 augustss Exp $ */ -/* $FreeBSD: /repoman/r/ncvs/src/sys/dev/usb/ohci.c,v 1.118 2003/03/05 13:17:15 shiba Exp $ */ - -/* Also, already ported: - * $NetBSD: ohci.c,v 1.127 2002/08/07 20:03:19 augustss Exp $ - */ - /* * Copyright (c) 1998 The NetBSD Foundation, Inc. * All rights reserved. @@ -46,7 +39,7 @@ * USB Open Host Controller driver. * * OHCI spec: http://www.compaq.com/productinfo/development/openhci.html - * USB spec: http://www.usb.org/developers/data/usbspec.zip + * USB spec: http://www.usb.org/developers/docs/usbspec.zip */ #include <sys/param.h> @@ -255,6 +248,7 @@ struct ohci_pipe { struct usbd_pipe pipe; ohci_soft_ed_t *sed; + u_int32_t aborting; union { ohci_soft_td_t *td; ohci_soft_itd_t *itd; @@ -503,7 +497,7 @@ dataphys = DMAADDR(dma, 0); dataphysend = OHCI_PAGE(DMAADDR(dma, len - 1)); - tdflags = htole32( + tdflags = ( (rd ? OHCI_TD_IN : OHCI_TD_OUT) | (flags & USBD_SHORT_XFER_OK ? OHCI_TD_R : 0) | OHCI_TD_NOCC | OHCI_TD_TOGGLE_CARRY); @@ -635,6 +629,7 @@ OHCI_ITD_ALIGN, &dma); if (err) return (NULL); + s = splusb(); for(i = 0; i < OHCI_SITD_CHUNK; i++) { offs = i * OHCI_SITD_SIZE; sitd = KERNADDR(&dma, offs); @@ -642,6 +637,7 @@ sitd->nextitd = sc->sc_freeitds; sc->sc_freeitds = sitd; } + splx(s); } s = splusb(); @@ -669,9 +665,11 @@ #ifdef DIAGNOSTIC if (!sitd->isdone) { - panic("ohci_free_sitd: sitd=%p not done\n", sitd); + panic("ohci_free_sitd: sitd=%p not done", sitd); return; } + /* Warn double free */ + sitd->isdone = 0; #endif s = splusb(); @@ -708,6 +706,8 @@ sc->sc_bus.usbrev = USBREV_1_0; for (i = 0; i < OHCI_HASH_SIZE; i++) + LIST_INIT(&sc->sc_hash_tds[i]); + for (i = 0; i < OHCI_HASH_SIZE; i++) LIST_INIT(&sc->sc_hash_itds[i]); SIMPLEQ_INIT(&sc->sc_free_xfers); @@ -974,6 +974,14 @@ ohci_freex(struct usbd_bus *bus, usbd_xfer_handle xfer) { struct ohci_softc *sc = (struct ohci_softc *)bus; + struct ohci_xfer *oxfer = (struct ohci_xfer *)xfer; + ohci_soft_itd_t *sitd; + + if (oxfer->ohci_xfer_flags & OHCI_ISOC_DIRTY) { + for (sitd = xfer->hcpriv; sitd != NULL && sitd->xfer == xfer; + sitd = sitd->nextitd) + ohci_free_sitd(sc, sitd); + } #ifdef DIAGNOSTIC if (xfer->busy_free != XFER_BUSY) { @@ -1146,7 +1154,7 @@ return (0); } - intrs = 0; + intrs = 0; done = le32toh(sc->sc_hcca->hcca_done_head); /* The LSb of done is used to inform the HC Driver that an interrupt @@ -1253,8 +1261,11 @@ ohci_rhsc_enable(void *v_sc) { ohci_softc_t *sc = v_sc; + int s; + s = splhardusb(); ohci_rhsc_able(sc, 1); + splx(s); } #ifdef USB_DEBUG @@ -1302,7 +1313,7 @@ DPRINTFN(5,("add ITD %p\n", sitd)); continue; } - panic("ohci_add_done: addr 0x%08lx not found\n", (u_long)done); + panic("ohci_add_done: addr 0x%08lx not found", (u_long)done); } /* sdone & sidone now hold the done lists. */ @@ -1322,9 +1333,11 @@ ohci_soft_itd_t *sitd, *sidone, *sitdnext; ohci_soft_td_t *std, *sdone, *stdnext; usbd_xfer_handle xfer; + struct ohci_pipe *opipe; int len, cc, s; + int i, j, iframes; - DPRINTFN(10,("ohci_softintr: enter\n:")); + DPRINTFN(10,("ohci_softintr: enter\n")); sc->sc_bus.intr_context++; @@ -1380,7 +1393,9 @@ xfer->actlen += len; if (std->flags & OHCI_CALL_DONE) { xfer->status = USBD_NORMAL_COMPLETION; + s = splusb(); usb_transfer_complete(xfer); + splx(s); } ohci_free_std(sc, std); } else { @@ -1420,7 +1435,9 @@ xfer->status = USBD_STALLED; else xfer->status = USBD_IOERROR; + s = splusb(); usb_transfer_complete(xfer); + splx(s); } } @@ -1434,6 +1451,7 @@ for (sitd = sidone; sitd != NULL; sitd = sitdnext) { xfer = sitd->xfer; sitdnext = sitd->dnext; + sitd->flags |= OHCI_ITD_INTFIN; DPRINTFN(1, ("ohci_process_done: sitd=%p xfer=%p hcpriv=%p\n", sitd, xfer, xfer ? xfer->hcpriv : 0)); if (xfer == NULL) @@ -1445,28 +1463,68 @@ /* Handled by abort routine. */ continue; } + if (xfer->pipe) + if (xfer->pipe->aborting) + continue; /*Ignore.*/ #ifdef DIAGNOSTIC if (sitd->isdone) printf("ohci_softintr: sitd=%p is done\n", sitd); sitd->isdone = 1; #endif + opipe = (struct ohci_pipe *)xfer->pipe; + if (opipe->aborting) + continue; + + if (sitd->flags & OHCI_CALL_DONE) { + ohci_soft_itd_t *next; + + opipe->u.iso.inuse -= xfer->nframes; + xfer->status = USBD_NORMAL_COMPLETION; + for (i = 0, sitd = xfer->hcpriv;;sitd = next) { + next = sitd->nextitd; + if (OHCI_ITD_GET_CC(sitd->itd.itd_flags) != OHCI_CC_NO_ERROR) + xfer->status = USBD_IOERROR; + + if (xfer->status == USBD_NORMAL_COMPLETION) { + iframes = OHCI_ITD_GET_FC(sitd->itd.itd_flags); + for (j = 0; j < iframes; i++, j++) { + len = le16toh(sitd->itd.itd_offset[j]); + len = + (OHCI_ITD_PSW_GET_CC(len) == + OHCI_CC_NOT_ACCESSED) ? 0 : + OHCI_ITD_PSW_LENGTH(len); + xfer->frlengths[i] = len; + } + } + if (sitd->flags & OHCI_CALL_DONE) + break; + } + + s = splusb(); + usb_transfer_complete(xfer); + splx(s); + } +#ifdef notdef cc = OHCI_ITD_GET_CC(le32toh(sitd->itd.itd_flags)); if (cc == OHCI_CC_NO_ERROR) { /* XXX compute length for input */ - struct ohci_pipe *opipe = - (struct ohci_pipe *)xfer->pipe; if (sitd->flags & OHCI_CALL_DONE) { opipe->u.iso.inuse -= xfer->nframes; /* XXX update frlengths with actual length */ /* XXX xfer->actlen = actlen; */ xfer->status = USBD_NORMAL_COMPLETION; + s = splusb(); usb_transfer_complete(xfer); + splx(s); } } else { /* XXX Do more */ xfer->status = USBD_IOERROR; + s = splusb(); usb_transfer_complete(xfer); + splx(s); } +#endif } #ifdef USB_USE_SOFTINTR @@ -1483,11 +1541,11 @@ void ohci_device_ctrl_done(usbd_xfer_handle xfer) { - DPRINTFN(10,("ohci_ctrl_done: xfer=%p\n", xfer)); + DPRINTFN(10,("ohci_device_ctrl_done: xfer=%p\n", xfer)); #ifdef DIAGNOSTIC if (!(xfer->rqflags & URQ_REQUEST)) { - panic("ohci_ctrl_done: not a request\n"); + panic("ohci_device_ctrl_done: not a request"); } #endif xfer->hcpriv = NULL; @@ -1502,7 +1560,7 @@ ohci_soft_td_t *data, *tail; - DPRINTFN(10,("ohci_intr_done: xfer=%p, actlen=%d\n", + DPRINTFN(10,("ohci_device_intr_done: xfer=%p, actlen=%d\n", xfer, xfer->actlen)); xfer->hcpriv = NULL; @@ -1540,7 +1598,7 @@ void ohci_device_bulk_done(usbd_xfer_handle xfer) { - DPRINTFN(10,("ohci_bulk_done: xfer=%p, actlen=%d\n", + DPRINTFN(10,("ohci_device_bulk_done: xfer=%p, actlen=%d\n", xfer, xfer->actlen)); xfer->hcpriv = NULL; @@ -1820,7 +1878,7 @@ for (p = head; p != NULL && p->next != sed; p = p->next) ; if (p == NULL) - panic("ohci_rem_ed: ED not found\n"); + panic("ohci_rem_ed: ED not found"); p->next = sed->next; p->ed.ed_nexted = sed->ed.ed_nexted; } @@ -2079,6 +2137,7 @@ if (sitd == NULL) goto bad1; opipe->tail.itd = sitd; + opipe->aborting = 0; tdphys = sitd->physaddr; fmt = OHCI_ED_FORMAT_ISO; if (UE_GET_DIR(ed->bEndpointAddress) == UE_DIR_IN) @@ -2220,7 +2279,7 @@ } if (xfer->device->bus->intr_context || !curproc) - panic("ohci_abort_xfer: not in process context\n"); + panic("ohci_abort_xfer: not in process context"); /* * Step 1: Make interrupt routine and hardware ignore xfer. @@ -3013,7 +3072,7 @@ #ifdef DIAGNOSTIC if (xfer->rqflags & URQ_REQUEST) - panic("ohci_device_intr_transfer: a request\n"); + panic("ohci_device_intr_transfer: a request"); #endif len = xfer->length; @@ -3104,7 +3163,7 @@ #ifdef DIAGNOSTIC if ((le32toh(sed->ed.ed_tailp) & OHCI_HEADMASK) != (le32toh(sed->ed.ed_headp) & OHCI_HEADMASK)) - panic("%s: Intr pipe %p still has TDs queued\n", + panic("%s: Intr pipe %p still has TDs queued", USBDEVNAME(sc->sc_bus.bdev), pipe); #endif @@ -3112,7 +3171,7 @@ ; #ifdef DIAGNOSTIC if (p == NULL) - panic("ohci_device_intr_close: ED not found\n"); + panic("ohci_device_intr_close: ED not found"); #endif p->next = sed->next; p->ed.ed_nexted = sed->ed.ed_nexted; @@ -3223,8 +3282,9 @@ ohci_softc_t *sc = (ohci_softc_t *)dev->bus; ohci_soft_ed_t *sed = opipe->sed; struct iso *iso = &opipe->u.iso; + struct ohci_xfer *oxfer = (struct ohci_xfer *)xfer; ohci_soft_itd_t *sitd, *nsitd; - ohci_physaddr_t buf, offs, noffs, bp0; + ohci_physaddr_t buf, offs, noffs, bp0, tdphys; int i, ncur, nframes; int s; @@ -3242,6 +3302,24 @@ iso->next)); } + if (xfer->hcpriv) { + for (sitd = xfer->hcpriv; sitd != NULL && sitd->xfer == xfer; + sitd = sitd->nextitd) + ohci_free_sitd(sc, sitd); /* Free ITDs in prev xfer*/ + + if (sitd == NULL) { + sitd = ohci_alloc_sitd(sc); + if (sitd == NULL) + panic("cant alloc isoc"); + opipe->tail.itd = sitd; + tdphys = sitd->physaddr; + sed->ed.ed_flags |= htole32(OHCI_ED_SKIP); /* Stop*/ + sed->ed.ed_headp = + sed->ed.ed_tailp = htole32(tdphys); + sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); /* Start.*/ + } + } + sitd = opipe->tail.itd; buf = DMAADDR(&xfer->dmabuf, 0); bp0 = OHCI_PAGE(buf); @@ -3273,7 +3351,7 @@ sitd->itd.itd_nextitd = htole32(nsitd->physaddr); sitd->itd.itd_be = htole32(bp0 + offs - 1); sitd->xfer = xfer; - sitd->flags = 0; + sitd->flags = OHCI_ITD_ACTIVE; sitd = nsitd; iso->next = iso->next + ncur; @@ -3301,7 +3379,7 @@ sitd->itd.itd_nextitd = htole32(nsitd->physaddr); sitd->itd.itd_be = htole32(bp0 + offs - 1); sitd->xfer = xfer; - sitd->flags = OHCI_CALL_DONE; + sitd->flags = OHCI_CALL_DONE | OHCI_ITD_ACTIVE; iso->next = iso->next + ncur; iso->inuse += nframes; @@ -3310,6 +3388,8 @@ xfer->status = USBD_IN_PROGRESS; + oxfer->ohci_xfer_flags |= OHCI_ISOC_DIRTY; + #ifdef USB_DEBUG if (ohcidebug > 5) { DPRINTF(("ohci_device_isoc_enter: frame=%d\n", @@ -3321,6 +3401,7 @@ s = splusb(); opipe->tail.itd = nsitd; + sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); sed->ed.ed_tailp = htole32(nsitd->physaddr); splx(s); @@ -3340,6 +3421,8 @@ { struct ohci_pipe *opipe = (struct ohci_pipe *)xfer->pipe; ohci_softc_t *sc = (ohci_softc_t *)opipe->pipe.device->bus; + ohci_soft_ed_t *sed; + int s; DPRINTFN(5,("ohci_device_isoc_start: xfer=%p\n", xfer)); @@ -3353,6 +3436,11 @@ /* XXX anything to do? */ + s = splusb(); + sed = opipe->sed; /* Turn off ED skip-bit to start processing */ + sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); /* ED's ITD list.*/ + splx(s); + return (USBD_IN_PROGRESS); } @@ -3362,10 +3450,11 @@ struct ohci_pipe *opipe = (struct ohci_pipe *)xfer->pipe; ohci_softc_t *sc = (ohci_softc_t *)opipe->pipe.device->bus; ohci_soft_ed_t *sed; - ohci_soft_itd_t *sitd; - int s; + ohci_soft_itd_t *sitd, *tmp_sitd; + int s,undone,num_sitds; s = splusb(); + opipe->aborting = 1; DPRINTFN(1,("ohci_device_isoc_abort: xfer=%p\n", xfer)); @@ -3383,6 +3472,7 @@ sed = opipe->sed; sed->ed.ed_flags |= htole32(OHCI_ED_SKIP); /* force hardware skip */ + num_sitds = 0; sitd = xfer->hcpriv; #ifdef DIAGNOSTIC if (sitd == NULL) { @@ -3391,7 +3481,8 @@ return; } #endif - for (; sitd->xfer == xfer; sitd = sitd->nextitd) { + for (; sitd != NULL && sitd->xfer == xfer; sitd = sitd->nextitd) { + num_sitds++; #ifdef DIAGNOSTIC DPRINTFN(1,("abort sets done sitd=%p\n", sitd)); sitd->isdone = 1; @@ -3400,14 +3491,43 @@ splx(s); - usb_delay_ms(&sc->sc_bus, OHCI_ITD_NOFFSET); + /* + * Each sitd has up to OHCI_ITD_NOFFSET transfers, each can + * take a usb 1ms cycle. Conservatively wait for it to drain. + * Even with DMA done, it can take awhile for the "batch" + * delivery of completion interrupts to occur thru the controller. + */ + + do { + usb_delay_ms(&sc->sc_bus, 2*(num_sitds*OHCI_ITD_NOFFSET)); + + undone = 0; + tmp_sitd = xfer->hcpriv; + for (; tmp_sitd != NULL && tmp_sitd->xfer == xfer; + tmp_sitd = tmp_sitd->nextitd) { + if (OHCI_CC_NO_ERROR == + OHCI_ITD_GET_CC(le32toh(tmp_sitd->itd.itd_flags)) && + tmp_sitd->flags & OHCI_ITD_ACTIVE && + (tmp_sitd->flags & OHCI_ITD_INTFIN) == 0) + undone++; + } + } while( undone != 0 ); + s = splusb(); /* Run callback. */ usb_transfer_complete(xfer); - sed->ed.ed_headp = htole32(sitd->physaddr); /* unlink TDs */ + if (sitd != NULL) + /* + * Only if there is a `next' sitd in next xfer... + * unlink this xfer's sitds. + */ + sed->ed.ed_headp = htole32(sitd->physaddr); + else + sed->ed.ed_headp = 0; + sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); /* remove hardware skip */ splx(s); @@ -3416,21 +3536,23 @@ void ohci_device_isoc_done(usbd_xfer_handle xfer) { - struct ohci_pipe *opipe = (struct ohci_pipe *)xfer->pipe; - ohci_softc_t *sc = (ohci_softc_t *)opipe->pipe.device->bus; - ohci_soft_itd_t *sitd, *nsitd; - - DPRINTFN(1,("ohci_device_isoc_done: xfer=%p\n", xfer)); - - for (sitd = xfer->hcpriv; - !(sitd->flags & OHCI_CALL_DONE); - sitd = nsitd) { - nsitd = sitd->nextitd; - DPRINTFN(1,("ohci_device_isoc_done: free sitd=%p\n", sitd)); - ohci_free_sitd(sc, sitd); - } - ohci_free_sitd(sc, sitd); - xfer->hcpriv = NULL; + /* This null routine corresponds to non-isoc "done()" routines + * that free the stds associated with an xfer after a completed + * xfer interrupt. However, in the case of isoc transfers, the + * sitds associated with the transfer have already been processed + * and reallocated for the next iteration by + * "ohci_device_isoc_transfer()". + * + * Routine "usb_transfer_complete()" is called at the end of every + * relevant usb interrupt. "usb_transfer_complete()" indirectly + * calls 1) "ohci_device_isoc_transfer()" (which keeps pumping the + * pipeline by setting up the next transfer iteration) and 2) then + * calls "ohci_device_isoc_done()". Isoc transfers have not been + * working for the ohci usb because this routine was trashing the + * xfer set up for the next iteration (thus, only the first + * UGEN_NISOREQS xfers outstanding on an open would work). Perhaps + * this could all be re-factored, but that's another pass... + */ } usbd_status @@ -3456,11 +3578,19 @@ { struct ohci_pipe *opipe = (struct ohci_pipe *)pipe; ohci_softc_t *sc = (ohci_softc_t *)pipe->device->bus; + ohci_soft_ed_t *sed; DPRINTF(("ohci_device_isoc_close: pipe=%p\n", pipe)); - ohci_close_pipe(pipe, sc->sc_isoc_head); + + sed = opipe->sed; + sed->ed.ed_flags |= htole32(OHCI_ED_SKIP); /* Stop device. */ + + ohci_close_pipe(pipe, sc->sc_isoc_head); /* Stop isoc list, free ED.*/ + + /* up to NISOREQs xfers still outstanding. */ + #ifdef DIAGNOSTIC opipe->tail.itd->isdone = 1; #endif - ohci_free_sitd(sc, opipe->tail.itd); + ohci_free_sitd(sc, opipe->tail.itd); /* Next `avail free' sitd.*/ } --- ohci-5-1-RELEASE.patch ends here --- --- ohcireg-5-1-RELEASE.patch begins here --- --- /sys/dev/usb/ohcireg.h Sat Oct 4 10:32:06 2003 +++ /sys/dev/usb/ohcireg.h Sat Oct 4 10:32:45 2003 @@ -187,9 +187,11 @@ #define OHCI_TD_GET_DI(x) (((x) >> 21) & 7) /* Delay Interrupt */ #define OHCI_TD_SET_DI(x) ((x) << 21) #define OHCI_TD_NOINTR 0x00e00000 +#define OHCI_TD_INTR_MASK 0x00e00000 #define OHCI_TD_TOGGLE_CARRY 0x00000000 #define OHCI_TD_TOGGLE_0 0x02000000 #define OHCI_TD_TOGGLE_1 0x03000000 +#define OHCI_TD_TOGGLE_MASK 0x03000000 #define OHCI_TD_GET_EC(x) (((x) >> 26) & 3) /* Error Count */ #define OHCI_TD_GET_CC(x) ((x) >> 28) /* Condition Code */ #define OHCI_TD_NOCC 0xf0000000 --- ohcireg-5-1-RELEASE.patch ends here --- --- ohcivar-5-1-RELEASE.patch begins here --- --- /sys/dev/usb/ohcivar.h Sat Oct 4 10:30:06 2003 +++ /sys/dev/usb/ohcivar.h Sat Oct 4 10:31:24 2003 @@ -70,6 +70,8 @@ LIST_ENTRY(ohci_soft_itd) hnext; usbd_xfer_handle xfer; u_int16_t flags; +#define OHCI_ITD_ACTIVE 0x0010 /* Hardware op in progress */ +#define OHCI_ITD_INTFIN 0x0020 /* Hw completion interrupt seen.*/ #ifdef DIAGNOSTIC char isdone; #endif @@ -149,9 +151,11 @@ struct ohci_xfer { struct usbd_xfer xfer; struct usb_task abort_task; + u_int32_t ohci_xfer_flags; }; +#define OHCI_ISOC_DIRTY 0x01 -#define OXFER(xfer) ((struct ehci_xfer *)(xfer)) +#define OXFER(xfer) ((struct ohci_xfer *)(xfer)) usbd_status ohci_init(ohci_softc_t *); int ohci_intr(void *); --- ohcivar-5-1-RELEASE.patch ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031210103121.59ED41B>