On 04/04/2017 09:56 PM, Dr. Philipp Tomsich wrote: > >> On 04 Apr 2017, at 21:01, Marek Vasut <ma...@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.
OK >>> 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. OK >>>>> 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… So invalidate() is enough ? > 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). So flush+invalidate ? Keep in mind this driver may not be used on ARMv7/v8 only ... -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot