rtl8139 pci mem freeze (more info)

Daniel Kobras daniel.kobras@student.uni-tuebingen.de
Wed Sep 29 17:31:27 1999


Hi!

On Tue, 28 Sep 1999, Donald Becker wrote:

> [[ Background: This topic is about getting the RTL8139 to work in memory
> rather than I/O mode on certain Intel chipset motherboards. ]]
> 
> Ahhh, then it might be that the outb() must instead be an outl(), e.g. 
> +	struct rtl8129_private *tp = dev->priv;
> ...
> -  	outb(rx_mode, ioaddr + RxConfig);
> +  	outl(tp->rx_config | rx_mode, ioaddr + RxConfig);
> 
> My current theory is that the something about those Intel chipset causes
> memory mode to not work when writing bytes of RTL8139 registers that have a
> larger natural size.  The driver *does* work on my PR-440FX motherboards.

That's a good theory. I liked it even more when I grabbed out my RTL
manual and checked that the places in the code where I experienced lock
ups were indeed the only ones where a register was accessed with a smaller
width than its natural size. There's only one fault with this theory. It's
wrong. Or at least, it's not the whole truth. I did the changes you
suggested and my box still hung but this time apparently at a new
location. I found the culprit at the end of rtl8129_rx[*]:

	outw(cur_rx - 16, ioaddr + RxBufPtr);

(that's registers 0x38-0x39). As 0x3a and 0x3b are read-only, I tried to
be smart and changed the outw to an outl. On the first try the chip only
produced garbage, on the second it again locked the machine. So I changed
it to an outw_f, which gave my a fine working driver as long as I
insmod'ed and ifconfig'ed manually. Using a skript where insmod was
immediatly followed by an ifconfig would hang the computer at least after
some tries, until I inserted _both_ the outl_f's back in when setting the
MAC and Multicast addresses. 

([*] Read that as "it doesn't lock anymore when I change this command".
 Finding the exact location where the machine hangs is next to impossible
 as this whole stuff is highly timing related. Add a few printk()s to a
 previously locking section and the hangs will be magically cured. That's 
 fine if you want a running machine but it's a nightmare to debug. For
 this reason, I did all my tests with the default debug level btw., i. e.
 debug=1)

With these changes applied, I haven't been able to reproduce lock ups (so
far), but then I only tested default operation and I'm not sure what might
happen if any code in an error condition path gets executed. Perhaps some
other can give this a try. (Maybe put it back into the ac patches?)

Here's the diff against 2.3.18ac8:

--[snip]--

--- rtl8139.c.orig	Wed Sep 29 22:45:15 1999
+++ rtl8139.c	Wed Sep 29 22:43:37 1999
@@ -137,7 +137,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 +186,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 +269,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 +610,14 @@
 		if ((inb(ioaddr + ChipCmd) & CmdReset) == 0)
 			break;
 
-	for (i = 0; i < 6; i++)
-		outb(dev->dev_addr[i], ioaddr + MAC0 + i);
+	outl_f(dev->dev_addr[0], ioaddr + MAC0);
+	outl(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 +686,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(dev->dev_addr[0], ioaddr + MAC0);
+	outl(dev->dev_addr[4], ioaddr + MAC0 + 4);
 
 	/* Hmmm, do these belong here? */
 	outb(0x00, ioaddr + Cfg9346);
@@ -686,8 +695,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 +846,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(dev->dev_addr[0], ioaddr + MAC0);
+	outl(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 +1126,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);
@@ -1174,8 +1186,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 {
@@ -1222,7 +1235,7 @@
 		}
 
 		cur_rx = (cur_rx + rx_size + 4 + 3) & ~3;
-		outw(cur_rx - 16, ioaddr + RxBufPtr);
+		outw_f(cur_rx - 16, ioaddr + RxBufPtr);
 	}
 	if (rtl8129_debug > 4)
 		printk(KERN_DEBUG"%s: Done rtl8129_rx(), current %4.4x BufAddr %4.4x,"
@@ -1342,6 +1355,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 +1383,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;
 }


--[snap]--

Regards,

Daniel.

 | 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.