kern/52589: [patch] Add isochronous usb OHCI support to ohci.c

Bruce R. Montague brucem at cruzio.com
Thu May 22 12:10:11 PDT 2003


>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 at 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:


More information about the freebsd-bugs mailing list