[PATCH 7/8] net: mrmac: fix Rx packet loss on back-to-back frames

The Rx path keeps no record of which descriptor holds the packet handed
to the network stack. recv() always starts at rx_bd[0], falls back to
rx_bd[1] only when rx_bd[0] carries no length, and then clears the
status of both descriptors:

	priv->rx_bd[0].status = 0;
	priv->rx_bd[1].status = 0;

The status field is the only record that a frame arrived, since it
holds both the COMPLETE bit and the length. free_pkt() then re-arms the
whole ring and lets hardware reuse every buffer.

So when two frames arrive back to back, recv() returns the one in
rx_bd[0] and wipes the status of rx_bd[1] in the same call. The second
frame still sits in memory, but nothing is left to show it is there.
free_pkt() hands its buffer back to hardware and it never reaches the
network stack. Nothing reports an error.

This shows up with multiple boards on a switch. The switch sends extra
traffic to the board, so packets often arrive back to back and one of
them is dropped. Network transfers then time out.

Completion also comes from the channel interrupt status register, which
only reports that a packet arrived on the channel, not which descriptor
holds it. That signal cannot drive a ring of more than one descriptor.

Track the descriptor to consume next in rx_bd_idx and touch only that
one:

 - recv() reads rx_bd[rx_bd_idx] and takes completion from the
   per-descriptor COMPLETE bit, so the descriptor is unambiguous. It
   leaves the status alone, keeping the descriptor owned by the driver
   while the network stack reads the buffer.

 - free_pkt() clears the status, restores the buffer length, flushes
   the descriptor and extends the tail pointer to it, then steps
   rx_bd_idx. Only the descriptor whose packet the stack has finished
   with goes back to hardware.

Clearing the status after delivery rather than before it means an
unread frame in the next descriptor keeps both its data and its
COMPLETE bit, and the following recv() returns it.

isrxready() loses its only caller once completion comes from the
descriptor, so drop it.

A completed descriptor with no length, or one flagged with an error,
holds no frame for the network stack. Report an empty packet for it, so
that the caller recycles the descriptor through axi_mrmac_free_pkt().

Fixes: 258ce79cfce4 ("net: xilinx: axi_mrmac: Add MRMAC driver")
Signed-off-by: Padmarao Begari <[email protected]>
---
 drivers/net/xilinx_axi_mrmac.c | 124 +++++++++++++++------------------
 drivers/net/xilinx_axi_mrmac.h |   1 +
 2 files changed, 57 insertions(+), 68 deletions(-)

diff --git a/drivers/net/xilinx_axi_mrmac.c b/drivers/net/xilinx_axi_mrmac.c
index 45112a42758..e1531a4b548 100644
--- a/drivers/net/xilinx_axi_mrmac.c
+++ b/drivers/net/xilinx_axi_mrmac.c
@@ -202,6 +202,8 @@ static int axi_mrmac_start(struct udevice *dev)
 		bd->cntrl = PKTSIZE_ALIGN;
 	}
 
+	priv->rx_bd_idx = 0;
+
 	/* Flush the BDs so DMA core could see the updates */
 	flush_cache((phys_addr_t)priv->rx_bd, RX_BD_TOTAL_SIZE);
 
@@ -326,26 +328,6 @@ static int axi_mrmac_send(struct udevice *dev, void *ptr, int len)
 	return 0;
 }
 
