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

[Ian Dowse]

In message <200611271839.kARId33k039747 at repoman.freebsd.org>, Marius Strobl wri
tes:
>  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

	bus_dmamap_sync(reqdma.block->tag,
            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?) 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.

Ian