Alan Stern wrote:
> On Mon, 1 Mar 2010, Albert Herranz wrote:
>
>>>> 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().
>>>
>> I think that logic is not quite right.
>> Remember that the final goal is to avoid allocating USB buffers from
>> coherent memory (because the USB drivers access USB buffers without access
>> restrictions but the platform I'm working on can't write to coherent memory
>> unless it is done in 32-bit chunks).
>
> Actually the final goal is to make the mapping/unmapping algorithms
> clear and correct. One of the subgoals involves avoiding coherent USB
> buffers, but there are others as well (if you look back the through the
> linux-usb mailing list for the last few weeks you'll see a discussion
> about a controller which has to use PIO for control transfers but can
> use DMA for other types).
>
Well, I was talking about our particular case here. I did not imply that we
should forget about the other cases.
>> And we want to avoid bouncing at the USB layer too (that's what v1 did).
>>
>> The strategy so far is:
>> - we have modified the USB buffer allocation routines
>> hcd_buffer_alloc()/hcd_buffer_free() to always return normal kernel memory
>> when HCD_NO_COHERENT_MEM is set on the host controller.
>> - during map_urb_for_dma()/unmap_urb_for_dma() we need to make sure that
>> those USB buffers are sync'ed, even if we are told
>> USB_NO_{SETUP,TRANSFER}_DMA_MAP
>>
>> So the logic would be:
>>
>> If URB_NO_TRANSFER_DMA_MAP is _cleared_ then do the mapping
>
> No, that's wrong because it ignores the HCD_LOCAL_MEM flag.
>
When I said "do the mapping" there I meant to do a dma_map_single() if
self.uses_dma, else if HCD_LOCAL_MEM is set then do a hcd_alloc_coherent().
I should have been more clear on that.
>> - this case covers normal kernel memory used as a buffer and not
>> already DMA mapped by a USB driver
>>
>> Otherwise if HCD_NO_COHERENT_MEM is set _and_ num_sgs == 0 _and_
>> transfer_buffer_length > 0 then do the mapping too
>> - this case covers USB buffers allocated via usb_buffer_alloc() and
>> marked URB_NO_TRANSFER_DMA_MAP by a USB driver, which are allocated from
>> normal kernel memory when HCD_NO_COHERENT_MEM is set (we avoid bouncing
>> buffers here too, at least if they sit already within MEM2 in the Wii, but
>> anyway that's part of the platform DMA mapping code)
>>
>> s-g urbs do not need a mapping as they have already been mapped, marked
>> URB_NO_TRANSFER_DMA_MAP and have num_sgs > 0
>
> Actually the test for transfer_buffer_length == 0 should be done first,
> since obviously no mapping is needed if there's no data. (And in fact
> the current code does do this; I was wrong earlier when I said it
> doesn't.)
>
> So let's make things a little easier by first testing the conditions
> under which no mapping is needed:
>
> 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
> are both set (this avoids your HCD_NO_COHERENT_MEM
> case) then do nothing.
>
I see. This case would include the PIO case too (for which dma_handle is set to
all 1s).
So this assumes that transfer_dma should be set initially to 0 when allocating
USB buffers for HCD_NO_COHERENT_MEM.
> The remaining cases all need mapping and/or bounce buffers:
>
> Otherwise if HCD_LOCAL_MEM is set then call hcd_alloc_coherent.
> Otherwise call dma_map_single.
>
> Finally, the unmapping tests can be simplified greatly if the kind of
> mapping is recorded in the URB flags.
>
Good point.
> Alan Stern
>
Thanks for your comments,
Albert
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
