From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felipe Balbi Date: Mon, 16 Oct 2017 17:51:36 +0300 Subject: [U-Boot] [PATCH v2] usb: dwc3: Allocate and flush dwc->ep0_trb in a cache aligned manner In-Reply-To: <2e7a3942-1d71-600e-a2a7-22ed219142d8@ti.com> References: <1508131315-23549-1-git-send-email-faiz_abbas@ti.com> <72a54562-4437-a008-37bc-f11b2440aecc@denx.de> <87inffkxjc.fsf@linux.intel.com> <2f303f10-36a0-1e7a-75e0-02c88fd08474@ti.com> <87efq3kwd9.fsf@linux.intel.com> <2e7a3942-1d71-600e-a2a7-22ed219142d8@ti.com> Message-ID: <878tgbkux3.fsf@linux.intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, Faiz Abbas writes: >>>> Marek Vasut 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 >>>>> >>>>> 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 -- balbi