On 13. 12. 18 15:19, Bin Meng wrote: > On Thu, Dec 13, 2018 at 10:02 PM Stefan Theil > <[email protected]> wrote: >> >> >> >>> -----Ursprüngliche Nachricht----- >>> Von: Bin Meng [mailto:[email protected]] >>> 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:[email protected]] >>>>> 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:[email protected]] >>>>>>> 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 anySYS_CACHE_SHIFT_6 >>>>> 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?
I think it will be good to compare this code with macb driver which is also in the tree. Not sure what other drivers are using but zynq_gem is using non cached area for BDs and cached area for data. Also in probe there should be flush of priv->rxbuffers to make sure that it is initialized properly. (memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); - this should be useless) And then invalidation of data cache after packet is received to make sure that before cpu reads the packet cache is empty. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs
signature.asc
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
