Re: [U-Boot] [PATCH v3 4/4] usb: dwc2: invalidate dcache before starting DMA

On 2016年07月12日 20:59, Simon Glass wrote:

Hi Ziyuan,

On 6 July 2016 at 03:34, Ziyuan Xu xzy.xu@rock-chips.com wrote:

...

From: Xu Ziyuan xzy.xu@rock-chips.com

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 xzy.xu@rock-chips.com

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(&reg->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, &reg->out_endp[ep_num].doepdma);
     writel(DOEPT_SIZ_PKT_CNT(pktcnt) | DOEPT_SIZ_XFER_SIZE(length),
            &reg->out_endp[ep_num].doeptsiz);
  

-- 1.9.1

Regards, Simon