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

Faiz Abbas faiz_abbas at ti.com
Tue Oct 17 05:25:03 UTC 2017



On Monday 16 October 2017 08:52 PM, Marek Vasut wrote:
> On 10/16/2017 04:51 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Faiz Abbas <faiz_abbas at ti.com> writes:
>>>>>> Marek Vasut <marex at denx.de> writes:
>>>>>>> On 10/16/2017 07:21 AM, Faiz Abbas wrote:
>>>>>>>> A flush of the cache is required before any outbound DMA access can
>>>>>>>> take place. The minimum size that can be flushed from the cache is
>>>>>>>> one cache line size. Therefore, any buffer allocated for DMA should
>>>>>>>> be in multiples of cache line size.
>>>>>>>>
>>>>>>>> Thus, allocate memory for ep0_trb in multiples of cache line size.
>>>>>>>>
>>>>>>>> Also, when local variable trb is assigned to dwc->ep0_trb[1] and used
>>>>>>>> to flush cache, it leads to cache misaligned messages as only the base
>>>>>>>> address dwc->ep0_trb is cache aligned.
>>>>>>>>
>>>>>>>> Therefore, flush cache using ep0_trb_addr which is always cache aligned.
>>>>>>>>
>>>>>>>> Signed-off-by: Faiz Abbas <faiz_abbas at ti.com>
>>>>>>>
>>>>>>> SGTM, Felipe, can you review this please ?
>>>>>>
>>>>>> is cache maintenance done correctly in u-boot? Isn't the whole idea of a
>>>>>> coherent memory area that is is non-cacheable, non-bufferable memory?
>>>>>>
>>>>>> Also, why isn't the API itself guaranteeing alignment requirements?
>>>>>>
>>>>> There is no support in u-boot to make a memory area non-cacheable.
>>>>> This is the definition of dma_alloc_coherent()
>>>>>
>>>>> static inline void *dma_alloc_coherent(size_t len, unsigned long *handle)
>>>>> {
>>>>>         *handle = (unsigned long)memalign(ARCH_DMA_MINALIGN, len);
>>>>>         return (void *)*handle;
>>>>> }
>>>>>
>>>>> This driver is mostly copied from kernel (where dma_alloc_coherent() is
>>>>> what you describe) and extra flush_cache functions are added because of
>>>>> U-Boot's inability to allocate coherent memory.
>>>>
>>>> then that's what should be fixed. No?
>>>>
>>>
>>> You're right but that sounds like a long-term feature which will affect
>>> a huge part of u-boot. Until it is implemented, I guess this is the best
>>> way to handle the issue.
>>
>> Not my call to make. I'll defer to Marek and Tom
>>
> We're deep in RC anyway, so feel free to prepare a fix for next MW .
> 

Fix as in rebase same patch for next merge window?

Thanks,
Faiz


More information about the U-Boot mailing list