rtl8139 pci mem freeze (more info)

Daniel Kobras daniel.kobras@student.uni-tuebingen.de
Sat Oct 2 08:28:16 1999


On Thu, 30 Sep 1999, Donald Becker wrote:

> The endian conversion is confusing.  The typical big-endian PCI bus
> implementation orders the bytes so that host writes of addresses and
> integers are correct.  When the card does bus-master operations the bytes
> are ordered (reversed) so that byte streams are in the correct order, but
> addresses and integers are byte reversed.

Okay, got it finally. Thanks for explaining. 

> > > > -		outw(cur_rx - 16, ioaddr + RxBufPtr);
> > > > +		outw_f(cur_rx - 16, ioaddr + RxBufPtr);
>
> Does reading any register avoid the hang, or must it be RxBufPtr?

[Note that reverting the above change doesn't always cause a hang,
sometimes I only get tons of Rx errors, sometimes it hangs. But I have
_never_ seen any hang with the above change applied.]

Any delay will suffice, it doesn't have to be a read[bwl]. And I think
I've found the reason: The Buffer Empty flag in ChipCmd occasionally isn't
updated fast enough after writing out the new RxBufPtr, so we run an Rx
cycle without data pending. That does explain the errors but still I don't
see why this would lock my machine. 

Below is the complete patch against 2.3.18ac8. Compiling with RX_DEBUG
defined will yield some debugging output if RxBufEmpty was reset too late.
Here's some example output on my machine:

yksi:/tmp# ping abulafia
PING abulafia.sau98.de (192.168.100.43): 56 data bytes
64 bytes from 192.168.100.43: icmp_seq=0 ttl=255 time=0.4 ms
64 bytes from 192.168.100.43: icmp_seq=1 ttl=255 time=0.2 ms
eth0: Huh!? Come home early today?
64 bytes from 192.168.100.43: icmp_seq=2 ttl=255 time=0.2 ms
64 bytes from 192.168.100.43: icmp_seq=3 ttl=255 time=0.2 ms
eth0: Huh!? Come home early today?
64 bytes from 192.168.100.43: icmp_seq=4 ttl=255 time=0.2 ms
64 bytes from 192.168.100.43: icmp_seq=5 ttl=255 time=0.2 ms

The udelay() in the interrupt path of course bothers me a lot and from my
testing it looks like a much shorter delay would suffice but I don't know
of a portable way to do that. (Is there sort of an ndelay() available?)

Regards,

Daniel.

--[snip]--
 
--- rtl8139.c.orig	Sat Oct  2 14:14:35 1999
+++ rtl8139.c	Sat Oct  2 14:12:55 1999
@@ -78,6 +78,7 @@
 #include <asm/io.h>
 
 #include <linux/pci_scan.h>
+#include <linux/delay.h>
 
 /* Condensed operations for readability.
    Compatibility defines are now in drv_compat.h */
@@ -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,19 @@
 #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)
+#define iodelay()		udelay(1)
 #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
+#define iodelay()		do{}while(0)
 #endif
 
 /* The rest of these values should never change. */
@@ -262,6 +272,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 +613,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_le32(*(u32 *)(dev->dev_addr+0)), ioaddr + MAC0);
+	outl(cpu_to_le32(*(u32 *)(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 +689,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_le32(*(u32 *)(dev->dev_addr+0)), ioaddr + MAC0);
+	outl(cpu_to_le32(*(u32 *)(dev->dev_addr+4)), ioaddr + MAC0 + 4);
 
 	/* Hmmm, do these belong here? */
 	outb(0x00, ioaddr + Cfg9346);
@@ -686,8 +698,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 +849,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_le32(*(u32 *)(dev->dev_addr+0)), ioaddr + MAC0);
+	outl(cpu_to_le32(*(u32 *)(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. */
@@ -1114,7 +1129,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,14 +1147,17 @@
 	long ioaddr = dev->base_addr;
 	unsigned char *rx_ring = tp->rx_ring;
 	u16 cur_rx = tp->cur_rx;
-
+#ifdef RX_DEBUG
+	u8 cmddebug = 1;
+#endif
+	
 	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) {
+	while ((inb(ioaddr + ChipCmd) & RxBufEmpty) == 0) {
 		int ring_offset = cur_rx % RX_BUF_LEN;
 		u32 rx_status = le32_to_cpu(*(u32*)(rx_ring + ring_offset));
 		int rx_size = rx_status >> 16;
@@ -1174,8 +1192,9 @@
 			tp->cur_rx = 0;
 			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 {
@@ -1223,7 +1242,20 @@
 
 		cur_rx = (cur_rx + rx_size + 4 + 3) & ~3;
 		outw(cur_rx - 16, ioaddr + RxBufPtr);
+		/* In mem mode the chip occasionally fails to update 
+		 * RxBufEmpty in time after RxBufPtr has been changed. 
+		 * Hence the extra delay. [dk]
+		 */
+#ifdef RX_DEBUG
+		cmddebug = inb(ioaddr + ChipCmd) & RxBufEmpty;
+#else
+		iodelay();
+#endif
 	}
+#ifdef RX_DEBUG
+	if (!cmddebug)
+		printk(KERN_ERR "%s: Huh!? Come home early today?\n", dev->name);
+#endif
 	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 +1374,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 +1402,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.