Avoiding check of RxBufEmpty.

Daniel Kobras daniel.kobras@student.uni-tuebingen.de
Tue Oct 5 18:23:38 1999


Hi!

On Sat, 2 Oct 1999, Donald Becker wrote:

> It might be possible that we can compare RxBufPtr with RxBufAddr, and
> avoid any interaction with the ChipCmd register.  I'll check on that the
> next time I have a rtl8139 card in the test machine. 

I gave it a try and was partially successful. I changed rtl8129_rx to
compare RxBufPtr and RxBufAddr to determine if the Rx buffer is empty and
it works most of the time. But my implementation can't be 100% correct as
from time to time the driver reports rx errors. In case of an error,
rx_size always equals 0xfff0 but that's the only thing they have in
common. Unfortunately I fail to see the bug. But then I also fail to fully
understand the code, especially as far as the '- 16' in

outw(cur_rx - 16, ioaddr + RxBufPtr);
[taken from rtl8129_rx]

is concerned. I see RX_BUF_LEN+16 bytes kmalloc()ed for the rx ring but
apart from debugging the last 16 bytes should never be touched, no? Is
this somehow related?

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:

belbo@yksi:~> /sbin/ifconfig eth0
eth0      Link encap:Ethernet  HWaddr 00:00:E8:58:33:23
          inet addr:192.168.100.45  Bcast:192.168.100.255 Mask:255.255.255.0
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:6728 errors:8 dropped:0 overruns:4 frame:7
          TX packets:6106 errors:0 dropped:0 overruns:0 carrier:0
          Collisions:0
          Interrupt:17 Base address:0xff80

Don, can you give me a hint what I might be doing wrong?

Regards,

Daniel.

--[snip]--

--- rtl8139.c.orig	Tue Oct  5 22:19:50 1999
+++ rtl8139.c	Tue Oct  5 22:23:12 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
 
@@ -137,7 +138,7 @@
 /*
  *	For now while we investigate a few things.
  */
-#define USE_IO_OPS
+/* #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);
@@ -186,10 +187,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 +270,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 +611,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 +687,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 +696,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 +847,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 +1104,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 +1127,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 +1145,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)
 		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) {
@@ -1171,11 +1184,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,8 +1237,12 @@
 		}
 
 		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 > 1 && !(inw(ioaddr + ChipCmd) & RxBufEmpty))
+		printk(KERN_DEBUG "%s: Ouch. Rx ended while still busy!\n");
+
 	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",
@@ -1342,6 +1361,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 +1389,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.