Marius Strobl wrote:
On Wed, Apr 23, 2008 at 02:03:27PM -0700, Sam Leffler wrote:
Marius Strobl wrote:
On Wed, Apr 23, 2008 at 01:43:55PM -0700, Sam Leffler wrote:
Marius Strobl wrote:
On Sat, Apr 12, 2008 at 09:33:58PM +0200, Marius Strobl wrote:
On Thu, Mar 20, 2008 at 04:19:26PM +0000, Sam Leffler wrote:
sam 2008-03-20 16:19:25 UTC
FreeBSD src repository
Modified files:
sys/dev/usb ehci.c ohci.c
Log:
Workaround design botch in usb: blindly mixing bus_dma with PIO does
not
work on architectures with a write-back cache as the PIO writes end up
in the cache which the sync(BUS_DMASYNC_POSTREAD) in
usb_transfer_complete
then discards; compensate in the xfer methods that do PIO by pushing
the
writes out of the cache before usb_transfer_complete is called.
This fixes USB on xscale and likely other places.
Sponsored by: hobnob
Reviewed by: cognet, imp
MFC after: 1 month
Revision Changes Path
1.62 +16 -0 src/sys/dev/usb/ehci.c
1.171 +16 -0 src/sys/dev/usb/ohci.c
This causes a crash during boot on sparc64. Looks like map is still
NULL at that point.
Are you ok with the change below or would that also prevent
your kludge from taking effect?
Marius
Index: ehci.c
===================================================================
RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ehci.c,v
retrieving revision 1.62
diff -u -r1.62 ehci.c
--- ehci.c 20 Mar 2008 16:19:25 -0000 1.62
+++ ehci.c 23 Apr 2008 20:23:58 -0000
@@ -664,6 +664,8 @@
usbd_pipe_handle pipe = xfer->pipe;
bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag;
struct usb_dma_mapping *dmap = &xfer->dmamap;
+ if (dmap->map == NULL)
+ return;
bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
}
Index: ohci.c
===================================================================
RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ohci.c,v
retrieving revision 1.171
diff -u -r1.171 ohci.c
--- ohci.c 20 Mar 2008 16:19:25 -0000 1.171
+++ ohci.c 21 Apr 2008 19:13:54 -0000
@@ -1571,6 +1571,8 @@
usbd_pipe_handle pipe = xfer->pipe;
bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag;
struct usb_dma_mapping *dmap = &xfer->dmamap;
+ if (dmap->map == NULL)
+ return;
bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
}
You have not identified why you don't have a dma map. I don't have a
way to diagnose your problem and so far as I know no other platform had
an issue w/ the change. I suggest you figure out why your map is not
setup instead of adding a hack.
It's because the usb(4) code doesn't create DMA maps for
zero-length transfers, see usbd_transfer(). In the case of
the backtrace I posted not for usbd_set_address(), which
does USETW(req.wLength, 0) so later on size is 0 in
usbd_transfer() hence no DMA map. I don't know why your
hack doesn't also crash other platforms.
Thanks for explaining, I will look. Please hold off for a bit.
Style-wise the version below probably is more appropriate than
the above one. The question still is whether that fix prevents
hacksync() taking effect as desired, which would make it a very
evil hack though as hacksync() then relies on bus_dmamap_sync()
working on uninitialized DMA maps.
Marius
Index: ehci.c
===================================================================
RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ehci.c,v
retrieving revision 1.62
diff -u -p -r1.62 ehci.c
--- ehci.c 20 Mar 2008 16:19:25 -0000 1.62
+++ ehci.c 27 Apr 2008 14:09:53 -0000
@@ -661,9 +661,13 @@ ehci_pcd_enable(void *v_sc)
static __inline void
hacksync(usbd_xfer_handle xfer)
{
- usbd_pipe_handle pipe = xfer->pipe;
- bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag;
- struct usb_dma_mapping *dmap = &xfer->dmamap;
+ bus_dma_tag_t tag;
+ struct usb_dma_mapping *dmap;
+
+ if (xfer->length == 0)
+ return;
+ tag = xfer->pipe->device->bus->buffer_dmatag;
+ dmap = &xfer->dmamap;
bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
}
Index: ohci.c
===================================================================
RCS file: /usr/data/bsd/cvs/fbsd/src/sys/dev/usb/ohci.c,v
retrieving revision 1.171
diff -u -p -r1.171 ohci.c
--- ohci.c 20 Mar 2008 16:19:25 -0000 1.171
+++ ohci.c 27 Apr 2008 14:09:37 -0000
@@ -1568,9 +1568,13 @@ ohci_device_bulk_done(usbd_xfer_handle x
static __inline void
hacksync(usbd_xfer_handle xfer)
{
- usbd_pipe_handle pipe = xfer->pipe;
- bus_dma_tag_t tag = pipe->device->bus->buffer_dmatag;
- struct usb_dma_mapping *dmap = &xfer->dmamap;
+ bus_dma_tag_t tag;
+ struct usb_dma_mapping *dmap;
+
+ if (xfer->length == 0)
+ return;
+ tag = xfer->pipe->device->bus->buffer_dmatag;
+ dmap = &xfer->dmamap;
bus_dmamap_sync(tag, dmap->map, BUS_DMASYNC_PREWRITE);
}
Sorry for taking so long to look at this. It appears the reason things
work everywhere but sparc is because all the other archs use the
definition of bus_dmamap_sync which looks like this:
#define bus_dmamap_sync(dmat, dmamap, op) \
do { \
if ((dmamap) != NULL) \
_bus_dmamap_sync(dmat, dmamap, op); \
} while (0)
So it explicitly checks for the map being NULL and since sparc does not
it blows up. Now checking for a NULL map seems very wrong (as the map
should be opaque as scott noted) but having sparc have different
semantics seems wrong. So rather than add yet another hack in the usb
code perhaps we should make sparc's bus_dma code consistent with
everyone else on this?
There's no mention of this in the man page.
Sam
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"
