On Thu, Dec 13, 2018 at 10:02 PM Stefan Theil Stefan.Theil@mixed-mode.de wrote:
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Donnerstag, 13. Dezember 2018 14:59 An: Stefan Theil Cc: Michal Simek; U-Boot Mailing List Betreff: Re: [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
Hi Stefan,
On Thu, Dec 13, 2018 at 9:51 PM Stefan Theil <Stefan.Theil@mixed- mode.de> wrote:
-----Ursprüngliche Nachricht----- Von: Michal Simek [mailto:monstr@monstr.eu] Gesendet: Donnerstag, 13. Dezember 2018 14:48 An: Stefan Theil; Bin Meng Cc: U-Boot Mailing List Betreff: Re: [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
On 13. 12. 18 14:41, Stefan Theil wrote:
-----Ursprüngliche Nachricht----- Von: Bin Meng [mailto:bmeng.cn@gmail.com] Gesendet: Donnerstag, 13. Dezember 2018 14:37 An: Stefan Theil Cc: U-Boot Mailing List Betreff: Re: Re: [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
Hi Stefan,
On Thu, Dec 13, 2018 at 9:26 PM Stefan Theil <Stefan.Theil@mixed- mode.de> wrote: > > Hmm good question. I went with flush because that's what's done > in the transmit function: > > addr = (ulong) ptr; > addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup(len, > ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr > + size); > > addr = (ulong)priv->rxbuffers; > addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup((RX_BUF * > PKTSIZE_ALIGN), ARCH_DMA_MINALIGN);
flush_dcache_range(addr,
> addr + size); barrier(); > > But since we actually want the uncached data invalidation seems > logical. I have to admit though, I don't have much experience with caches. This patch completely fixed my problem... Maybe somebody with a bit more expertise can add their opinion?
It should be 'invalidate' primitive when it comes to the RX path. For TX path, it should be 'flush'.
Regards, Bin
Then why are all receive buffers flushed before sending a packet? In any
case, I'll try it with invalidate and submit an updated version.
When you creating packet, cpu is used and caches are filled with data which you are asking by DMA to send. That's why you need to flush them to DDR for dma to take it. If you don't do that DMA can work
with incorrect data.
I know, but you don't create *receive* packets when you're sending a
packet, are you? Why would you need to flush those before sending a packet? That's what I'm wondering about.
Why did you mention *receive* packets and sending a packet? The receive packets and send packets are two different buffers.
Take a look at "zynq_gem_send" in "drivers/net/zynq_gem.c" then, where it says:
I just took a look at the driver, and (see below)
addr = (ulong)priv->rxbuffers; addr &= ~(ARCH_DMA_MINALIGN - 1); size = roundup((RX_BUF * PKTSIZE_ALIGN), ARCH_DMA_MINALIGN); flush_dcache_range(addr, addr + size); barrier();
I'm fairly certain that "priv->rxbuffers" has nothing to do with sending a packet.
I strongly suspect the above codes are some leftovers of copy & paste. Could you please remove these codes and have a test?
On RX path if cpu touches that memory space before DMA receive data to the memory, cpu can work with incorrect data. That's why you need to invalidate that range to ensure that data which you will read is that one got from DMA.
It is kind of interesting that we didn't observe this issue.
Probably because most people don't receive enough packets between
sending packets, so the receive buffers are flushed often enough.
Regards, Bin