Avoiding check of RxBufEmpty.

Daniel Kobras daniel.kobras@student.uni-tuebingen.de
Sun Oct 10 21:53:14 1999


On Wed, 6 Oct 1999, Donald Becker wrote:

> > Below's the patch against 2.3.18ac8 I'm currently running with. While http
> > and terminal access via slogin work fine, scp and ssh myserver somecmd
> > seem to trigger random rx errors in the driver:
> 
> Check to see if these errors are always at the end of the ring, or just
> after wrapping.

No, they were completely unrelated to wrapping but now I've figured out
what's going on. Consider two consecutive packets received, the second one
being bigger than the RX FiFo threshold (256 bytes by default). So by the
time the driver has processed the first packet, the second gets already
DMAed to main memory, RxBufAddr points to some random location inside the
second packet but - whoops! - the status at rx_ring + ring_offset isn't
set before the whole packet has been transferred.

So we have the choice between RxBufEmpty which gets updated too slow and
RxBufAddr which gets updated too early. Rats! The latter one however can
be worked around by setting RX_FIFO_TRESH and RX_DMA_BURST to 7 (no
limit). Far from being pretty but at least it does the trick. I've added
the complete patch against 2.3.18ac8 below. This is the first time I can
reliably use the new rtl8139 in mem mode. 

Donald, thanks a lot for patiently answering my questions and helping me
sort this one out. (And for writing the driver in the first place, of
course)

Regards,

Daniel.

--[snip]--

--- rtl8139.c.orig	Mon Oct 11 03:19:43 1999
+++ rtl8139.c	Mon Oct 11 03:27:09 1999
@@ -42,6 +42,7 @@
 /* Size of the in-memory receive ring. */
 #define RX_BUF_LEN_IDX	3			/* 0==8K, 1==16K, 2==32K, 3==64K */
 #define RX_BUF_LEN (8192 << RX_BUF_LEN_IDX)
+#define RX_BUF_MASK (RX_BUF_LEN-1)
 /* Size of the Tx bounce buffers -- must be at least (dev->mtu+14+4). */
 #define TX_BUF_SIZE	1536
 
@@ -49,9 +50,10 @@
    Threshold is bytes transferred to chip before transmission starts. */
 #define TX_FIFO_THRESH 256	/* In bytes, rounded down to 32 byte units. */
 
-/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6==1024. */
-#define RX_FIFO_THRESH	4		/* Rx buffer level before first PCI xfer.  */
-#define RX_DMA_BURST	4		/* Maximum PCI burst, '4' is 256 bytes */
+/* The following settings are log_2(bytes)-4:  0 == 16 bytes .. 6==1024,
+ * 7 means no limit - don't limit with new rx code! [dk] */
+#define RX_FIFO_THRESH	7		/* Rx buffer level before first PCI xfer.  */
+#define RX_DMA_BURST	7		/* Maximum PCI burst */
 #define TX_DMA_BURST	4		/* Calculate as 16<<val. */
 
 /* Operational parameters that usually are not changed. */
@@ -134,11 +136,6 @@
 */
 
 
-/*
- *	For now while we investigate a few things.
- */
-#define USE_IO_OPS
-
 static void *rtl8139_probe1(struct pci_dev *pdev, void *init_dev,
 							long ioaddr, int irq, int chp_idx, int card_idx);
 enum {HAS_MII_XCVR=0x01, HAS_CHIP_XCVR=0x02, HAS_LNK_CHNG=0x04};
@@ -186,10 +183,17 @@
 #define outb writeb
 #define outw writew
 #define outl writel
+#define outb_f(val,port)        do{writeb((val),(port));readb((port));}while(0)
+#define outw_f(val,port)        do{writew((val),(port));readw((port));}while(0)
+#define outl_f(val,port)        do{writel((val),(port));readl((port));}while(0)
 #undef request_region
 #undef release_region
 #define request_region request_mem_region
 #define release_region release_mem_region
+#else
+#define outb_f  outb
+#define outw_f  outw
+#define outl_f  outl
 #endif
 
 /* The rest of these values should never change. */
@@ -262,6 +266,7 @@
 	struct net_device_stats stats;
 	struct timer_list timer;	/* Media selection timer. */
 	unsigned char *rx_ring;
+	unsigned int rx_config;
 	unsigned int cur_rx;		/* Index into the Rx buffer of next Rx pkt. */
 	unsigned int cur_tx, dirty_tx, tx_flag;
 	unsigned long tx_full;				/* The Tx queue is full. */
@@ -602,13 +607,14 @@
 		if ((inb(ioaddr + ChipCmd) & CmdReset) == 0)
 			break;
 
