* [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
@ 2018-12-13 10:18 Stefan Theil
2018-12-13 13:12 ` Bin Meng
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Stefan Theil @ 2018-12-13 10:18 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Stefan Theil <stefan.theil@mixed-mode.de>
---
drivers/net/zynq_gem.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index 9bd79b198a..5825b6dd99 100644
--- a/drivers/net/zynq_gem.c
+++ b/drivers/net/zynq_gem.c
@@ -621,6 +621,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
*packetp = (uchar *)(uintptr_t)addr;
+ flush_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
+ barrier();
+
return frame_len;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler
2018-12-13 10:18 [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler Stefan Theil
@ 2018-12-13 13:12 ` Bin Meng
2018-12-17 7:49 ` [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX Stefan Theil
2018-12-17 8:12 ` [U-Boot] [PATCH v5] " Stefan Theil
2 siblings, 0 replies; 10+ messages in thread
From: Bin Meng @ 2018-12-13 13:12 UTC (permalink / raw)
To: u-boot
On Thu, Dec 13, 2018 at 6:18 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.
>
> Signed-off-by: Stefan Theil <stefan.theil@mixed-mode.de>
> ---
> drivers/net/zynq_gem.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index 9bd79b198a..5825b6dd99 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -621,6 +621,9 @@ static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
>
> *packetp = (uchar *)(uintptr_t)addr;
>
> + flush_dcache_range(addr, addr + roundup(PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
Shouldn't it be invalidate_dcache_range()?
> + barrier();
> +
> return frame_len;
> }
>
Regards,
Bin
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-13 10:18 [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler Stefan Theil
2018-12-13 13:12 ` Bin Meng
@ 2018-12-17 7:49 ` Stefan Theil
2018-12-17 7:52 ` Bin Meng
2018-12-17 8:12 ` [U-Boot] [PATCH v5] " Stefan Theil
2 siblings, 1 reply; 10+ messages in thread
From: Stefan Theil @ 2018-12-17 7:49 UTC (permalink / raw)
To: u-boot
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 <stefan.theil@mixed-mode.de>
---
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();
/* Align bd_space to MMU_SECTION_SHIFT */
bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 7:49 ` [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX Stefan Theil
@ 2018-12-17 7:52 ` Bin Meng
2018-12-17 7:57 ` Stefan Theil
0 siblings, 1 reply; 10+ messages in thread
From: Bin Meng @ 2018-12-17 7:52 UTC (permalink / raw)
To: u-boot
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 <stefan.theil@mixed-mode.de>
>
> ---
> 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.
> /* Align bd_space to MMU_SECTION_SHIFT */
> bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
> --
Regards,
Bin
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 7:52 ` Bin Meng
@ 2018-12-17 7:57 ` Stefan Theil
2018-12-17 8:05 ` Bin Meng
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Theil @ 2018-12-17 7:57 UTC (permalink / raw)
To: u-boot
> -----Ursprüngliche Nachricht-----
> Von: Bin Meng [mailto:bmeng.cn at gmail.com]
> 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 <stefan.theil@mixed-mode.de>
> >
> > ---
> > 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)
>
> > /* Align bd_space to MMU_SECTION_SHIFT */
> > bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
> > --
>
> Regards,
> Bin
Regards,
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 7:57 ` Stefan Theil
@ 2018-12-17 8:05 ` Bin Meng
2018-12-17 8:08 ` Stefan Theil
0 siblings, 1 reply; 10+ messages in thread
From: Bin Meng @ 2018-12-17 8:05 UTC (permalink / raw)
To: u-boot
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:bmeng.cn at gmail.com]
> > 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 <stefan.theil@mixed-mode.de>
> > >
> > > ---
> > > 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().
> >
> > > /* Align bd_space to MMU_SECTION_SHIFT */
> > > bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
> > > --
> >
Regards,
Bin
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 8:05 ` Bin Meng
@ 2018-12-17 8:08 ` Stefan Theil
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Theil @ 2018-12-17 8:08 UTC (permalink / raw)
To: u-boot
> -----Ursprüngliche Nachricht-----
> Von: Bin Meng [mailto:bmeng.cn at gmail.com]
> 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:bmeng.cn at gmail.com]
> > > 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 <stefan.theil@mixed-mode.de>
> > > >
> > > > ---
> > > > 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v5] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-13 10:18 [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler Stefan Theil
2018-12-13 13:12 ` Bin Meng
2018-12-17 7:49 ` [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX Stefan Theil
@ 2018-12-17 8:12 ` Stefan Theil
2018-12-17 8:25 ` Bin Meng
2018-12-20 8:52 ` Michal Simek
2 siblings, 2 replies; 10+ messages in thread
From: Stefan Theil @ 2018-12-17 8:12 UTC (permalink / raw)
To: u-boot
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 <stefan.theil@mixed-mode.de>
---
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
Changes for v5:
- Clear and flush receive buffers in
zynq_gem_probe instead of invalidating the
cache
---
drivers/net/zynq_gem.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
index 9bd79b198a..3bd0093b7a 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;
}
@@ -706,6 +704,9 @@ static int zynq_gem_probe(struct udevice *dev)
return -ENOMEM;
memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
+ u32 addr = (ulong)priv->rxbuffers;
+ flush_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
+ barrier();
/* Align bd_space to MMU_SECTION_SHIFT */
bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v5] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 8:12 ` [U-Boot] [PATCH v5] " Stefan Theil
@ 2018-12-17 8:25 ` Bin Meng
2018-12-20 8:52 ` Michal Simek
1 sibling, 0 replies; 10+ messages in thread
From: Bin Meng @ 2018-12-17 8:25 UTC (permalink / raw)
To: u-boot
On Mon, Dec 17, 2018 at 4:12 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 <stefan.theil@mixed-mode.de>
>
> ---
> 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
> Changes for v5:
> - Clear and flush receive buffers in
> zynq_gem_probe instead of invalidating the
> cache
> ---
> drivers/net/zynq_gem.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH v5] zynq-gem: Use appropriate cache flush/invalidate for RX and TX
2018-12-17 8:12 ` [U-Boot] [PATCH v5] " Stefan Theil
2018-12-17 8:25 ` Bin Meng
@ 2018-12-20 8:52 ` Michal Simek
1 sibling, 0 replies; 10+ messages in thread
From: Michal Simek @ 2018-12-20 8:52 UTC (permalink / raw)
To: u-boot
On 17. 12. 18 9:12, Stefan Theil 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 <stefan.theil@mixed-mode.de>
>
> ---
> 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
> Changes for v5:
> - Clear and flush receive buffers in
> zynq_gem_probe instead of invalidating the
> cache
> ---
> drivers/net/zynq_gem.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index 9bd79b198a..3bd0093b7a 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;
> }
>
> @@ -706,6 +704,9 @@ static int zynq_gem_probe(struct udevice *dev)
> return -ENOMEM;
>
> memset(priv->rxbuffers, 0, RX_BUF * PKTSIZE_ALIGN);
> + u32 addr = (ulong)priv->rxbuffers;
> + flush_dcache_range(addr, addr + roundup(RX_BUF * PKTSIZE_ALIGN, ARCH_DMA_MINALIGN));
> + barrier();
>
> /* Align bd_space to MMU_SECTION_SHIFT */
> bd_space = memalign(1 << MMU_SECTION_SHIFT, BD_SPACE);
>
Applied.
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181220/2948a503/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-12-20 8:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 10:18 [U-Boot] [PATCH] zynq-gem: Flush cache before handing RX packet to receive handler Stefan Theil
2018-12-13 13:12 ` Bin Meng
2018-12-17 7:49 ` [U-Boot] [PATCH v4] zynq-gem: Use appropriate cache flush/invalidate for RX and TX Stefan Theil
2018-12-17 7:52 ` Bin Meng
2018-12-17 7:57 ` Stefan Theil
2018-12-17 8:05 ` Bin Meng
2018-12-17 8:08 ` Stefan Theil
2018-12-17 8:12 ` [U-Boot] [PATCH v5] " Stefan Theil
2018-12-17 8:25 ` Bin Meng
2018-12-20 8:52 ` Michal Simek
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.