cvs commit: src/sys/dev/usb usbdi.c
marius at alchemy.franken.de
Mon Nov 27 21:32:38 PST 2006
On Mon, Nov 27, 2006 at 10:32:12PM +0000, Ian Dowse wrote:
> In message <200611271839.kARId33k039747 at repoman.freebsd.org>, Marius Strobl wri
> > Refine the previous change to only call bus_dmamap_sync() in case of
> > an URQ_REQUEST when DMA segments are passed to usbd_start_transfer();
> > when the request doesn't include the optional data buffer the size of
> > the transfer (xfer->length) is 0, in which case usbd_transfer() won't
> > create a DMA map but call usbd_start_transfer() with no DMA segments.
> > With the previous change this could result in the bus_dmamap_sync()
> > implementation dereferencing the NULL-pointer passed as the DMA map
> > argument.
> > While at it fix what appears to be a typo in usbd_start_transfer();
> > in order to determine wheter usbd_start_transfer() was called with
> > DMA segments check whether the number of segments is > 0 rather than
> > the pointer to them being > 0.
> Thanks for spotting the typo - note though that the recently added
> bus_dmamap_sync() call appears to be using the wrong bus_dma tag
> and a potentially uninitialised map, so it is likely to only work
> on architectures where bus_dmamap_sync doesn't depend on the tag
> and map.
> The only bus_dmamap_sync() calls in the usb tree at the moment are
> ones I added as part of the scatter-gather work a while ago, and
> they all relate to the data buffer associated with a transfer. For
> the control transfer SETUP packet buffer, each host controller driver
> has a "reqdma" field that holds the DMA mapping information. It's
> probably easiest to make the changes in the individual host controller
> drivers so they do something like
> reqdma.block->map, BUS_DMASYNC_PREWRITE);
> after filling out the setup packet.
> I guess other memory structures such as descriptors and queue heads
> might need bus_dmamap_sync calls too - what are the features of the
> platform(s) where the original issues were seen? (e.g. is some IOMMU
> operation or memory barrier required between host and I/O access
> to memory?)
Your suggestion sounds reasonable to me, but please talk to
imp@ about it as I was merely trying to fix fallout seen on
sparc64 which was caused by his change but don't know about
the original problam that motivated that change.
> Apart from the handling of data buffers, the USB code
> appears to currently assume that with BUS_DMA_COHERENT it doesn't
> need any further synchronisation, which can't be right in general.
Hrm, because a certain platform might silently ignore
BUS_DMA_COHERENT and provide a non-coherent mapping instead
or because of another reason?
More information about the cvs-src