net: macb: dma_sync_* receive buffers
PATCH 1/1 net: macb: dma_sync_* receive buffers
Receive buffers are properly synchronized only if Cadence is GEM. Fix it for MACB as well.
---
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(-)
drivers/net/macb.c +9 -1
@@ -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 ;
Signature · 7 lines
2.19.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Am Donnerstag, den 15.11.2018, 01:36 +0100 schrieb Ladislav Michl:
Quoted 40 lines
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)Quoted 1 line
Signed-off-by Ladislav Michl <[email protected]>---
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(-)drivers/net/macb.c +0
-0@@ -216,9 +216,11 @@ static int macb_recv(struct eth_device *edev) > dev_dbg(macb->dev, "%s\n", __func__);Quoted 4 lines
for (;;) {
(quoted diff) +1
-0+ barrier();if (!(macb->rx_ring[rx_tail].addr &
MACB_BIT(RX_USED))) return -1;Quoted 4 lines
(quoted diff) +1
-0+ barrier();status = macb->rx_ring[rx_tail].ctrl; if (status &
MACB_BIT(RX_SOF)) { if (rx_tail!=macb->rx_tail)drivers/net/macb.c +0
-0@@ -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) {Quoted 1 line
unsigned int headlen, taillen;
drivers/net/macb.c static int macb_recv(struct eth_device *edev) · line 247
Quoted 7 lines
drivers/net/macb.c +0
-0@@ -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
Quoted 16 lines
drivers/net/macb.c +0
-0buffer = (void *)NetRxPackets[0]; > }Quoted 7 lines
net_receive(edev, buffer, length);drivers/net/macb.c +3
-0+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);drivers/net/macb.c +0
-0@@ -252,7 +261,6 @@ static int macb_recv(struct eth_device *edev) > rx_tail = 0; > } > } > - barrier(); > }
Quoted 2 lines
Quoted 1 line
return 0;
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
Ladis,
Quoted 11 lines · Lucas Stach
Review static int macb_recv(struct eth_device *edev) · line 247
Quoted 8 lines
(quoted diff) +0
-0@@ -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
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Quoted 12 lines · Sascha Hauer
Ladis,
Quoted 9 lines · Lucas Stach
Quoted 8 lines
(quoted diff) +0
-0@@ -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);
Quoted 4 lines
Quoted 2 lines
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 <[email protected]>
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.
--- drivers/net/macb.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)
drivers/net/macb.c +20 -7
@@ -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 ;
Signature · 7 lines
2.20.1 _______________________________________________ barebox mailing list [email protected] http://lists.infradead.org/mailman/listinfo/barebox
Sascha Hauer
AppliedQuoted 16 lines · Ladislav Michl
Quoted 15 lines · Sascha Hauer
Ladis,
Quoted 11 lines · Lucas Stach
Review static int macb_recv(struct eth_device *edev) · line 247
Quoted 8 lines
(quoted diff) +0
-0@@ -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.
Quoted 6 lines
Quoted 1 line
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
Quoted 78 lines
ladis
--- 8< ---
From: Ladislav Michl <[email protected]>
Subject: [PATCH v2] net: macb: dma_sync_* receive buffersReceive 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 <[email protected]>--- drivers/net/macb.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-)drivers/net/macb.c +20
-7@@ -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 ;Signature · 1 line
2.20.1
Signature · 9 lines
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 [email protected] http://lists.infradead.org/mailman/listinfo/barebox