Orinoco EIO & SMP fix

Jean Tourrilhes jt at bougret.hpl.hp.com
Wed Dec 19 10:14:17 EST 2001


On Tue, Dec 18, 2001 at 03:26:51PM +1100, David Gibson wrote:
>
> > 	Anyway, if we didn't get the event, there is trouble
> > somewhere. I did extensive testing last week on my SMP box and I've
> > never seen a single valid case where we needed a fake event (extensive
> > as cross UDP streams).
> 
> Ok, that's good to know.  Did you test old firmware versions - maybe
> the problem only showed up on the really early cards.

	I had to fix the cor_reset, and then everything work fine with
firmware 1.16. I never see a NULL event.
	My belief is that Andreas, when writing the initial driver,
got seriously confused with the ALLOC event and never seeing any TX
event and made this crude workaround.

> I've just pushed a new version which reorganises the IRQ handler in a
> way which should fix the problem, although I've done it a little
> differently from your patch:  I only do a netif_wake_queue() on ALLOC
> events, not on TX or TXEXC, on the basis that we don't get them now,
> and if we add the bits to get them later we should still get the ALLOC
> events.

	Yes, I like the way you have done it. vvery sensible.

> In fact as I mentioned before it's not clear that the buffer
> is safe for re-use between the TX and ALLOC anyway, so we should wait
> for the ALLOC.

	That's the reverse, the ALLOC event arrives before the TX
event (see my log).

> At the moment I seem to be getting a Tx timeout as
> soon as the driver loads, but then it resets and works.  I'm not sure
> what the problem here is.

	Probably the COR reset. It was optional for a reason.

	Ok, I did a patch that fix all the remaining problems with the
driver and also fix a few little details based on my latest
experiments. Please see attached.
	I've been doing a bit of testing, and it seems solid. Nice
throughput on my SMP box (netperf reliably says 5Mb/s), and resist
cross UDP stress test.
	Wow, I'm glad we are putting this nasty bug behind us, with
all the other cleanups, I feel that the next release should be real
solid. The only thing left is checking the RSSI formula.

	Have fun...

	Jean
-------------- next part --------------
diff -u -p -r linux/drivers/net/wireless-8c/orinoco.c linux/drivers/net/wireless/orinoco.c
--- linux/drivers/net/wireless-8c/orinoco.c	Tue Dec 18 11:43:07 2001
+++ linux/drivers/net/wireless/orinoco.c	Tue Dec 18 14:54:34 2001
@@ -215,9 +215,11 @@
  *	o Some new PCI IDs for PLX cards.
  *	o Removed broken attempt to do ALLMULTI reception.  Just use
  *	  promiscuous mode instead
- *	o Preliminary work for list-AP
+ *	o Preliminary work for list-AP (Jean II)
  *	o Airport updates from (BenH)
  *	o Eliminated racy hw_ready stuff
+ *	o Fix Tx done + fake event in interrupt handler (Jean II + David)
+ *	o Fix set/get rate (Jean II)
  *
  * TODO - Jean II
  *	o inline functions (lots of candidate, need to reorder code)
@@ -291,7 +293,7 @@
 #define SIOCIWFIRSTPRIV		SIOCDEVPRIVATE
 #endif /* SIOCIWFIRSTPRIV */
 
-static char version[] __initdata = "orinoco.c 0.08b (David Gibson <hermes at gibson.dropbear.id.au> and others)";
+static char version[] __initdata = "orinoco.c 0.08c (David Gibson <hermes at gibson.dropbear.id.au> and others)";
 MODULE_AUTHOR("David Gibson <hermes at gibson.dropbear.id.au>");
 MODULE_DESCRIPTION("Driver for Lucent Orinoco, Prism II based and similar wireless cards");
 MODULE_LICENSE("Dual MPL/GPL");
@@ -319,7 +321,8 @@ MODULE_PARM(orinoco_debug, "i");
 #define SMALL_KEY_SIZE		5
 #define MAX_FRAME_SIZE		2304
 
