[U-Boot] [PATCH] dwc: ep0: Allocate and flush dwc->ep0_trb in a cache aligned manner

[Marek Vasut]

On 10/10/2017 12:45 PM, Faiz Abbas wrote:
> Hi Marek,
> 
> On Tuesday 10 October 2017 01:30 PM, Marek Vasut wrote:
>> On 10/10/2017 07:48 AM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>
>> Hi,
>>
>> [...]
>>
>>>>>>>>>>>>> -	dwc3_flush_cache((uintptr_t)trb, sizeof(*trb));
>>>>>>>>>>>>> +	dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2);
>>>>>>>>>>>>
>>>>>>>>>>>> Why *2 ?
>>>>>>>>>>>
>>>>>>>>>>> Because its allocated as sizeof(*dwc->ep0_trb) * 2 below. This is not
>>>>>>>>>>> strictly required as dwc3_flush_cache() rounds up the size to
>>>>>>>>>>> CACHELINE_SIZE but from a caller POV, flush everything we allocated.
>>>>>>>>>>
>>>>>>>>>> Can the other TRB be in use ? Maybe aligning the TRBs to cacheline size
>>>>>>>>>> would be better ?
>>>>>>>>>>
>>>>>>>>> A single trb is 16 bytes in size and two of them are allocated
>>>>>>>>> contiguously.
>>>>>>>>
>>>>>>>> Why are two allocated continuously ? (I am not dwc3 expert)
>>>
>>> The TRB's should be allocated contiguously for dwc3 and only the base of the
>>> entire TRB table is programmed in the HW.
>>>  ________________ <------------------ TRB table base address
>>> |     TRB0       |
>>> |________________|
>>> |     TRB1       |
>>> |________________|
>>> |     TRB2       |
>>> |________________|
>>> |     TRBn       |
>>> |________________|
>>>
>>>
>>>>>>>
>>>>>>> Neither am I. I did try to pad to the dwc_trb structure such that each
>>>>>>> trb is 64 bytes in size but this leads to failures when testing. I
>>>>>>> didn't get a chance to debug this though. I suspect its because the code
>>>>>>> expects the trbs to be contiguous and/or 16 bytes in size.
>>>
>>> It's not the code but it's the HW.
>>
>> That'd imply we need either some sort of flushing scheme or non-cachable
>> memory allocation. What does Linux do ?
>> The dma_alloc_coherent in linux kernel allocates non-cachable memory.
> 
> Currently, the code is using local variable trb to flush the cache. When
> the first trb is used, dwc3_flush_cache flushes the complete
> CACHELINE_SIZE (including the 2nd trb).
> When the 2nd trb is used to flush cache, since it is an unaligned flush,
> it will issue a warning and will align it to the lower cache line
> boundary (flushing the 1st trb in the process).
> 
> So with or without this patch, both trbs are getting flushed with every
> call. With the patch, we can just avoid misaligned messages by always
> flushing using an aligned address.

What worries me is that you can flush something into the memory while
the controller is writing into it as well. Is that possible ?

-- 
Best regards,
Marek Vasut