[U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

> On 04 Apr 2017, at 21:01, Marek Vasut <marex at denx.de> wrote:
> 
>> Good point on the “long”, especially as I just copied this from other occurrences and it’s consistently wrong throughout DWC3 in U-Boot:
> 
> Hrm, I thought the driver was ported over from Linux, so is this broken
> in Linux too ?

Apparently, the dwc3_flush_cache calls (and the function itself) have been
introduced during the porting. There’s no explicit cache-maintenance in DWC3
for Linux. 

>> I’ll revise all of these and make a patch-series out of this.
> 
> Maybe you should check the Linux first and see if there are some fixes
> already.
> 
> Thanks

Given that this seems to have been introduced with the port to U-Boot, there’s
no applicable fixes there.

>>>> 	return evt;
>>>> }
>>>> 
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index 1156662..61af71b 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -2668,11 +2668,12 @@ void dwc3_gadget_uboot_handle_interrupt(struct dwc3 *dwc)
>>>> 		int i;
>>>> 		struct dwc3_event_buffer *evt;
>>>> 
>>>> +		dwc3_thread_interrupt(0, dwc);
>>>> +
>>>> +		/* Clean + Invalidate the buffers after touching them */
>>>> 		for (i = 0; i < dwc->num_event_buffers; i++) {
>>>> 			evt = dwc->ev_buffs[i];
>>>> 			dwc3_flush_cache((long)evt->buf, evt->length);
>>>> 		}
>>>> -
>>> 
>>> This makes me wonder, don't you need to invalidate the event buffer
>>> somewhere so that the new data would be fetched from RAM ?
>> 
>> We flush the event buffer before leaving the function.
>> So the cache line will not be present in the cache, when we enter this function again.
> 
> Then shouldn't we invalidate it instead ? flush and invalidate are two
> different things …

The DWC3 flush expands to a clean+invalidate. It is not wrong, as long as
it is used as in my patch:
a. before the first time data is expected to be written by the peripheral (i.e.
before the peripheral is started)—to ensure that the cache line is not cached
any longer…
b. after the driver modifies any buffers (i.e. anything modified will be written
back) and before it next reads the buffers expecting possibly changed data
(i.e. invalidating).

Regards,
Philipp.