Alan Stern wrote: > On Thu, 4 Mar 2010, Albert Herranz wrote: > Reread what I wrote above: If HCD_NO_COHERENT_MEM _isn't_ set! With > the correct test this becomes: > > if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (true) and HCD_NO_COHERENT_MEM isn't set (false) do nothing > transfer gets mapped again and works properly > Sorry, I read you wrong again. I'm a bit thick today :) So the map logic would be like this? If transfer_buffer_length is 0 then do nothing. Otherwise if num_sgs > 0 then do nothing. Otherwise if URB_NO_TRANSFER_DMA_MAP and transfer_dma != ~0 and HCD_NO_COHERENT_MEM is not set (this avoids your HCD_NO_COHERENT_MEM case) then do nothing. Otherwise if hcd->self.uses_dma is set then call dma_map_single Otherwise if HCD_LOCAL_MEM is set then call hcd_alloc_coherent Otherwise do nothing (PIO case). Then we have: (HCD_NO_COHERENT_MEM): usb_buffer_alloc() /* urb->transfer_dma == ~0 */ urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; /* 1st transfer */ usb_submit_urb(urb, GFP_KERNEL); if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (false) and HCD_NO_COHERENT_MEM not set (false) do nothing transfer gets mapped, Ok /* urb->transfer_dma != ~0 */ /* 2nd transfer */ usb_submit_urb(urb, GFP_KERNEL); if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (true) and HCD_NO_COHERENT_MEM not set (false) do nothing transfer gets mapped, Ok (!HCD_NO_COHERENT_MEM): usb_buffer_alloc() /* urb->transfer_dma != ~0 */ urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; /* 1st transfer */ usb_submit_urb(urb, GFP_KERNEL); if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (true) and HCD_NO_COHERENT_MEM not set (true) do nothing transfer doesn't get mapped, Ok /* 2nd transfer */ usb_submit_urb(urb, GFP_KERNEL); if URB_NO_TRANSFER_DMA_MAP is set (true) and transfer_dma != ~0 (true) and HCD_NO_COHERENT_MEM not set (true) do nothing transfer doesn't get mapped, Ok Can we then get rid of the transfer_dma != ~0 check? It seems useless now. >>> You're concerned that transfer_dma might contain a valid DMA address >>> for an existing mapping. How could it? usb_buffer_alloc() won't >>> return a valid address if HCD_NO_COHERENT_MEM is set. And even if it >>> did, you would overwrite that valid address at unmap time anyway. >>> >> transfer_dma gets a valid mapping when it is mapped by dma_map_single() at map time. > > That address is no longer valid after the transfer buffer gets unmapped > by dma_unmap_single() at unmap time. > Yes and no. Yes, if you consider the real map/unmap that happens behind the scenes. No, if you look at what the USB driver thinks it's happening. The USB driver thinks that it got a "coherent" buffer from usb_alloc_buffer() and assumes that urb->transfer_dma is preserved because the USB stack is supposed to not touch that when URB_NO_TRANSFER_DMA_MAP. But, anyway, this shouldn't cause a real problem, I guess. >> In the scenario I propose, if the transfer buffer wasn't mapped then it won't get unmapped (the new flags control that), and thus transfer_dma won't be touched. >> The only case it will be overwritten to ~0 at unmap time is when transfer_dma was ~0 at map time _and_ it was actually mapped (HCD_NO_COHERENT_MEM case). > > It's better to have all the tricky computations in one place: map time. > Don't split them up into two places. > Ok, I'll cook a patch with your approach. > Alan Stern > Thanks again, Albert -- 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