-#define DUMMY_FID		0xFFFF
+#define DUMMY_FID		0xDEAD
+/* Don't use 0x0000 or 0xFFFF, that usually means hardware removed */
 
 const long channel_frequency[] = {
 	2412, 2417, 2422, 2427, 2432, 2437, 2442,
@@ -1101,8 +1104,14 @@ void orinoco_interrupt(int irq, void * d
 	evstat = hermes_read_regn(hw, EVSTAT);
 	events = evstat & hw->inten;
 	
-	if (! events) /* Sometimes the card generates Tx interrupts without setting EVSTAT */
+	/* Sometimes the card generates interrupts without setting EVSTAT.
+	 * This mostly happens after we process many Rx packet on the same
+	 * irq, but in rare case it may be a Tx event. - Jean II */
+	if ((! events) && (netif_queue_stopped(dev))) {
+		DEBUG(1, "%s: orinoco_interrupt : NULL event !!!\n",
+		      priv->ndev.name);
 		__orinoco_ev_alloc(priv, hw);
+	}
 
 	if (jiffies != last_irq_jiffy)
 		loops_this_jiffy = 0;
@@ -1125,6 +1134,9 @@ much! Shutting down.\n",
 			break;
 		}
 
+		DEBUG(4, "__orinoco_interrupt(): count=%d EVSTAT=0x%04x inten=0x%04x\n",
+		      count, evstat, hw->inten);
+
 		if (events & HERMES_EV_TICK)
 			__orinoco_ev_tick(priv, hw);
 		if (events & HERMES_EV_WTERR)
@@ -1401,34 +1413,54 @@ static void __orinoco_ev_txexc(struct or
 {
 	struct net_device *dev = &priv->ndev;
 	struct net_device_stats *stats = &priv->stats;
+#if 0
 	u16 fid = hermes_read_regn(hw, TXCOMPLFID);
+#endif
 
+#if 0
+	/* The FID will *always* be the "wrong" FID because the firmware
+	 * remap the allocated FID to its own Tx FID list - Jean II */
 	if (fid != priv->txfid) {
-		printk(KERN_ERR "%s: Tx exception on unexpected fid (%04X)\n",
+		printk(KERN_DEBUG "%s: Tx exception on unexpected fid (%04X)\n",
 		       dev->name, fid);
 		return;
 	}
+#endif
 
 	printk(KERN_WARNING "%s: Tx error!\n", dev->name);
 	stats->tx_errors++;
 
+	/* We don't wake up the Tx queue because we already did it long ago
+	 * when we received the alloc event.
+	 * Also, decrease tx_packet to compensate increase in ev_alloc...
+	 * Jean II */
+	stats->tx_packets--;
+
 	hermes_write_regn(hw, TXCOMPLFID, DUMMY_FID);
 }
 
 static void __orinoco_ev_tx(struct orinoco_private *priv, hermes_t *hw)
 {
-	struct net_device_stats *stats = &priv->stats;
+#if 0
 	u16 fid = hermes_read_regn(hw, TXCOMPLFID);
+#endif
 
 	DEBUG(3, "%s: Transmit completed\n", priv->ndev.name);
 
+	/* Note : in normal operation, we don't enable this event
+	 * to minimise the number of irqs generated, and we process
+	 * the whole Tx Done stuff in __orinoco_ev_alloc(), so don't put
+	 * any code in here - Jean II */
+
+#if 0
+	/* The FID will *always* be the "wrong" FID because the firmware
+	 * remap the allocated FID to its own Tx FID list - Jean II */
 	if (fid != priv->txfid) {
-		printk(KERN_ERR "%s: Tx completion on unexpected fid (%04X)\n",
+		printk(KERN_DEBUG "%s: Tx completion on unexpected fid (%04X)\n",
 		       priv->ndev.name, fid);
 		return;
 	}
-
-	stats->tx_packets++;
+#endif
 
 	hermes_write_regn(hw, TXCOMPLFID, DUMMY_FID);
 }
@@ -1436,18 +1468,31 @@ static void __orinoco_ev_tx(struct orino
 static void __orinoco_ev_alloc(struct orinoco_private *priv, hermes_t *hw)
 {
 	struct net_device *dev = &priv->ndev;
+	struct net_device_stats *stats = &priv->stats;
 	u16 fid = hermes_read_regn(hw, ALLOCFID);
 
 	DEBUG(3, "%s: Allocation complete FID=0x%04x\n", priv->ndev.name, fid);
 
-	/* For some reason we don't seem to get transmit completed events properly */
+	/* In orinoco_interrupt() we will call this function when we have
+	 * a NULL event. But, the alloc command may not have completed, and
+	 * this test check that.
+	 * In fact, 90% of the time it's not completed, so don't make
+	 * too much noise about it... - Jean II */
 	if (fid != priv->txfid) {
-		printk(KERN_WARNING "%s: Allocate event on unexpected fid (%04X)\n",
+		DEBUG(1, "%s: Allocate event on unexpected fid (%04X)\n",
 		      dev->name, fid);
 		return;
 	}
 
+	/* Note : this event will always get called prior to a
+	 * successful or unsuccessful Tx request, because we always
+	 * ask the card to reclaim the buffer - Jean II */
+
+	stats->tx_packets++;
+
 	netif_wake_queue(dev);
+	/* The firmware doesn't clear the FID register, so we do it ourselves
+	 * otherwise the test above is meaningless - Jean II */
 	hermes_write_regn(hw, ALLOCFID, DUMMY_FID);
 }
 
@@ -1889,6 +1934,11 @@ orinoco_xmit(struct sk_buff *skb, struct
 	memcpy(hdr.p80211.addr2, eh->h_source, ETH_ALEN);
 	hdr.p80211.frame_ctl = IEEE802_11_FTYPE_DATA;
 
+	/* Enable TX_EX event to properly detect transmit errors.
+	 * Do *NOT* enable TX_OK to minimise number of interrupts in
+	 * normal case - Jean II */
+	hdr.desc.tx_control = HERMES_TXCTRL_TX_EX;
+
 	/* Encapsulate Ethernet-II frames */
 	if (ntohs(eh->h_proto) > 1500) { /* Ethernet-II frame */
 		data_len = len;
@@ -2533,9 +2583,16 @@ static int orinoco_ioctl_setrate(struct 
 	int bitrate; /* 100s of kilobits */
 	int i;
 	
-	bitrate = rrq->value / 100000;
-	if (rrq->value % 100000)
-		return -EINVAL;
+	/* As the user space doesn't know our highest rate, it uses
+	 * -1 to ask us to set the highest rate.
+	 * Test it using "iwconfig ethX rate auto" - Jean II */
+	if(rrq->value == -1)
+		bitrate = 110;
+	else {
+		bitrate = rrq->value / 100000;
+		if (rrq->value % 100000)
+			return -EINVAL;
+	}
 
 	if ( (bitrate != 10) && (bitrate != 20) &&
 	     (bitrate != 55) && (bitrate != 110) )
@@ -2574,8 +2631,15 @@ static int orinoco_ioctl_getrate(struct 
 	if ( (ratemode < 0) || (ratemode > BITRATE_TABLE_SIZE) )
 		BUG();
 
+	/* The firmware will tell us the current rate, but won't tell
+	 * us if it's automatic or not, so we need to remember what we
+	 * set - Jean II */
+	rrq->fixed = ! bitrate_table[ratemode].automatic;
+	rrq->value = bitrate_table[ratemode].bitrate * 100000;
+	rrq->disabled = 0;
+
 	if (netif_running(dev)) {
-		/* If the device is actually operation we try to find more,
+		/* If the device is actually in operation we try to find more,
 		   about the current mode */
 		err = hermes_read_wordrec(hw, USER_BAP,
 					  HERMES_RID_CURRENTTXRATE, &val);
@@ -2584,14 +2648,15 @@ static int orinoco_ioctl_getrate(struct 
 		
 		switch (priv->firmware_type) {
 		case FIRMWARE_TYPE_AGERE: /* Lucent style rate */
-			for (i = 0; i < BITRATE_TABLE_SIZE; i++)
-				if (bitrate_table[i].agere_txratectrl == val) {
-					ratemode = i;
-					break;
-				}
-			if (i >= BITRATE_TABLE_SIZE)
-				printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
-				       dev->name, val);
+			/* Note : in Lucent firmware, the return value of 
+			 * HERMES_RID_CURRENTTXRATE is the bitrate in Mb/s,
+			 * and therefore is totally different from the
+			 * encoding of HERMES_RID_CNFTXRATECONTROL.
+			 * Don't forget that 6Mb/s is really 5.5Mb/s */
+			if(val == 6)
+				rrq->value = 5500000;
+			else
+				rrq->value = val * 1000000;
 			break;
 		case FIRMWARE_TYPE_INTERSIL: /* Intersil style rate */
 		case FIRMWARE_TYPE_SYMBOL: /* Symbol style rate */
@@ -2603,15 +2668,12 @@ static int orinoco_ioctl_getrate(struct 
 			if (i >= BITRATE_TABLE_SIZE)
 				printk(KERN_INFO "%s: Unable to determine current bitrate (0x%04hx)\n",
 				       dev->name, val);
+			rrq->value = bitrate_table[ratemode].bitrate * 100000;
 			break;
 		default:
 			BUG();
 		}
 	}
-
-	rrq->value = bitrate_table[ratemode].bitrate * 100000;
-	rrq->fixed = ! bitrate_table[ratemode].automatic;
-	rrq->disabled = 0;
 
  out:
 	orinoco_unlock(priv);
diff -u -p -r linux/drivers/net/wireless-8c/orinoco_cs.c linux/drivers/net/wireless/orinoco_cs.c
--- linux/drivers/net/wireless-8c/orinoco_cs.c	Tue Dec 18 11:43:08 2001
+++ linux/drivers/net/wireless/orinoco_cs.c	Tue Dec 18 14:50:54 2001
@@ -588,8 +588,10 @@ orinoco_cs_config(dev_link_t * link)
 	ndev->irq = link->irq.AssignedIRQ;
 
 	/* Now do a PCMCIA soft reset on the card, to make sure its in
-	   a sane state */
-	orinoco_cs_cor_reset(priv);
+	   a sane state. */
+	/* Optional because it really mess up old Lucent firmwares - Jean II */
+	if(reset_cor)
+		orinoco_cs_cor_reset(priv);
 
 	/* register_netdev will give us an ethX name */
 	ndev->name[0] = '\0';


More information about the wireless mailing list