mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] net: macb: dma_sync_* receive buffers
@ 2018-11-15  0:36 Ladislav Michl
  2018-11-16 15:07 ` Lucas Stach
  0 siblings, 1 reply; 5+ messages in thread
From: Ladislav Michl @ 2018-11-15  0:36 UTC (permalink / raw)
  To: barebox

Receive buffers are properly synchronized only if Cadence is
GEM. Fix it for MACB as well.

Fixes: 86dc5259e25d (net: macb: no need for coherent memory for receive buffer)
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Hi Sascha,

 I'm unsure this is proper fix, but works for me (as well as
 reverting 86dc5259e25d)

 drivers/net/macb.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 1c5752d10..2bd5b62a4 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -216,9 +216,11 @@ static int macb_recv(struct eth_device *edev)
 	dev_dbg(macb->dev, "%s\n", __func__);
 
 	for (;;) {
+		barrier();
 		if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED)))
 			return -1;
 
+		barrier();
 		status = macb->rx_ring[rx_tail].ctrl;
 		if (status & MACB_BIT(RX_SOF)) {
 			if (rx_tail != macb->rx_tail)
@@ -229,6 +231,8 @@ static int macb_recv(struct eth_device *edev)
 		if (status & MACB_BIT(RX_EOF)) {
 			buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
 			length = MACB_BFEXT(RX_FRMLEN, status);
+			dma_sync_single_for_cpu((unsigned long)buffer, length,
+						DMA_FROM_DEVICE);
 			if (wrapped) {
 				unsigned int headlen, taillen;
 
@@ -237,12 +241,17 @@ static int macb_recv(struct eth_device *edev)
 				taillen = length - headlen;
 				memcpy((void *)NetRxPackets[0],
 				       buffer, headlen);
+				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
+							taillen, DMA_FROM_DEVICE);
 				memcpy((void *)NetRxPackets[0] + headlen,
 				       macb->rx_buffer, taillen);
 				buffer = (void *)NetRxPackets[0];
 			}
 
 			net_receive(edev, buffer, length);
+			dma_sync_single_for_device((unsigned long)buffer, length,
+						   DMA_FROM_DEVICE);
+			barrier();
 			if (++rx_tail >= macb->rx_ring_size)
 				rx_tail = 0;
 			reclaim_rx_buffers(macb, rx_tail);
@@ -252,7 +261,6 @@ static int macb_recv(struct eth_device *edev)
 				rx_tail = 0;
 			}
 		}
-		barrier();
 	}
 
 	return 0;
-- 
2.19.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: macb: dma_sync_* receive buffers
  2018-11-15  0:36 [PATCH] net: macb: dma_sync_* receive buffers Ladislav Michl
@ 2018-11-16 15:07 ` Lucas Stach
  2018-12-04  7:58   ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Lucas Stach @ 2018-11-16 15:07 UTC (permalink / raw)
  To: Ladislav Michl, barebox

Am Donnerstag, den 15.11.2018, 01:36 +0100 schrieb Ladislav Michl:
> Receive buffers are properly synchronized only if Cadence is
> GEM. Fix it for MACB as well.
> 
> Fixes: 86dc5259e25d (net: macb: no need for coherent memory for receive buffer)
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  Hi Sascha,
> 
>  I'm unsure this is proper fix, but works for me (as well as
>  reverting 86dc5259e25d)
> 
>  drivers/net/macb.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 1c5752d10..2bd5b62a4 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -216,9 +216,11 @@ static int macb_recv(struct eth_device *edev)
> >  	dev_dbg(macb->dev, "%s\n", __func__);
>  
> >  	for (;;) {
> > +		barrier();
> >  		if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED)))
> >  			return -1;
>  
> > +		barrier();
> >  		status = macb->rx_ring[rx_tail].ctrl;
> >  		if (status & MACB_BIT(RX_SOF)) {
> >  			if (rx_tail != macb->rx_tail)
> @@ -229,6 +231,8 @@ static int macb_recv(struct eth_device *edev)
> >  		if (status & MACB_BIT(RX_EOF)) {
> >  			buffer = macb->rx_buffer + macb->rx_buffer_size * macb->rx_tail;
> >  			length = MACB_BFEXT(RX_FRMLEN, status);
> > +			dma_sync_single_for_cpu((unsigned long)buffer, length,
> > +						DMA_FROM_DEVICE);
> >  			if (wrapped) {
> >  				unsigned int headlen, taillen;
>  
> @@ -237,12 +241,17 @@ static int macb_recv(struct eth_device *edev)
> >  				taillen = length - headlen;
> >  				memcpy((void *)NetRxPackets[0],
> >  				       buffer, headlen);
> > +				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
> > +							taillen, DMA_FROM_DEVICE);
> >  				memcpy((void *)NetRxPackets[0] + headlen,
>  				       macb->rx_buffer, taillen);

This is missing a dma_sync_single_for_device() here, matching the
...for_cpu added above. Otherwise patch looks good.

Regards,
Lucas

>  				buffer = (void *)NetRxPackets[0];
> >  			}
>  
> >  			net_receive(edev, buffer, length);
> > +			dma_sync_single_for_device((unsigned long)buffer, length,
> > +						   DMA_FROM_DEVICE);
> > +			barrier();
> >  			if (++rx_tail >= macb->rx_ring_size)
> >  				rx_tail = 0;
> >  			reclaim_rx_buffers(macb, rx_tail);
> @@ -252,7 +261,6 @@ static int macb_recv(struct eth_device *edev)
> >  				rx_tail = 0;
> >  			}
> >  		}
> > -		barrier();
> >  	}
>  
> >  	return 0;

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: macb: dma_sync_* receive buffers
  2018-11-16 15:07 ` Lucas Stach
@ 2018-12-04  7:58   ` Sascha Hauer
  2019-05-30 12:28     ` Ladislav Michl
  0 siblings, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2018-12-04  7:58 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: barebox

Ladis,

On Fri, Nov 16, 2018 at 04:07:25PM +0100, Lucas Stach wrote:
> > @@ -237,12 +241,17 @@ static int macb_recv(struct eth_device *edev)
> > >  				taillen = length - headlen;
> > >  				memcpy((void *)NetRxPackets[0],
> > >  				       buffer, headlen);
> > > +				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
> > > +							taillen, DMA_FROM_DEVICE);
> > >  				memcpy((void *)NetRxPackets[0] + headlen,
> >  				       macb->rx_buffer, taillen);
> 
> This is missing a dma_sync_single_for_device() here, matching the
> ...for_cpu added above. Otherwise patch looks good.

Any input to this one or an updated patch?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: macb: dma_sync_* receive buffers
  2018-12-04  7:58   ` Sascha Hauer
