[0/2][via-rhine][ANNOUNCE] 1.17rc

[0/2][via-rhine][ANNOUNCE] 1.17rc

[Roger Luethi]

Two more patches for the via-rhine driver. Please apply.

With these patches, the Rhine-II passes stress testing for the first time.
There are still a few issues, but the driver doesn't break down under load
like all previous ones did.

Roger

[1/2][via-rhine][PATCH] reset logic

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

Since Linus and Jeff raised the issue of PCI posted writes, I cleaned up
wait_for_reset() some more. Experiments show that with MMIO, a reset may
indeed take seemingly longer -- that is fixed by flushing that buffer.

Also, the driver now polls the appropriate register while waiting for the
reset to finish.


[-- Attachment #2: via-rhine-1.17rc-01_reset.diff --]
[-- Type: text/plain, Size: 1404 bytes --]

--- 06_2.5.62/drivers/net/via-rhine.c	Thu Feb 20 18:20:56 2003
+++ 07_reset/drivers/net/via-rhine.c	Sat Feb 22 01:18:26 2003
@@ -368,6 +368,8 @@ enum chip_capability_flags {
 #else
 #define RHINE_IOTYPE (PCI_USES_IO  | PCI_USES_MASTER | PCI_ADDR0)
 #endif
+/* Beware of PCI posted writes */
+#define IOSYNC	do { readb(dev->base_addr + StationAddr); } while (0)
 
 /* directly indexed by enum via_rhine_chips, above */
 static struct via_rhine_chip_info via_rhine_chip_info[] __devinitdata =
@@ -530,11 +532,12 @@ static int  via_rhine_close(struct net_d
 static void wait_for_reset(struct net_device *dev, int chip_id, char *name)
 {
 	long ioaddr = dev->base_addr;
+	int boguscnt = 20;
 
-	udelay(5);
+	IOSYNC;
 
 	if (readw(ioaddr + ChipCmd) & CmdReset) {
-		printk(KERN_INFO "%s: Reset did not complete in 5 us. "
+		printk(KERN_INFO "%s: Reset not complete yet. "
 			"Trying harder.\n", name);
 
 		/* Rhine-II needs to be forced sometimes */
@@ -543,12 +546,14 @@ static void wait_for_reset(struct net_de
 
 		/* VT86C100A may need long delay after reset (dlink) */
 		/* Seen on Rhine-II as well (rl) */
-		udelay(100);
+		while ((readw(ioaddr + ChipCmd) & CmdReset) && --boguscnt);
+			udelay(5);
+
 	}
 
 	if (debug > 1)
 		printk(KERN_INFO "%s: Reset %s.\n", name,
-			(readw(ioaddr + ChipCmd) & CmdReset) ? "failed" : "succeeded");
+			boguscnt ? "succeeded" : "failed");
 }
 
 #ifdef USE_MEM

[2/2][via-rhine][PATCH] fix races

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

This patch addresses two distinct races:

- Until now, the driver started the chip for Tx regardless of errors
  pending in the status register. Not good if an error occured while
  we were queueing packets -- the chip counter had not been reset,
  so Tx died. (We can't reliably get an interrupt for every error
  condition)

- The Rhine-II (when under load) frequently produces a Tx descriptor
  write-back race error. Failing to handle this means waiting for the
  netdev watchdog. Fixed.

  In addition, we must wait for the Tx engine to turn off on error
  conditions before we scavenge the descriptor entries. Failing to do
  so will typically lead to performance going down to about 10%: Burst,
  timeout, burst, timeout.. (again, with a Rhine-II under load).


[-- Attachment #2: via-rhine-1.17rc-02_tdwb_race.diff --]
[-- Type: text/plain, Size: 7322 bytes --]

--- 07_reset/drivers/net/via-rhine.c	Sat Feb 22 01:18:26 2003
+++ 08_tdwb_race/drivers/net/via-rhine.c	Thu Feb 27 11:45:47 2003
@@ -405,7 +405,8 @@ enum register_offsets {
 	MIICmd=0x70, MIIRegAddr=0x71, MIIData=0x72, MACRegEEcsr=0x74,
 	ConfigA=0x78, ConfigB=0x79, ConfigC=0x7A, ConfigD=0x7B,
 	RxMissed=0x7C, RxCRCErrs=0x7E, MiscCmd=0x81,
-	StickyHW=0x83, WOLcrClr=0xA4, WOLcgClr=0xA7, PwrcsrClr=0xAC,
+	StickyHW=0x83, IntrStatus2=0x84, WOLcrClr=0xA4, WOLcgClr=0xA7,
+	PwrcsrClr=0xAC,
 };
 
 /* Bits in ConfigD */
@@ -432,6 +433,8 @@ enum intr_status_bits {
 	IntrTxAborted=0x2000, IntrLinkChange=0x4000,
 	IntrRxWakeUp=0x8000,
 	IntrNormalSummary=0x0003, IntrAbnormalSummary=0xC260,
+	IntrTxDescRace=0x080000,	/* mapped from IntrStatus2 */
+	IntrTxErrSummary=0x082210,
 };
 
 /* The Rx and Tx buffer descriptors. */
@@ -529,6 +532,19 @@ static struct net_device_stats *via_rhin
 static int netdev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 static int  via_rhine_close(struct net_device *dev);
 
+static inline u32 get_intr_status(struct net_device *dev)
+{
+	long ioaddr = dev->base_addr;
+	struct netdev_private *np = dev->priv;
+	u32 intr_status;
+
+	intr_status = readw(ioaddr + IntrStatus);
+	/* On Rhine-II, Bit 3 indicates Tx descriptor write-back race. */
+	if (np->chip_id == VT6102)
+		intr_status |= readb(ioaddr + IntrStatus2) << 16;
+	return intr_status;
+}
+
 static void wait_for_reset(struct net_device *dev, int chip_id, char *name)
 {
 	long ioaddr = dev->base_addr;
@@ -1231,6 +1247,7 @@ static int via_rhine_start_tx(struct sk_
 {
 	struct netdev_private *np = dev->priv;
 	unsigned entry;
+	u32 intr_status;
 
 	/* Caution: the write order is important here, set the field
 	   with the "ownership" bits last. */
@@ -1280,8 +1297,15 @@ static int via_rhine_start_tx(struct sk_
 
 	/* Non-x86 Todo: explicitly flush cache lines here. */
 
-	/* Wake the potentially-idle transmit channel. */
-	writew(CmdTxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
+	/*
+	 * Wake the potentially-idle transmit channel unless errors are
+	 * pending (the ISR must sort them out first).
+	 */
+	intr_status = get_intr_status(dev);
+	if ((intr_status & IntrTxErrSummary) == 0) {
+		writew(CmdTxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
+	}
+	IOSYNC;
 
 	if (np->cur_tx == np->dirty_tx + TX_QUEUE_LEN)
 		netif_stop_queue(dev);
@@ -1308,38 +1332,51 @@ static void via_rhine_interrupt(int irq,
 
 	ioaddr = dev->base_addr;
 	
-	while ((intr_status = readw(ioaddr + IntrStatus))) {
+	while ((intr_status = get_intr_status(dev))) {
 		/* Acknowledge all of the current interrupt sources ASAP. */
+		if (intr_status & IntrTxDescRace)
+			writeb(0x08, ioaddr + IntrStatus2);
 		writew(intr_status & 0xffff, ioaddr + IntrStatus);
+		IOSYNC;
 
 		if (debug > 4)
-			printk(KERN_DEBUG "%s: Interrupt, status %4.4x.\n",
+			printk(KERN_DEBUG "%s: Interrupt, status %8.8x.\n",
 				   dev->name, intr_status);
 
 		if (intr_status & (IntrRxDone | IntrRxErr | IntrRxDropped |
 						   IntrRxWakeUp | IntrRxEmpty | IntrRxNoBuf))
 			via_rhine_rx(dev);
 
-		if (intr_status & (IntrTxDone | IntrTxError | IntrTxUnderrun |
-						   IntrTxAborted))
+		if (intr_status & (IntrTxErrSummary | IntrTxDone)) {
+			if (intr_status & IntrTxErrSummary) {
+				int cnt = 20;
+				/* Avoid scavenging before Tx engine turned off */
+				while ((readw(ioaddr+ChipCmd) & CmdTxOn) && --cnt)
+					udelay(5);
+				if (debug > 2 && !cnt)
+					printk(KERN_WARNING "%s: via_rhine_interrupt() "
+						   "Tx engine still on.\n",
+						   dev->name);
+			}
 			via_rhine_tx(dev);
+		}
 
 		/* Abnormal error summary/uncommon events handlers. */
 		if (intr_status & (IntrPCIErr | IntrLinkChange |
 				   IntrStatsMax | IntrTxError | IntrTxAborted |
-				   IntrTxUnderrun))
+				   IntrTxUnderrun | IntrTxDescRace))
 			via_rhine_error(dev, intr_status);
 
 		if (--boguscnt < 0) {
 			printk(KERN_WARNING "%s: Too much work at interrupt, "
-				   "status=0x%4.4x.\n",
+				   "status=%#8.8x.\n",
 				   dev->name, intr_status);
 			break;
 		}
 	}
 
 	if (debug > 3)
-		printk(KERN_DEBUG "%s: exiting interrupt, status=%4.4x.\n",
+		printk(KERN_DEBUG "%s: exiting interrupt, status=%8.8x.\n",
 			   dev->name, readw(ioaddr + IntrStatus));
 }
 
@@ -1517,7 +1554,8 @@ static void via_rhine_rx(struct net_devi
 	}
 
 	/* Pre-emptively restart Rx engine. */
-	writew(CmdRxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
+	writew(readw(dev->base_addr + ChipCmd) | CmdRxOn | CmdRxDemand,
+		   dev->base_addr + ChipCmd);
 }
 
 /* Clears the "tally counters" for CRC errors and missed frames(?).
@@ -1531,15 +1569,35 @@ static inline void clear_tally_counters(
 	readw(ioaddr + RxMissed);
 }
 
-static inline void via_rhine_restart_tx(struct net_device *dev) {
+static void via_rhine_restart_tx(struct net_device *dev) {
 	struct netdev_private *np = dev->priv;
+	long ioaddr = dev->base_addr;
 	int entry = np->dirty_tx % TX_RING_SIZE;
+	u32 intr_status;
 
-	/* We know better than the chip where it should continue */
-	writel(np->tx_ring_dma + entry * sizeof(struct tx_desc),
-		   dev->base_addr + TxRingPtr);
+	/*
+	 * If new errors occured, we need to sort them out before doing Tx.
+	 * In that case the ISR will be back here RSN anyway.
+	 */
+	intr_status = get_intr_status(dev);
+
+	if ((intr_status & IntrTxErrSummary) == 0) {
+
+		/* We know better than the chip where it should continue. */
+		writel(np->tx_ring_dma + entry * sizeof(struct tx_desc),
+			   ioaddr + TxRingPtr);
+
+		writew(CmdTxDemand | np->chip_cmd, ioaddr + ChipCmd);
+		IOSYNC;
+	}
+	else {
+		/* This should never happen */
+		if (debug > 1)
+			printk(KERN_WARNING "%s: via_rhine_restart_tx() "
+				   "Another error occured %8.8x.\n",
+				   dev->name, intr_status);
+	}
 
-	writew(CmdTxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
 }
 
 static void via_rhine_error(struct net_device *dev, int intr_status)
@@ -1569,9 +1627,8 @@ static void via_rhine_error(struct net_d
 	}
 	if (intr_status & IntrTxAborted) {
 		if (debug > 1)
-			printk(KERN_INFO "%s: Abort %4.4x, frame dropped.\n",
+			printk(KERN_INFO "%s: Abort %8.8x, frame dropped.\n",
 				   dev->name, intr_status);
-		via_rhine_restart_tx(dev);
 	}
 	if (intr_status & IntrTxUnderrun) {
 		if (np->tx_thresh < 0xE0)
@@ -1580,15 +1637,21 @@ static void via_rhine_error(struct net_d
 			printk(KERN_INFO "%s: Transmitter underrun, Tx "
 				   "threshold now %2.2x.\n",
 				   dev->name, np->tx_thresh);
-		via_rhine_restart_tx(dev);
 	}
+	if (intr_status & IntrTxDescRace) {
+		if (debug > 2)
+			printk(KERN_INFO "%s: Tx descriptor write-back race.\n",
+				   dev->name);
+	}
+	if (intr_status & ( IntrTxAborted | IntrTxUnderrun | IntrTxDescRace ))
+		via_rhine_restart_tx(dev);
+
 	if (intr_status & ~( IntrLinkChange | IntrStatsMax | IntrTxUnderrun |
- 						 IntrTxError | IntrTxAborted | IntrNormalSummary)) {
+ 						 IntrTxError | IntrTxAborted | IntrNormalSummary |
+						 IntrTxDescRace )) {
 		if (debug > 1)
-			printk(KERN_ERR "%s: Something Wicked happened! %4.4x.\n",
-			   dev->name, intr_status);
-		/* Recovery for other fault sources not known. */
-		writew(CmdTxDemand | np->chip_cmd, dev->base_addr + ChipCmd);
+			printk(KERN_ERR "%s: Something Wicked happened! %8.8x.\n",
+				   dev->name, intr_status);
 	}
 
 	spin_unlock (&np->lock);

[3/2][via-rhine][PATCH] fixing the reset fix

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 97 bytes --]

Bad semicolon in previous patch ([1/2]). Thanks to
Erik@harddisk-recovery.nl for spotting this.


[-- Attachment #2: via-rhine-1.17rc-03_reset_trivial.diff --]
[-- Type: text/plain, Size: 443 bytes --]

--- 08_tdwb_race/drivers/net/via-rhine.c	Thu Feb 27 11:45:47 2003
+++ 09_reset_trivial/drivers/net/via-rhine.c	Thu Feb 27 13:27:00 2003
@@ -562,7 +562,7 @@ static void wait_for_reset(struct net_de
 
 		/* VT86C100A may need long delay after reset (dlink) */
 		/* Seen on Rhine-II as well (rl) */
-		while ((readw(ioaddr + ChipCmd) & CmdReset) && --boguscnt);
+		while ((readw(ioaddr + ChipCmd) & CmdReset) && --boguscnt)
 			udelay(5);
 
 	}

[via-rhine][PATCH] 1.17 release

[Roger Luethi]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

Alas no rave reviews on lkml, but the private feedback I have received so
far on the recent changes has been excellent. The Rhine-II is now finally
usable with via-rhine. Time to call it 1.17. -- Please apply.

FWIW I think the four patches (including this one) leading up to 1.17 are
2.4 material, too. The drivers were identical at 1.16, and some kind souls
successfully tested 1.17 on 2.4. Given the low frequency of 2.4 releases
and the brokenness of the driver until now, it would seem like a good idea
to have it in 2.4.21.

Roger


[-- Attachment #2: via-rhine-1.17-version_log.diff --]
[-- Type: text/plain, Size: 701 bytes --]

--- 09_reset_trivial/drivers/net/via-rhine.c	Thu Feb 27 13:27:00 2003
+++ 10_version_log/drivers/net/via-rhine.c	Sat Mar  1 12:53:03 2003
@@ -108,11 +108,18 @@
 	- New reset code uses "force reset" cmd on Rhine-II
 	- Various clean ups
 
+	LK1.1.17 (Roger Luethi)
+	- Fix race in via_rhine_start_tx()
+	- On errors, wait for Tx engine to turn off before scavenging
+	- Handle Tx descriptor write-back race on Rhine-II
+	- Force flushing for PCI posted writes
+	- More reset code changes
+
 */
 
 #define DRV_NAME	"via-rhine"
-#define DRV_VERSION	"1.1.16"
-#define DRV_RELDATE	"February-15-2003"
+#define DRV_VERSION	"1.1.17"
+#define DRV_RELDATE	"March-1-2003"
 
 
 /* A few user-configurable values.