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

[Simon Glass]

Hi Marek,

On 6 April 2017 at 09:33, Marek Vasut <marex at denx.de> wrote:
> On 04/06/2017 04:06 PM, Simon Glass wrote:
>> Hi Marek,
>>
>> On 6 April 2017 at 04:22, Marek Vasut <marex at denx.de> wrote:
>>> On 04/06/2017 04:40 AM, Simon Glass wrote:
>>>> Hi Marek,
>>>>
>>>> On 5 April 2017 at 19:32, Marek Vasut <marex at denx.de> wrote:
>>>>> On 04/06/2017 03:24 AM, Simon Glass wrote:
>>>>>> 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 :-)
>>>>>
>>>>> I am seeing problems around this code and this patch makes sense to me,
>>>>> so I think this patch should go in as well ...
>>>>
>>>> OK, well up to you. What sort of problems?
>>>
>>> Some sticks are not detected for me and adding a small delay here
>>> magically fixes it. I suspect I'm hitting this cache issue.
>>
>> Is this with CONFIG_DM_USB or without?
>
> With.

OK, then this is no different from the rpi problem. My DM fix should
fix it for you. The current driver (without DM) works fine.

>
>> Also does your platform have a write-through or write-back cache?
>
> I think WT, but I'm not 100% sure . I can check when I have access to
> the board . It's SoCFPGA, C-A9 .

I ask because with a WT cache the problem fixes itself after a short
time (i.e. if you wait a bit before kicking the DMA the cache will
have had time to flush).

Regards,
Simon