@ 2019-05-30 12:28     ` Ladislav Michl
  2019-06-04  6:49       ` Sascha Hauer
  0 siblings, 1 reply; 5+ messages in thread
From: Ladislav Michl @ 2019-05-30 12:28 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Tue, Dec 04, 2018 at 08:58:01AM +0100, Sascha Hauer wrote:
> Ladis,
> 
> On Fri, Nov 16, 2018 at 04:07:25PM +0100, Lucas Stach wrote:
> > > @@ -237,12 +241,17 @@ static int macb_recv(struct eth_device *edev)
> > > >  				taillen = length - headlen;
> > > >  				memcpy((void *)NetRxPackets[0],
> > > >  				       buffer, headlen);
> > > > +				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
> > > > +							taillen, DMA_FROM_DEVICE);
> > > >  				memcpy((void *)NetRxPackets[0] + headlen,
> > >  				       macb->rx_buffer, taillen);
> > 
> > This is missing a dma_sync_single_for_device() here, matching the
> > ...for_cpu added above. Otherwise patch looks good.
> 
> Any input to this one or an updated patch?

This must have slipped to the very bottom of my mailbox, sorry.

We should also not running sync outside buffer, if it is wrapped.
Updated patch bellow.

	ladis

--- 8< ---

From: Ladislav Michl <ladis@linux-mips.org>
Subject: [PATCH v2] net: macb: dma_sync_* receive buffers

Receive buffers are properly synchronized only if Cadence is
GEM. Fix it for MACB as well.

Fixes: 86dc5259e25d (net: macb: no need for coherent memory for receive buffer)
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/net/macb.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 0c0d17ee9..a0411d6e4 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -209,9 +209,11 @@ static int macb_recv(struct eth_device *edev)
 	dev_dbg(macb->dev, "%s\n", __func__);
 
 	for (;;) {
+		barrier();
 		if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED)))
 			return -1;
 
+		barrier();
 		status = macb->rx_ring[rx_tail].ctrl;
 		if (status & MACB_BIT(RX_SOF)) {
 			if (rx_tail != macb->rx_tail)
@@ -228,14 +230,26 @@ static int macb_recv(struct eth_device *edev)
 				headlen = macb->rx_buffer_size * (macb->rx_ring_size
 						 - macb->rx_tail);
 				taillen = length - headlen;
-				memcpy((void *)NetRxPackets[0],
-				       buffer, headlen);
+				dma_sync_single_for_cpu((unsigned long)buffer,
+							headlen, DMA_FROM_DEVICE);
+				memcpy((void *)NetRxPackets[0], buffer, headlen);
+				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
+							taillen, DMA_FROM_DEVICE);
 				memcpy((void *)NetRxPackets[0] + headlen,
-				       macb->rx_buffer, taillen);
-				buffer = (void *)NetRxPackets[0];
+					macb->rx_buffer, taillen);
+				dma_sync_single_for_device((unsigned long)buffer,
+							headlen, DMA_FROM_DEVICE);
+				dma_sync_single_for_device((unsigned long)macb->rx_buffer,
+							taillen, DMA_FROM_DEVICE);
+				net_receive(edev, NetRxPackets[0], length);
+			} else {
+				dma_sync_single_for_cpu((unsigned long)buffer, length,
+							DMA_FROM_DEVICE);
+				net_receive(edev, buffer, length);
+				dma_sync_single_for_device((unsigned long)buffer, length,
+							DMA_FROM_DEVICE);
 			}
