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

[Ziyuan Xu]

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

On 6 July 2016 at 03:34, Ziyuan Xu <xzy...@rock-chips.com> wrote:
From: Xu Ziyuan <xzy...@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...@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





_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot