>> > - qh->dw_align_buf = dma_alloc_coherent(hsotg->dev, buf_size,
>> > - &qh->dw_align_buf_dma,
>> > - GFP_ATOMIC);
>> > + qh->dw_align_buf = kzalloc(buf_size, GFP_ATOMIC);
>>
>> Shouldn't this also set GFP_DMA? [...]
>
> No, it should be GFP_ATOMIC, as this can be reached from interrupt handler.
Are the two mutually exclusive? I meant that I think this should be
'kzalloc(buf_size, GFP_DMA | GFP_ATOMIC)', because I wouldn't be sure
that GFP_ATOMIC always returns DMA-able memory on systems that may
have limitations there.
>> > }
>> > }
>> >
>> > + qh->dw_align_buf_dma = dma_map_single(hsotg->dev,
>> > + qh->dw_align_buf, qh->dw_align_buf_size,
>> > + DMA_TO_DEVICE);
>>
>> Documentation/DMA-API.txt says that you must always use the same
>> arguments for matching dma_map_single() and dma_unmap_single()... so I
>> think this and all the unmaps should use DMA_BIDIRECTIONAL.
>
> The mapping is wrong. It should consider endpoint direction. Something like chan->ep_is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE
Yeah, I think this should work as well.
However, looking at this again I think there are more problems on
unmapping. Since some of those calls are guarded by chan->ep_is_in,
you will not unmap the memory out OUT transfers. DMA map/unmap calls
must always be balanced.
I think in all the cases where the code previously had something like
if (chan->align_buf && chan->ep_is_in) {
memcpy(...);
...;
}
you'll need to change it into
if (chan->align_buf) {
if (chan->ep_is_in) {
memcpy(...);
dma_unmap_single(..., DMA_FROM_DEVICE);
...
} else {
dma_unmap_single(..., DMA_TO_DEVICE);
}
}
You might also want to double-check all abnormal error paths to ensure
you're not leaking a DMA mapping. The previous code might not have
been so careful (since for a really bad error you usually don't care
about memcpy()ing the receive buffer back), but if you use DMA
mappings you have to.
--
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
