From owner-freebsd-bugs@FreeBSD.ORG Thu May 22 12:10:10 2003 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E1DBB37B404 for ; Thu, 22 May 2003 12:10:10 -0700 (PDT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id A009943F75 for ; Thu, 22 May 2003 12:10:08 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.9/8.12.9) with ESMTP id h4MJA7Up097285 for ; Thu, 22 May 2003 12:10:07 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.9/8.12.9/Submit) id h4MJA7Uu097284; Thu, 22 May 2003 12:10:07 -0700 (PDT) Resent-Date: Thu, 22 May 2003 12:10:07 -0700 (PDT) Resent-Message-Id: <200305221910.h4MJA7Uu097284@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, "Bruce R. Montague" Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E3BE937B401; Thu, 22 May 2003 12:07:07 -0700 (PDT) Received: from mail.cruzio.com (mail.cruzio.com [63.249.95.37]) by mx1.FreeBSD.org (Postfix) with ESMTP id 343ED43F3F; Thu, 22 May 2003 12:07:07 -0700 (PDT) (envelope-from brucem@cruzio.com) Received: from cruzio.com (dsl3-63-249-85-132.cruzio.com [63.249.85.132]) by mail.cruzio.com with ESMTP id h4MJDWaX023632; Thu, 22 May 2003 12:13:32 -0700 (PDT) Received: (from brucem@localhost) by cruzio.com (8.12.6/8.12.6/Submit) id h4MK5Wlx094455; Thu, 22 May 2003 13:05:32 -0700 (PDT) Message-Id: <200305222005.h4MK5Wlx094455@cruzio.com> Date: Thu, 22 May 2003 13:05:32 -0700 (PDT) From: "Bruce R. Montague" To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 cc: ticso@bwct.de cc: rizzo@icir.org cc: joe@tao.org.uk cc: n_hibma@FreeBSD.org Subject: kern/52589: [patch] Add isochronous usb OHCI support to ohci.c X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: "Bruce R. Montague" List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 May 2003 19:10:11 -0000 >Number: 52589 >Category: kern >Synopsis: [patch] Add isochronous usb OHCI support to ohci.c >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: update >Submitter-Id: current-users >Arrival-Date: Thu May 22 12:10:07 PDT 2003 >Closed-Date: >Last-Modified: >Originator: Bruce R. Montague >Release: FreeBSD 5.1-BETA i386 >Organization: >Environment: System: FreeBSD geode 5.1-BETA FreeBSD 5.1-BETA #9: Thu May 22 03:14:15 GMT 2003 brucem@geode:/usr/obj/usr/src/sys/GENERIC i386 FreeBSD 5.1 -Current with a Geode GX1 CPU and CS5530 southbridge containing an OHCI compatible usb controller. The "ohci.c" and "ohcivar.h" file do not appear to have been modified in over 2 months. >Description: Isochronous OHCI usb transfers do not currently work with "ohci.c". The initial transfers created on open will operate once, after which the driver will stall. There is at least one critical bug (each iteration of the isochronous transfer, resources just allocated are erroneously immediately deallocated). >How-To-Repeat: Attempt to use a web-camera with an OHCI controller. >Fix: The following patch to: /usr/src/sys/dev/usb/ohci.c /usr/src/sys/dev/usb/ohcivar.h implements support for usb isochronous input transfers on the OHCI usb controller. The patch is with respect to the latest -current HEAD files: ohci.c - rev 1.118 ohcivar.h - rev 1.33 This patch has been tested with a DLINK DSB-C100 web camera. This code has acquired images (at a rate of roughly one per second) for over a week at a time under varying loads on a very small machine; no panics, errors, or obvious memory leaks have been observed. I have not tested it on anything else. This patch has one significant bug-fix with respect to the previous patch I posted to freebsd-hardware. The current ohci isochronous code has no possibility of working for any request that does more than a few isochronous transfers, so this patch is of interest to anyone wanting to work with web cameras using the OHCI usb controller. I will try to MFC this into -stable. A copy of this patch, and the patched, working "ohci.c" and "ohcivar.h" source files, can currently be found at: http://63.249.85.132/ohci_patches.htm --- send-pr.diff.txt begins here --- diff -u old/ohci.c new/ohci.c --- old/ohci.c Thu May 22 08:35:50 2003 +++ new/ohci.c Thu May 22 08:32:39 2003 @@ -255,6 +255,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; @@ -635,6 +636,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 +644,7 @@ sitd->nextitd = sc->sc_freeitds; sc->sc_freeitds = sitd; } + splx(s); } s = splusb(); @@ -974,6 +977,18 @@ 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) { + sitd = xfer->hcpriv; + if (sitd) { + for (; sitd->xfer == xfer; sitd = sitd->nextitd) { + if ( sitd == NULL ) break; + ohci_free_sitd(sc, sitd); + } + } + } #ifdef DIAGNOSTIC if (xfer->busy_free != XFER_BUSY) { @@ -1380,7 +1395,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 +1437,10 @@ xfer->status = USBD_STALLED; else xfer->status = USBD_IOERROR; + + s = splusb(); usb_transfer_complete(xfer); + splx(s); } } @@ -1434,6 +1454,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,27 +1466,37 @@ /* 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 + struct ohci_pipe *opipe = + (struct ohci_pipe *)xfer->pipe; + if (opipe->aborting ) + continue; + 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); } } @@ -2079,6 +2110,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) @@ -3223,8 +3255,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 +3275,24 @@ iso->next)); } + if (xfer->hcpriv) { + sitd = xfer->hcpriv; + for (; sitd->xfer == xfer; sitd = sitd->nextitd) { + if (sitd == NULL) break; + 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 +3324,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 +3352,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 +3361,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", @@ -3340,6 +3393,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 +3408,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 +3422,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 +3444,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,23 +3453,54 @@ return; } #endif - for (; sitd->xfer == xfer; sitd = sitd->nextitd) { + if ( sitd ) { /* Find head sitd in next xfer, only if xfer has ITDs. */ + for (; sitd->xfer == xfer; sitd = sitd->nextitd) { + if ( sitd == NULL ) break; + num_sitds++; #ifdef DIAGNOSTIC DPRINTFN(1,("abort sets done sitd=%p\n", sitd)); sitd->isdone = 1; #endif + } } 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; + if ( tmp_sitd ) { + for (; tmp_sitd->xfer == xfer; tmp_sitd = tmp_sitd->nextitd) { + if (tmp_sitd != NULL) { + if (OHCI_CC_NO_ERROR == OHCI_ITD_GET_CC(le32toh(tmp_sitd->itd.itd_flags)) ) { + if (tmp_sitd->flags & OHCI_ITD_ACTIVE) { + if ( (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 ) { /* Only if there is a `next' sitd in next xfer... */ + sed->ed.ed_headp = htole32(sitd->physaddr); /* unlink this xfer's sitds.*/ + } else + sed->ed.ed_headp = 0; + sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); /* remove hardware skip */ splx(s); @@ -3416,21 +3509,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)); +/* 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... + */ - 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; } usbd_status @@ -3456,11 +3551,20 @@ { 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.*/ } diff -u old/ohcivar.h new/ohcivar.h --- old/ohcivar.h Thu May 22 08:35:50 2003 +++ new/ohcivar.h Thu May 22 08:32:39 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,7 +151,9 @@ 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)) --- send-pr.diff.txt ends here --- >Release-Note: >Audit-Trail: >Unformatted: