cvs commit: src/sys/dev/usb usbdi.c

Ian Dowse iedowse at iedowse.com
Mon Dec 11 19:17:58 PST 2006


In message <200612080009.19380.Danovitsch at vitsch.net>, "Daan Vreeken [PA4DAN]" 
writes:
>Hi Ian,
>
>I have done some digging and I think the _PREWRITE calls aren't needed for the
>status registers of the controller. The documentation of the OHCI-controller 
>inside the ARM processor can be found here :
>http://www.atmel.com/dyn/resources/prod_documents/doc1768.pdf (Page 590)
>
>The documentation talks about two memory regions ("Device registers in memory 
>space" and "Shared ram"). The pointers pointing to the chain of transfers 
>reside in the device registers. Correct me if I'm wrong, but I believe the 
>ohci registers are directly mapped into memory on the ARM. (Meaning that a 
>write to a register will go directly into the register instead of into the 
>cache.)
>If this is the case, syncing the data in the "shared ram" once before 
>adjusting the 'next' pointers would be enough.

Sorry for the delay in replying - that makes sense I guess. The
OHCI controller has registers, and then lots of data structures
(endpoint descriptors, transfer descriptors and transfer data) that
would fall into the shared ram category, since they are allocated
from normal host memory and their physical addresses are linked by
a chain starting from the device registers.

>I can help debug the problems we're seeing on the ARM platform though. During 
>the past week I have written a driver for the USB Device Port of the ARM. It 
>now provides a file in /dev/ that can be used with a simple userland 
>application to create a USB device. I've ported the firmware I once wrote for 
>a custom USB embedded device to use this userland interface. With this I can 
>see the actual data that moves over the USB cable to and from the device.
>Because I still wasn't sure what was really going wrong without the 
>SYNC_PREWRITE patch I connected the USB device port of the board to the USB 
>host port. (The ARM board can now be used to debug itself ;-)

Cool - sounds like a very useful tool to have when debugging host
controller driver code!

>So now I'm pretty sure that the problem is indeed with the content of the 
>"SETUP" packets. The intended content isn't making it to the device, while 
>the associated data seems to be correct every single time.

As far as I can tell, the setup packets certainly aren't the only
shared ram structures needing bus_dmamap_sync calls, but maybe many
of the other combinations of missing calls end up being harmless?
For example, if the transfer descriptor doesn't get flushed out to
memory for a while then the transfer just won't happen until it
does eventually get flushed. In the case of data transfers, the
bus_dmamap_sync calls for the data appear to have the side effect
of flushing the setup packet. So it may be just that if the transfer
and endpoint descriptors to get flushed before the setup packet
that the problem is actually visible.

Below is a first attempt at a patch for ohci that adds BUS_DMASYNC_PREWRITE
operations at most points where the driver expects the changes to
be flushed out. As I mentioned before, this is assuming that
BUS_DMASYNC_PREWRITE has "flush" style semantics, so it will not
work if the buffers are bounced/copied by the bus_dma code.

This is against -current from a week or two ago. I've probably
missed some cases, especially in the attach-time hardware initialisation
code, and I've done no testing beyond seeing that it compiles, boots
and detects a OHCI-attached USB keyboard on an amd64 box.

Ian


Index: ohci.c
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/ohci.c,v
retrieving revision 1.167
diff -u -r1.167 ohci.c
--- ohci.c	19 Oct 2006 01:15:58 -0000	1.167
+++ ohci.c	12 Dec 2006 02:42:49 -0000
@@ -438,6 +438,7 @@
 			offs = i * OHCI_SED_SIZE;
 			sed = KERNADDR(&dma, offs);
 			sed->physaddr = DMAADDR(&dma, offs);
+			sed->dmablock = DMABLOCKPTR(&dma);
 			sed->next = sc->sc_freeeds;
 			sc->sc_freeeds = sed;
 		}
@@ -476,6 +477,7 @@
 			offs = i * OHCI_STD_SIZE;
 			std = KERNADDR(&dma, offs);
 			std->physaddr = DMAADDR(&dma, offs);
+			std->dmablock = DMABLOCKPTR(&dma);
 			std->nexttd = sc->sc_freetds;
 			sc->sc_freetds = std;
 		}
@@ -693,6 +695,7 @@
 			offs = i * OHCI_SITD_SIZE;
 			sitd = KERNADDR(&dma, offs);
 			sitd->physaddr = DMAADDR(&dma, offs);
+			sitd->dmablock = DMABLOCKPTR(&dma);
 			sitd->nextitd = sc->sc_freeitds;
 			sc->sc_freeitds = sitd;
 		}
