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