-static bool isrxready(struct axi_mrmac_priv *priv)
-{
-	u32 status;
-
-	/* Read pending interrupts */
-	status = readl(&priv->mcdma_rx->status);
-
-	/* Acknowledge pending interrupts */
-	writel(status & XMCDMA_IRQ_ALL_MASK, &priv->mcdma_rx->status);
-
-	/*
-	 * If Reception done interrupt is asserted, call Rx call back function
-	 * to handle the processed BDs and then raise the according flag.
-	 */
-	if (status & (XMCDMA_IRQ_IOC_MASK | XMCDMA_IRQ_DELAY_MASK))
-		return 1;
-
-	return 0;
-}
-
 /**
  * axi_mrmac_recv - MRMAC Rx function
  * @dev:	udevice structure
@@ -354,47 +336,62 @@ static bool isrxready(struct axi_mrmac_priv *priv)
  *
  * Return:	received data length on success, negative value on errors
  *
- * This is a Rx function of MRMAC. Check if any data is received on MCDMA.
- * Copy buffer pointer to packetp and return received data length.
+ * This is a Rx function of MRMAC. Check whether the descriptor that
+ * rx_bd_idx points at has been completed by the DMA engine, and if so copy
+ * its buffer pointer to packetp and return the received data length. The
+ * descriptor stays owned by the driver until axi_mrmac_free_pkt() gives it
+ * back to hardware.
+ *
+ * A completed descriptor with no length, or one flagged with an error, holds
+ * no frame for the network stack. Report an empty packet for it, so that the
+ * caller recycles the descriptor through axi_mrmac_free_pkt().
  */
 static int axi_mrmac_recv(struct udevice *dev, int flags, uchar **packetp)
 {
 	struct axi_mrmac_priv *priv = dev_get_priv(dev);
-	u32 rx_bd_end;
+	struct mcdma_bd *bd;
+	uchar *buf;
 	u32 length;
 
+	bd = &priv->rx_bd[priv->rx_bd_idx];
+
+	/* Invalidate the descriptor to see the status written by DMA */
+	invalidate_dcache_range((phys_addr_t)bd,
+				(phys_addr_t)bd + roundup(sizeof(*bd),
+							  ARCH_DMA_MINALIGN));
+
 	/* Wait for an incoming packet */
-	if (!isrxready(priv))
+	if (!(bd->status & XMCDMA_BD_STS_COMPLETE))
 		return -EAGAIN;
 
-	/* Clear all interrupts */
-	writel(XMCDMA_IRQ_ALL_MASK, &priv->mcdma_rx->status);
-
-	/* Disable IRQ for a moment till packet is handled */
-	clrbits_le32(&priv->mcdma_rx->control, XMCDMA_IRQ_ALL_MASK);
+	/*
+	 * A completed descriptor with an error, or with no data in it, holds
+	 * no frame to pass up. Report an empty packet so that the caller
+	 * recycles the descriptor through free_pkt() and moves on.
+	 */
+	if (bd->status & XMCDMA_BD_STS_ALL_ERR) {
+		*packetp = NULL;
+		return 0;
+	}
 
-	/* Disable channel fetch */
-	clrbits_le32(&priv->mcdma_rx->control, XMCDMA_CR_RUNSTOP_MASK);
+	length = bd->status & XMCDMA_BD_STS_ACTUAL_LEN_MASK;
+	if (!length) {
+		*packetp = NULL;
+		return 0;
+	}
 
-	rx_bd_end = (ulong)priv->rx_bd + roundup(RX_BD_TOTAL_SIZE,
-						 ARCH_DMA_MINALIGN);
-	/* Invalidate Rx descriptors to see proper Rx length */
-	invalidate_dcache_range((phys_addr_t)priv->rx_bd, rx_bd_end);
+	buf = (uchar *)(ulong)(((u64)bd->buf_addr_msb << 32) | bd->buf_addr);
 
-	length = priv->rx_bd[0].status & XMCDMA_BD_STS_ACTUAL_LEN_MASK;
-	*packetp = (uchar *)(ulong)priv->rx_bd[0].buf_addr;
+	/* Invalidate the buffer before the network stack reads it */
+	invalidate_dcache_range((phys_addr_t)buf,
+				(phys_addr_t)buf + roundup(PKTSIZE_ALIGN,
+							   ARCH_DMA_MINALIGN));
 
-	if (!length) {
-		length = priv->rx_bd[1].status & XMCDMA_BD_STS_ACTUAL_LEN_MASK;
-		*packetp = (uchar *)(ulong)priv->rx_bd[1].buf_addr;
-	}
+	*packetp = buf;
 
 #ifdef DEBUG
 	print_buffer(*packetp, *packetp, 1, length, 16);
 #endif
-	/* Clear status */
-	priv->rx_bd[0].status = 0;
-	priv->rx_bd[1].status = 0;
 
 	return length;
 }