@@ -1733,7 +1736,7 @@
 	usb_device_request_t *req = &xfer->request;
 	usbd_device_handle dev = opipe->pipe.device;
 	ohci_softc_t *sc = (ohci_softc_t *)dev->bus;
-	ohci_soft_td_t *setup, *stat, *next, *tail;
+	ohci_soft_td_t *setup, *stat, *next, *p, *tail;
 	ohci_soft_ed_t *sed;
 	int isread;
 	int len;
@@ -1814,8 +1817,16 @@
 
 	/* Insert ED in schedule */
 	s = splusb();
+	for (p = setup; p != NULL; p = p->nexttd) {
+		bus_dmamap_sync(p->dmablock->tag, p->dmablock->map,
+		    BUS_DMASYNC_PREWRITE);
+	}
+	bus_dmamap_sync(opipe->u.ctl.reqdma.block->tag,
+	    opipe->u.ctl.reqdma.block->map, BUS_DMASYNC_PREWRITE);
 	sed->ed.ed_tailp = htole32(tail->physaddr);
 	opipe->tail.td = tail;
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	OWRITE4(sc, OHCI_COMMAND_STATUS, OHCI_CLF);
 	if (xfer->timeout && !sc->sc_bus.use_polling) {
 		usb_callout(xfer->timeout_handle, MS_TO_TICKS(xfer->timeout),
@@ -1858,8 +1869,12 @@
 	SPLUSBCHECK;
 	sed->next = head->next;
 	sed->ed.ed_nexted = head->ed.ed_nexted;
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	head->next = sed;
 	head->ed.ed_nexted = htole32(sed->physaddr);
+	bus_dmamap_sync(head->dmablock->tag, head->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 }
 
 /*
@@ -1879,6 +1894,8 @@
 		panic("ohci_rem_ed: ED not found");
 	p->next = sed->next;
 	p->ed.ed_nexted = sed->ed.ed_nexted;
+	bus_dmamap_sync(p->dmablock->tag, p->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 }
 
 /*
@@ -2315,6 +2332,8 @@
 	splx(s);
 	DPRINTFN(1,("ohci_abort_xfer: stop ed=%p\n", sed));
 	sed->ed.ed_flags |= htole32(OHCI_ED_SKIP); /* force hardware skip */
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 
 	/*
 	 * Step 2: Wait until we know hardware has finished any possible
@@ -2376,6 +2395,8 @@
 	 * Step 4: Turn on hardware again.
 	 */
 	sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP); /* remove hardware skip */
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 
 	/*
 	 * Step 5: Execute callback.
@@ -3025,11 +3046,17 @@
 	/* Insert ED in schedule */
 	s = splusb();
 	for (tdp = data; tdp != tail; tdp = tdp->nexttd) {
+		bus_dmamap_sync(tdp->dmablock->tag, tdp->dmablock->map,
+		    BUS_DMASYNC_PREWRITE);
 		tdp->xfer = xfer;
 	}
+	bus_dmamap_sync(tail->dmablock->tag, tail->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	sed->ed.ed_tailp = htole32(tail->physaddr);
 	opipe->tail.td = tail;
 	sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP);
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	OWRITE4(sc, OHCI_COMMAND_STATUS, OHCI_BLF);
 	if (xfer->timeout && !sc->sc_bus.use_polling) {
                 usb_callout(xfer->timeout_handle, MS_TO_TICKS(xfer->timeout),
@@ -3117,6 +3144,8 @@
 		return (err);
 
 	sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP);
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 
 	return (USBD_IN_PROGRESS);
 }
@@ -3190,7 +3219,13 @@
 
 	/* Insert ED in schedule */
 	s = splusb();
+	bus_dmamap_sync(data->dmablock->tag, data->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
+	bus_dmamap_sync(tail->dmablock->tag, tail->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	sed->ed.ed_tailp = htole32(tail->physaddr);
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	opipe->tail.td = tail;
 	splx(s);
 
@@ -3350,7 +3385,7 @@
 	ohci_soft_ed_t *sed = opipe->sed;
 	struct iso *iso = &opipe->u.iso;
 	struct usb_dma_mapping *dma = &xfer->dmamap;
-	ohci_soft_itd_t *sitd, *nsitd;
+	ohci_soft_itd_t *sitd, *nsitd, *p;
 	ohci_physaddr_t dataphys, bp0, physend, prevpage;
 	int curlen, i, len, ncur, nframes, npages, seg, segoff;
 	int s;
@@ -3488,9 +3523,15 @@
 #endif
 
 	s = splusb();
+	for (p = opipe->tail.itd; p != NULL; p = p->nextitd) {
+		bus_dmamap_sync(p->dmablock->tag, p->dmablock->map,
+		    BUS_DMASYNC_PREWRITE);
+	}
 	opipe->tail.itd = sitd;
 	sed->ed.ed_flags &= htole32(~OHCI_ED_SKIP);
 	sed->ed.ed_tailp = htole32(sitd->physaddr);
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	splx(s);
 
 #ifdef USB_DEBUG
@@ -3527,6 +3568,8 @@
 	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.*/
+	bus_dmamap_sync(sed->dmablock->tag, sed->dmablock->map,
+	    BUS_DMASYNC_PREWRITE);
 	splx(s);
 
 	return (USBD_IN_PROGRESS);
Index: ohcivar.h
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/ohcivar.h,v
retrieving revision 1.45
diff -u -r1.45 ohcivar.h
--- ohcivar.h	7 Sep 2006 00:06:41 -0000	1.45
+++ ohcivar.h	3 Dec 2006 03:54:30 -0000
@@ -42,6 +42,7 @@
 	ohci_ed_t ed;
 	struct ohci_soft_ed *next;
 	ohci_physaddr_t physaddr;
+	struct usb_dma_block *dmablock;
 } ohci_soft_ed_t;
 #define OHCI_SED_SIZE ((sizeof (struct ohci_soft_ed) + OHCI_ED_ALIGN - 1) / OHCI_ED_ALIGN * OHCI_ED_ALIGN)
 #define OHCI_SED_CHUNK	(PAGE_SIZE / OHCI_SED_SIZE)
@@ -51,6 +52,7 @@
 	struct ohci_soft_td *nexttd; /* mirrors nexttd in TD */
 	struct ohci_soft_td *dnext; /* next in done list */
 	ohci_physaddr_t physaddr;
+	struct usb_dma_block *dmablock;
 	LIST_ENTRY(ohci_soft_td) hnext;
 	usbd_xfer_handle xfer;
 	u_int16_t len;
@@ -66,6 +68,7 @@
 	struct ohci_soft_itd *nextitd; /* mirrors nexttd in ITD */
 	struct ohci_soft_itd *dnext; /* next in done list */
 	ohci_physaddr_t physaddr;
+	struct usb_dma_block *dmablock;
 	LIST_ENTRY(ohci_soft_itd) hnext;
 	usbd_xfer_handle xfer;
 	u_int16_t flags;
Index: usb_mem.h
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/usb_mem.h,v
retrieving revision 1.21
diff -u -r1.21 usb_mem.h
--- usb_mem.h	6 Jan 2005 01:43:29 -0000	1.21
+++ usb_mem.h	3 Dec 2006 03:53:12 -0000
@@ -61,6 +61,7 @@
 #endif
 #define KERNADDR(dma, o) \
 	((void *)((char *)((dma)->block->kaddr) + (dma)->offs + (o)))
+#define DMABLOCKPTR(dma) ((dma)->block)
 
 usbd_status	usb_allocmem(usbd_bus_handle,size_t,size_t, usb_dma_t *);
 void		usb_freemem(usbd_bus_handle, usb_dma_t *);
Index: usbdi.c
===================================================================
RCS file: /dump/FreeBSD-CVS/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.99
diff -u -r1.99 usbdi.c
--- usbdi.c	27 Nov 2006 18:39:02 -0000	1.99
+++ usbdi.c	2 Dec 2006 13:10:16 -0000
@@ -371,26 +371,14 @@
 	}
 	dmap->nsegs = nseg;
 
-	if (nseg > 0) {
-		if (!usbd_xfer_isread(xfer)) {
-			/*
-			 * Copy data if it is not already in the correct
-			 * buffer.
-			 */
-			if (!(xfer->flags & USBD_NO_COPY) &&
-			    xfer->allocbuf != NULL &&
-			    xfer->buffer != xfer->allocbuf)
-				memcpy(xfer->allocbuf, xfer->buffer,
-				    xfer->length);
-			bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
-		} else if (xfer->rqflags & URQ_REQUEST) {
-			/*
-			 * Even if we have no data portion we still need to
-			 * sync the dmamap for the request data in the SETUP
-			 * packet.
-			 */
-			bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
-		}
+	if (nseg > 0 && !usbd_xfer_isread(xfer)) {
+		/*
+		 * Copy data if it is not already in the correct buffer.
+		 */
+		if (!(xfer->flags & USBD_NO_COPY) && xfer->allocbuf != NULL &&
+		    xfer->buffer != xfer->allocbuf)
+			memcpy(xfer->allocbuf, xfer->buffer, xfer->length);
+		bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
 	}
 	err = pipe->methods->transfer(xfer);
 	if (err != USBD_IN_PROGRESS && err) {




More information about the cvs-src mailing list