Re: [PATCH] net: macb: dma_sync_* receive buffers

[Ladislav Michl <ladis@linux-mips.org>]

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