On 10/17/2017 01:10 PM, Faiz Abbas wrote: > Hey, > > On Tuesday 17 October 2017 03:31 PM, Marek Vasut wrote: >> On 10/17/2017 07:25 AM, Faiz Abbas wrote: >>> >>> >>> 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_ab...@ti.com> writes: >>>>>>>>> Marek Vasut <ma...@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_ab...@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? >> >> As in, add support for marking memory area noncachable and then use it >> here. It shouldn't be hard, it's only about some MMU table attributes. >> > > dma_alloc_coherent() is used by many architectures (arm, x86, nios2, > nds32). I can implement the feature in arm because I can test it but > someone else needs to do it for the other architectures.
Sounds good. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot