[U-Boot] [PATH V2] usb: dwc2: invalidate the dcache before starting the DMA

[Marek Vasut]

On 04/06/2017 10:34 AM, Kever Yang wrote:
> Hi Eddie,
> 
> 
> On 04/06/2017 10:14 AM, Marek Vasut wrote:
>> On 04/06/2017 04:03 AM, Eddie Cai wrote:
>>> We should invalidate the dcache before starting the DMA. In case
>>> there are
>>> any dirty lines from the DMA buffer in the cache, subsequent cache-line
>>> replacements may corrupt the buffer in memory while the DMA is still
>>> going on.
>>> Cache-line replacement can happen if the CPU tries to bring some
>>> other memory
>>> locations into the cache while the DMA is going on.
>>>
>>> Signed-off-by: Eddie Cai <eddie.cai.linux at gmail.com>
>>> Reviewed-by: Stefan Brüns <stefan.bruens at rwth-aachen.de>
>>> ---
>>>   drivers/usb/host/dwc2.c | 19 +++++++++++++------
>>>   1 file changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>>> index 5ac602e..e3a5d7d 100644
>>> --- a/drivers/usb/host/dwc2.c
>>> +++ b/drivers/usb/host/dwc2.c
>>> @@ -811,12 +811,19 @@ static int transfer_chunk(struct dwc2_hc_regs
>>> *hc_regs, void *aligned_buffer,
>>>              (*pid << DWC2_HCTSIZ_PID_OFFSET),
>>>              &hc_regs->hctsiz);
>>>   -    if (!in && xfer_len) {
>>> -        memcpy(aligned_buffer, buffer, xfer_len);
>>> -
>>> -        flush_dcache_range((unsigned long)aligned_buffer,
>>> -                   (unsigned long)aligned_buffer +
>>> -                   roundup(xfer_len, ARCH_DMA_MINALIGN));
>>> +    if (xfer_len) {
>>> +        if (in) {
>>> +            invalidate_dcache_range(
>>> +                    (unsigned long)aligned_buffer,
>>> +                    (unsigned long)aligned_buffer +
>> Should be uintptr_t all over the place to deal with 32/64 bit systems,
>> otherwise great, thanks.
>>
>>> +                    roundup(xfer_len, ARCH_DMA_MINALIGN));
>>> +        } else {
>>> +            memcpy(aligned_buffer, buffer, xfer_len);
>>> +            flush_dcache_range(
>>> +                    (unsigned long)aligned_buffer,
>>> +                    (unsigned long)aligned_buffer +
>>> +                    roundup(xfer_len, ARCH_DMA_MINALIGN));
>>> +        }
>>>       }
>>>         writel(phys_to_bus((unsigned long)aligned_buffer),
>>> &hc_regs->hcdma);
>>>
> 
> Did you test with remove another invalidate_dcache_range() in this
> function?
> I still think no need to have 2 invalidate_dcache in this function.

You need the first invalidate to prevent having data stuck in the cache
and accidentally being evicted into the RAM while the USB DMA is also
running , thus creating a total mess of the result.


-- 
Best regards,
Marek Vasut