@@ -407,43 +404,34 @@ static int axi_mrmac_recv(struct udevice *dev, int flags, uchar **packetp)
  *
  * Return:	0 on success, negative value on errors
  *
- * This is Rx free packet function of MRMAC. Prepare MRMAC for reception of
- * data again. Invalidate previous data from Rx buffers and set Rx buffer
- * descriptors. Trigger reception by updating tail descriptor.
+ * This is Rx free packet function of MRMAC. The caller is done with the
+ * descriptor that axi_mrmac_recv() looked at, so give that one descriptor
+ * back to hardware by extending the tail pointer to it. The channel is
+ * never stopped, so nothing else has to be touched: no halt, no current
+ * descriptor rewrite and no re-arming of the whole ring.
  */
 static int axi_mrmac_free_pkt(struct udevice *dev, uchar *packet, int length)
 {
 	struct axi_mrmac_priv *priv = dev_get_priv(dev);
+	struct mcdma_bd *bd = &priv->rx_bd[priv->rx_bd_idx];
 
 #ifdef DEBUG
 	/* It is useful to clear buffer to be sure that it is consistent */
 	memset(priv->rx_buf, 0, RX_BUFF_TOTAL_SIZE);
 #endif
-	/* Disable all Rx interrupts before RxBD space setup */
-	clrbits_le32(&priv->mcdma_rx->control, XMCDMA_IRQ_ALL_MASK);
-
-	/* Disable channel fetch */
-	clrbits_le32(&priv->mcdma_rx->control, XMCDMA_CR_RUNSTOP_MASK);
-
-	/* Update current descriptor */
-	axi_mrmac_dma_write(&priv->rx_bd[0], &priv->mcdma_rx->current);
-
-	/* Write bd to HW */
-	flush_cache((phys_addr_t)priv->rx_bd, RX_BD_TOTAL_SIZE);
-
-	/* It is necessary to flush rx buffers because if you don't do it
-	 * then cache will contain previous packet
+	/*
+	 * Clear the status written by the DMA engine, restore the buffer
+	 * length, flush the descriptor and extend the tail pointer to it so
+	 * the engine may reuse this slot.
 	 */
-	flush_cache((phys_addr_t)priv->rx_buf, RX_BUFF_TOTAL_SIZE);
+	bd->status = 0;
+	bd->cntrl = PKTSIZE_ALIGN;
 
-	/* Enable all IRQ */
-	setbits_le32(&priv->mcdma_rx->control, XMCDMA_IRQ_ALL_MASK);
+	flush_cache((phys_addr_t)bd, roundup(sizeof(*bd), ARCH_DMA_MINALIGN));
 
-	/* Channel fetch */
-	setbits_le32(&priv->mcdma_rx->control, XMCDMA_CR_RUNSTOP_MASK);
+	axi_mrmac_dma_write(bd, &priv->mcdma_rx->tail);
 
-	/* Update tail descriptor. Now it's ready to receive data */
-	axi_mrmac_dma_write(&priv->rx_bd[1], &priv->mcdma_rx->tail);
+	priv->rx_bd_idx = (priv->rx_bd_idx + 1) % RX_DESC;
 
 	log_debug("Rx completed, framelength = %x\n", length);
 
diff --git a/drivers/net/xilinx_axi_mrmac.h b/drivers/net/xilinx_axi_mrmac.h
index baf936f6b66..2c5c83421e5 100644
--- a/drivers/net/xilinx_axi_mrmac.h
+++ b/drivers/net/xilinx_axi_mrmac.h
@@ -38,6 +38,7 @@ struct axi_mrmac_priv {
 	struct mcdma_bd *rx_bd;	/* Base of the contiguous Rx BD ring */
 	u8 *txminframe;		/* Pointer to hold min length Tx frame(60) */
 	u8 *rx_buf;		/* Driver-owned RX buffer pool (RX_DESC * PKTSIZE_ALIGN) */
+	u32 rx_bd_idx;		/* Next Rx descriptor to consume: 0 .. RX_DESC-1 */
 	u32 mrmac_rate;		/* Speed to configure(Read from DT 10G/25G..) */
 };
 
-- 
2.34.1