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

[Simon Glass]

Hi Marek,

On 5 April 2017 at 15:34, Marek Vasut <marex at denx.de> wrote:
> 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.

Yes the driver work fine without DM and the code is correct. The
buffer is in BSS in that case and we don't have the cache problem. I
think we would have seen this problem before :-)

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

OK.

Regards,
Simon