[U-Boot] [PATCH] dwc: ep0: Allocate and flush dwc->ep0_trb in a cache aligned manner
Faiz Abbas
faiz_abbas at ti.com
Wed Oct 11 13:23:58 UTC 2017
On Wednesday 11 October 2017 02:28 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 11 October 2017 01:53 PM, Faiz Abbas wrote:
>> Hi,
>>
>> On Tuesday 10 October 2017 07:19 PM, Marek Vasut wrote:
>>> 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 ?
>>>
>> No, control to the hardware is only given after all the trbs have been
>> configured and flushed to memory. This is done by using the chain
>> variable in the code.
>>
>> dwc3_flush_cache((uintptr_t)buf_dma, len);
>> dwc3_flush_cache((uintptr_t)dwc->ep0_trb_addr, sizeof(*trb) * 2);
>
> this flush_cache can be moved after the chain so that flush is only invoked
> after all the TRB's has been configured.
Sure, that can be done.
Thanks,
Faiz
More information about the U-Boot
mailing list