From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ziyuan Xu Date: Wed, 13 Jul 2016 09:06:23 +0800 Subject: [U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA In-Reply-To: References: <1467797663-16276-1-git-send-email-xzy.xu@rock-chips.com> <1467797663-16276-5-git-send-email-xzy.xu@rock-chips.com> Message-ID: <5785940F.5090008@rock-chips.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 2016?07?12? 20:59, Simon Glass wrote: > Hi Ziyuan, > > On 6 July 2016 at 03:34, Ziyuan Xu wrote: >> From: Xu Ziyuan >> >> Invalidate dcache before starting the DMA to ensure coherency. In case >> there are any dirty lines from the DMA buffer in the cache, subsequent >> cache-line replacements may corrupt the buffer in memory while the DMA >> is still going on. Cache-line replacement can happen if the CPU tries to >> bring some other memory locations into the cache while the DMA is going >> on. >> >> Signed-off-by: Ziyuan Xu >> --- >> >> Changes in v3: >> - New commit since v3 to fix the coherence issue between memory and >> cache >> >> Changes in v2: None >> >> drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> index 12f5c85..0d6d2fb 100644 >> --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> @@ -110,6 +110,9 @@ static int setdma_rx(struct dwc2_ep *ep, struct dwc2_request *req) >> >> ctrl = readl(®->out_endp[ep_num].doepctl); >> >> + invalidate_dcache_range((unsigned long) ep->dma_buf, >> + (unsigned long) ep->dma_buf + ep->len); > There is an invalidate in complete_rx() which is one of the callers > for this function. It seems to me that we should not have this in two > places. Why do we have this problem? Is it because the other calls to > setdma_rx() don't invalidate? Yup, invoke invalidate method twice during one complete transmission: 1- invalidate in setdma_rx() in case of the write back cache, present from cache-line replacement after DMA transmission complete. i.e. 1) dma_buf = "123456789abcd"; 2) didn't invalidate in setdma_rx() 3) DMA complete interrupt coming 4) invalidate in complete_rx() 5) read dma_buf, it's "123456789abcd" If invalidate in step 2, we will achieve correct data. I think it's necessary to invalidate before starting DMA, and doc/README.arm-caches describe details. 2- invalidate in complete_rx(), cpu read the dma_buf from memory directly. > I think the invalidate should happen just before reading the data. Can > you please check if the other invalidate is needed? Also see how it > cache-aligns the end address. memalign(CONFIG_SYS_CACHELINE_SIZE, EP_BUFFER_SIZE); cache-aligns: 64 bytes. dma_buffer size: 4096 I had check cache-aligins and end address, rightful. Furthermore, everything works fine with noncached_alloc(). > >> + >> writel((unsigned int) ep->dma_buf, ®->out_endp[ep_num].doepdma); >> writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length), >> ®->out_endp[ep_num].doeptsiz); >> -- >> 1.9.1 >> >> > Regards, > Simon > > >