> -----Ursprüngliche Nachricht----- > Von: Bin Meng [mailto:[email protected]] > Gesendet: Montag, 17. Dezember 2018 09:05 > An: Stefan Theil > Cc: U-Boot Mailing List; Michal Simek > Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for > RX and TX > > Hi Stefan, > > On Mon, Dec 17, 2018 at 3:57 PM Stefan Theil <Stefan.Theil@mixed- > mode.de> wrote: > > > > > -----Ursprüngliche Nachricht----- > > > Von: Bin Meng [mailto:[email protected]] > > > Gesendet: Montag, 17. Dezember 2018 08:52 > > > An: Stefan Theil > > > Cc: U-Boot Mailing List; Michal Simek > > > Betreff: Re: [PATCH v4] zynq-gem: Use appropriate cache > > > flush/invalidate for RX and TX > > > > > > Hi Stefan, > > > > > > On Mon, Dec 17, 2018 at 3:49 PM Stefan Theil <stefan.theil@mixed- > > > mode.de> wrote: > > > > > > > > The cache was only flushed before *transmitting* packets, but not > > > > when receiving them, leading to an issue where new packets were > > > > handed to the receive handler with old contents in cache. This > > > > only happens when a lot of packets are received without sending > > > > packages every now and then. Also flushing the receive buffers in > > > > the transmit function makes no sense and can be removed. > > > > > > > > Signed-off-by: Stefan Theil <[email protected]> > > > > > > > > --- > > > > Changes for v2: > > > > - Use invalidate_dcache_range instead of > > > > flush_dcache_range > > > > Changes for v3: > > > > - Remove unnecessary flushing of all RX > > > > buffers in zynq_gem_send Changes for v4: > > > > - Invalidate receive buffers after allocating > > > > them in zynq_gem_probe > > > > --- > > > > drivers/net/zynq_gem.c | 12 ++++++------ > > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index > > > > 9bd79b198a..79a22fb1ed 100644 > > > > --- a/drivers/net/zynq_gem.c > > > > +++ b/drivers/net/zynq_gem.c > > > > @@ -570,11 +570,6 @@ static int zynq_gem_send(struct udevice *dev, > > > void *ptr, int len) > > > > 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(); > > > > > > > > /* Start transmit */ > > > > @@ -621,6 +616,9 @@ static int zynq_gem_recv(struct udevice *dev, > > > > int flags, uchar **packetp) > > > > > > > > *packetp = (uchar *)(uintptr_t)addr; > > > > > > > > + invalidate_dcache_range(addr, addr + > > > > + roundup(PKTSIZE_ALIGN, > > > ARCH_DMA_MINALIGN)); > > > > + barrier(); > > > > + > > > > return frame_len; > > > > } > > > > > > > > @@ -705,7 +703,9 @@ static int zynq_gem_probe(struct udevice *dev) > > > > if (!priv->rxbuffers) > > > > return -ENOMEM; > > > > > > > > - memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN); > > > > + u32 addr = (ulong)priv->rxbuffers; > > > > + invalidate_dcache_range(addr, addr + roundup(RX_BUF * > > > PKTSIZE_ALIGN, ARCH_DMA_MINALIGN)); > > > > + barrier(); > > > > > > > > > > Does this fix anything? I see no need to update this. > > > > I was just following Michal's suggestion (https://lists.denx.de/pipermail/u- > boot/2018-December/351969.html): > > > 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) > > > > Michal was suggesting 'flush', not 'invalidate', to make sure > priv->rxbuffers is really initialized to zero by memset(). >
Oh you're right. Shouldn't submit patches before my first coffee... I'll fix that. > > > > > > > /* Align bd_space to MMU_SECTION_SHIFT */ > > > > bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE); > > > > -- > > > > > Regards, > Bin _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