-
-			net_receive(edev, buffer, length);
+			barrier();
 			if (++rx_tail >= macb->rx_ring_size)
 				rx_tail = 0;
 			reclaim_rx_buffers(macb, rx_tail);
@@ -245,7 +259,6 @@ static int macb_recv(struct eth_device *edev)
 				rx_tail = 0;
 			}
 		}
-		barrier();
 	}
 
 	return 0;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] net: macb: dma_sync_* receive buffers
  2019-05-30 12:28     ` Ladislav Michl
@ 2019-06-04  6:49       ` Sascha Hauer
  0 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2019-06-04  6:49 UTC (permalink / raw)
  To: Ladislav Michl; +Cc: barebox

On Thu, May 30, 2019 at 02:28:15PM +0200, Ladislav Michl wrote:
> On Tue, Dec 04, 2018 at 08:58:01AM +0100, Sascha Hauer wrote:
> > Ladis,
> > 
> > On Fri, Nov 16, 2018 at 04:07:25PM +0100, Lucas Stach wrote:
> > > > @@ -237,12 +241,17 @@ static int macb_recv(struct eth_device *edev)
> > > > >  				taillen = length - headlen;
> > > > >  				memcpy((void *)NetRxPackets[0],
> > > > >  				       buffer, headlen);
> > > > > +				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
> > > > > +							taillen, DMA_FROM_DEVICE);
> > > > >  				memcpy((void *)NetRxPackets[0] + headlen,
> > > >  				       macb->rx_buffer, taillen);
> > > 
> > > This is missing a dma_sync_single_for_device() here, matching the
> > > ...for_cpu added above. Otherwise patch looks good.
> > 
> > Any input to this one or an updated patch?
> 
> This must have slipped to the very bottom of my mailbox, sorry.
> 
> We should also not running sync outside buffer, if it is wrapped.
> Updated patch bellow.

Applied, thanks

Sascha

> 
> 	ladis
> 
> --- 8< ---
> 
> From: Ladislav Michl <ladis@linux-mips.org>
> Subject: [PATCH v2] net: macb: dma_sync_* receive buffers
> 
> Receive buffers are properly synchronized only if Cadence is
> GEM. Fix it for MACB as well.
> 
> Fixes: 86dc5259e25d (net: macb: no need for coherent memory for receive buffer)
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  drivers/net/macb.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 0c0d17ee9..a0411d6e4 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -209,9 +209,11 @@ static int macb_recv(struct eth_device *edev)
>  	dev_dbg(macb->dev, "%s\n", __func__);
>  
>  	for (;;) {
> +		barrier();
>  		if (!(macb->rx_ring[rx_tail].addr & MACB_BIT(RX_USED)))
>  			return -1;
>  
> +		barrier();
>  		status = macb->rx_ring[rx_tail].ctrl;
>  		if (status & MACB_BIT(RX_SOF)) {
>  			if (rx_tail != macb->rx_tail)
> @@ -228,14 +230,26 @@ static int macb_recv(struct eth_device *edev)
>  				headlen = macb->rx_buffer_size * (macb->rx_ring_size
>  						 - macb->rx_tail);
>  				taillen = length - headlen;
> -				memcpy((void *)NetRxPackets[0],
> -				       buffer, headlen);
> +				dma_sync_single_for_cpu((unsigned long)buffer,
> +							headlen, DMA_FROM_DEVICE);
> +				memcpy((void *)NetRxPackets[0], buffer, headlen);
> +				dma_sync_single_for_cpu((unsigned long)macb->rx_buffer,
> +							taillen, DMA_FROM_DEVICE);
>  				memcpy((void *)NetRxPackets[0] + headlen,
> -				       macb->rx_buffer, taillen);
> -				buffer = (void *)NetRxPackets[0];
> +					macb->rx_buffer, taillen);
> +				dma_sync_single_for_device((unsigned long)buffer,
> +							headlen, DMA_FROM_DEVICE);
> +				dma_sync_single_for_device((unsigned long)macb->rx_buffer,
> +							taillen, DMA_FROM_DEVICE);
> +				net_receive(edev, NetRxPackets[0], length);
> +			} else {
> +				dma_sync_single_for_cpu((unsigned long)buffer, length,
> +							DMA_FROM_DEVICE);
> +				net_receive(edev, buffer, length);
> +				dma_sync_single_for_device((unsigned long)buffer, length,
> +							DMA_FROM_DEVICE);
>  			}
> -
> -			net_receive(edev, buffer, length);
> +			barrier();
>  			if (++rx_tail >= macb->rx_ring_size)
>  				rx_tail = 0;
>  			reclaim_rx_buffers(macb, rx_tail);
> @@ -245,7 +259,6 @@ static int macb_recv(struct eth_device *edev)
>  				rx_tail = 0;
>  			}
>  		}
> -		barrier();
>  	}
>  
>  	return 0;
> -- 
> 2.20.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-04  6:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15  0:36 [PATCH] net: macb: dma_sync_* receive buffers Ladislav Michl
2018-11-16 15:07 ` Lucas Stach
2018-12-04  7:58   ` Sascha Hauer
2019-05-30 12:28     ` Ladislav Michl
2019-06-04  6:49       ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox