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

[Marek Vasut]

On 04/05/2017 05:03 PM, Simon Glass wrote:
> +Tom
> 
> Hi Marek,
> 
> On 5 April 2017 at 04:21, Marek Vasut <marex at denx.de> wrote:
>> On 04/05/2017 12:08 PM, Simon Glass wrote:
>>> Hi Marek,
>>>
>>> On 5 April 2017 at 03:35, Marek Vasut <marex at denx.de> wrote:
>>>> On 04/05/2017 04:21 AM, Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On 4 April 2017 at 19:26, Kever Yang <kever.yang at rock-chips.com> wrote:
>>>>>> Hi Eddie,
>>>>>>
>>>>>>
>>>>>>     We should only need to do only one time cache operation for a buffer
>>>>>>
>>>>>> ready to do DMA transfer, so you need to remove another cache invalidate
>>>>>>
>>>>>> operation for the same buffer in the same function.
>>>>>
>>>>> I think this is a more general problem and might cause issues with
>>>>> other drivers. So I have sent this patch:
>>>>>
>>>>> http://patchwork.ozlabs.org/patch/746917/
>>>>>
>>>>
>>>> This feels like papering over a problem though ... which will bite you
>>>> later anyway.
>>>
>>> I believe the problem only happens because we have cached zero bytes
>>> caused by this function. If the driver does the right thing (as dwc2.c
>>> already does) then everything should be find from then on.
>>
>> And I think the driver is where this should be fixed ? That is, the
>> driver should do the right thing and flush/invalidate caches correctly.
>>
>>> Notice that the problem does not happen without driver model, since
>>> non-DM code in dwc2.c uses BSS for the buffers, which is zeroed with
>>> the cache off.
> 
> I'm not sure if you read the long and windy thread with Stefan B but
> the upshot is that the driver is doing the right thing.
> 
> If the driver were doing the memset() then you could make a case that
> we should change the driver, but since DM is doing it and DM is
> promising that DMA can be used on the buffer, I think the best place
> for the fix is in DM. The driver should not need to change and neither
> should any other driver when we convert it to DM.
> 
> On that last point I really want to avoid having to change the caching
> behaviour of drivers just to work with DM!

So will the driver work with your patch and without DM ? I don't think
so, so I think what Eddie's patch does is correct. I'd really like him
to send a new version and apply that.

If this also needs to be fixed in DM, so be it.

> Regards,
> Simon
> 


-- 
Best regards,
Marek Vasut