-	for (i = 0; i < 6; i++)
-		outb(dev->dev_addr[i], ioaddr + MAC0 + i);
+	outl_f(cpu_to_le32p(dev->dev_addr+0), ioaddr + MAC0);
+	outl(cpu_to_le32p(dev->dev_addr+4), ioaddr + MAC0 + 4);
 
 	/* Must enable Tx/Rx before setting transfer thresholds! */
 	outb(CmdRxEnb | CmdTxEnb, ioaddr + ChipCmd);
-	outl((RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) | (RX_DMA_BURST<<8),
-		 ioaddr + RxConfig);
+	tp->rx_config=(RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) 
+		| (RX_DMA_BURST<<8);
+	outl(tp->rx_config, ioaddr + RxConfig);
 	outl((TX_DMA_BURST<<8)|0x03000000, ioaddr + TxConfig);
 
 	if (tp->phys[0] >= 0  ||  (tp->drv_flags & HAS_MII_XCVR)) {
@@ -677,8 +683,8 @@
 		if ((inb(ioaddr + ChipCmd) & CmdReset) == 0)
 			break;
 	/* Restore our idea of the MAC address. */ 
-	for (i = 0; i < 6; i++)
-		outb(dev->dev_addr[i], ioaddr + MAC0 + i);
+	outl_f(cpu_to_le32p(dev->dev_addr+0), ioaddr + MAC0);
+	outl(cpu_to_le32p(dev->dev_addr+4), ioaddr + MAC0 + 4);
 
 	/* Hmmm, do these belong here? */
 	outb(0x00, ioaddr + Cfg9346);
@@ -686,8 +692,9 @@
 
 	/* Must enable Tx/Rx before setting transfer thresholds! */
 	outb(CmdRxEnb | CmdTxEnb, ioaddr + ChipCmd);
-	outl((RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) | (RX_DMA_BURST<<8),
-		 ioaddr + RxConfig);
+	tp->rx_config=(RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) 
+		| (RX_DMA_BURST<<8);
+	outl(tp->rx_config, ioaddr + RxConfig);
 	/* Check this value. |0x03000000 ?? */
 	outl((TX_DMA_BURST<<8), ioaddr + TxConfig);
 
@@ -836,15 +843,17 @@
 	for (i = 1000; i > 0; i--)
 		if ((inb(ioaddr + ChipCmd) & CmdReset) == 0)
 			break;
-	for (i = 0; i < 6; i++)
-		outb(dev->dev_addr[i], ioaddr + MAC0 + i);
+
+	outl_f(cpu_to_le32p(dev->dev_addr+0), ioaddr + MAC0);
+	outl(cpu_to_le32p(dev->dev_addr+4), ioaddr + MAC0 + 4);
 
 	outb(0x00, ioaddr + Cfg9346);
 	tp->cur_rx = 0;
 	/* Must enable Tx/Rx before setting transfer thresholds! */
 	outb(CmdRxEnb | CmdTxEnb, ioaddr + ChipCmd);
-	outl((RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) | (RX_DMA_BURST<<8),
-		 ioaddr + RxConfig);
+	tp->rx_config=(RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) 
+		| (RX_DMA_BURST<<8);
+	outl(tp->rx_config, ioaddr + RxConfig);
 	outl((TX_DMA_BURST<<8), ioaddr + TxConfig);
 	set_rx_mode(dev);
 	{							/* Save the unsent Tx packets. */
@@ -1091,7 +1100,7 @@
 			if (status & (RxUnderrun|RxFIFOOver)) tp->stats.rx_fifo_errors++;
 			if (status & RxOverflow) {
 				tp->stats.rx_over_errors++;
-				tp->cur_rx = inw(ioaddr + RxBufAddr) % RX_BUF_LEN;
+				tp->cur_rx = inw(ioaddr + RxBufAddr) & RX_BUF_MASK;
 				outw(tp->cur_rx - 16, ioaddr + RxBufPtr);
 			}
 			if (status & PCIErr) {
@@ -1114,7 +1123,7 @@
 
 	if (rtl8129_debug > 3)
 		printk(KERN_DEBUG"%s: exiting interrupt, intr_status=%#4.4x.\n",
-			   dev->name, inl(ioaddr + IntrStatus));
+			   dev->name, inw(ioaddr + IntrStatus));
 
 #if defined(__i386__)
 	clear_bit(0, (void*)&dev->interrupt);
@@ -1132,16 +1141,16 @@
 	long ioaddr = dev->base_addr;
 	unsigned char *rx_ring = tp->rx_ring;
 	u16 cur_rx = tp->cur_rx;
+	u16 ring_offset = cur_rx & RX_BUF_MASK;
 
-	if (rtl8129_debug > 4)
+	if (rtl8129_debug > 4) 
 		printk(KERN_DEBUG"%s: In rtl8129_rx(), current %4.4x BufAddr %4.4x,"
 			   " free to %4.4x, Cmd %2.2x.\n",
 			   dev->name, cur_rx, inw(ioaddr + RxBufAddr),
 			   inw(ioaddr + RxBufPtr), inb(ioaddr + ChipCmd));
 
-	while ((inb(ioaddr + ChipCmd) & 1) == 0) {
-		int ring_offset = cur_rx % RX_BUF_LEN;
-		u32 rx_status = le32_to_cpu(*(u32*)(rx_ring + ring_offset));
+	while (inw(ioaddr + RxBufAddr)!=ring_offset) {
+		u32 rx_status = le32_to_cpup(rx_ring + ring_offset);
 		int rx_size = rx_status >> 16;
 
 		if (rtl8129_debug > 4) {
@@ -1152,7 +1161,7 @@
 			printk(KERN_DEBUG"%s: Frame contents ", dev->name);
 			for (i = 0; i < 70; i++)
 				printk(" %2.2x", rx_ring[ring_offset + i]);
-			printk(".\n");
+			printk(".\n"); 
 		}
 		if (rx_status & RxTooLong) {
 			if (rtl8129_debug > 0)
@@ -1162,7 +1171,7 @@
 			/* A.C.: The chip hangs here. */
 		} else if (rx_status &
 				   (RxBadSymbol|RxRunt|RxTooLong|RxCRCErr|RxBadAlign)) {
-			if (rtl8129_debug > 1)
+			if (rtl8129_debug > 1) 
 				printk(KERN_DEBUG"%s: Ethernet frame had errors,"
 					   " status %4.4x.\n", dev->name, rx_status);
 			tp->stats.rx_errors++;
@@ -1171,11 +1180,13 @@
 			if (rx_status & (RxRunt|RxTooLong)) tp->stats.rx_length_errors++;
 			if (rx_status & RxCRCErr) tp->stats.rx_crc_errors++;
 			/* Reset the receiver, based on RealTek recommendation. (Bug?) */
-			tp->cur_rx = 0;
+			tp->cur_rx = cur_rx = 0;
+			rx_size = -4;
 			outb(CmdTxEnb, ioaddr + ChipCmd);
 			outb(CmdRxEnb | CmdTxEnb, ioaddr + ChipCmd);
-			outl((RX_FIFO_THRESH << 13) | (RX_BUF_LEN_IDX << 11) |
-				 (RX_DMA_BURST<<8), ioaddr + RxConfig);
+			tp->rx_config=(RX_FIFO_THRESH << 13) | 
+				(RX_BUF_LEN_IDX << 11) | (RX_DMA_BURST<<8);
+			outl(tp->rx_config, ioaddr + RxConfig);
 			/* A.C.: Reset the multicast list. */
 			set_rx_mode(dev);
 		} else {
@@ -1222,9 +1233,10 @@
 		}
 
 		cur_rx = (cur_rx + rx_size + 4 + 3) & ~3;
+		ring_offset = cur_rx & RX_BUF_MASK;
 		outw(cur_rx - 16, ioaddr + RxBufPtr);
 	}
-	if (rtl8129_debug > 4)
+	if (rtl8129_debug > 4) 
 		printk(KERN_DEBUG"%s: Done rtl8129_rx(), current %4.4x BufAddr %4.4x,"
 			   " free to %4.4x, Cmd %2.2x.\n",
 			   dev->name, cur_rx, inw(ioaddr + RxBufAddr),
@@ -1342,6 +1354,7 @@
 static void set_rx_mode(struct net_device *dev)
 {
 	long ioaddr = dev->base_addr;
+	struct rtl8129_private *tp=dev->priv;
 	u32 mc_filter[2];		 /* Multicast hash filter */
 	int i, rx_mode;
 
@@ -1369,8 +1382,11 @@
 			set_bit(ether_crc(ETH_ALEN, mclist->dmi_addr) >> 26, mc_filter);
 	}
 	/* We can safely update without stopping the chip. */
-	outb(rx_mode, ioaddr + RxConfig);
-	outl(mc_filter[0], ioaddr + MAR0 + 0);
+	if ((tp->rx_config & 0xff) ^ rx_mode) {
+		tp->rx_config |= rx_mode;
+		outl(tp->rx_config, ioaddr + RxConfig);
+	}
+	outl_f(mc_filter[0], ioaddr + MAR0 + 0);
 	outl(mc_filter[1], ioaddr + MAR0 + 4);
 	return;
 }


 | To unsubscribe, send mail to Majordomo@cesdis.gsfc.nasa.gov, and within the
 |  body of the mail, include only the text:
 |   unsubscribe this-list-name youraddress@wherever.org
 | You will be unsubscribed as speedily as possible.