On Mon, 1 Mar 2010, Albert Herranz wrote: > > Also, I can't help thinking that the corresponding *_map() and > > *_unmap() routines are so similar, it ought to be possible to combine > > them. The only difference is a check for a NULL DMA address, and it's > > not clear to me why it is present. It's also not clear why the test > > for a DMA address of all ones is present. Maybe they both can be > > removed. > > > > I think too that I can simplify that logic. > I added those checks in a defensive way seeking robustness while I > familiarize with the USB stack innards. So far, those cases are just > avoiding mappings when > urb_needs_transfer_dma_map()/urb_needs_transfer_dma_unmap() are > called with urb->transfer_buffer == 0 and urb->transfer_dma == 0. > > I guess that those cases are related to scatterlist-based urb requests. > What should be the correct way to check if a urb has already been > scatter/gather-mapped? If urb->num_sgs > 0 then urb has been s-g mapped. Although we don't currently check for it, quite a few URBs have transfer_buffer_length == 0 (a number of control requests are like this, for example) so they don't need a mapping either. > The final logic would be something like: > - map if URB_NO_TRANSFER_DMA_MAP is cleared > - otherwise (URB_TRANSFER_NO_DMA_MAP is set so) map if > HCD_NO_COHERENT_MEM is set _and_ it's not a scatter/gather request > (as that should have been mapped already by usb_buffer_map_sg()) > > Am I on the right path? More or less. I would do it somewhat differently: If URB_NO_TRANSFER_DMA_MAP is set then no map is needed. Otherwise if num_sgs > 0 then no map is needed. Otherwise if HCD_NO_COHERENT_MEM is set then use hcd_alloc_coherent(). Otherwise if transfer_buffer_length > 0 then use dma_map_single(). Similar logic is needed for the setup buffer mapping, but you can assume that control URBs never use scatter-gather so there's no need to check num_sgs (and there's no need to check the transfer length, since setup packets are always 8 bytes long). In fact, I think URB_NO_SETUP_DMA_MAP doesn't really offer any worthwhile advantages. (About the only place where multiple control requests are used in rapid succession is during firmware transfers, and those aren't time-constrained.) It is currently used in a few drivers, but we ought to be able to remove it without too much effort. That might make a good project. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
