Re: 3c59x driver: Patch for "transmit timed out" and other things.

Giuseppe Ciaccio (ciaccio@hobbes.disi.unige.it)
Fri, 22 Mar 1996 16:00:28 +0100 (MET)


On Wed, 20 Mar 1996, Michael H. Warfield wrote:

> This patch help fix a major problem for me. Thank you very much!
>
> I had two problems with this patch, though.
>
> 1) Please do not send out patches as MIME attachments in base 64
> encoding. That made it a royal pain to save and use the patch from elm.
> If it's a text patch, please make sure it's sent out as text. base 64
> encoding just makes it a pain for those of us who don't routinely use mime.
> I use it when necessary, but text files are not necessary.

mmhhh....You're right!

> 2) The patch breaks support for multiple 3c590 cards. I have a
> router with three of these beasts and when I put in this patch it could
> only see one.
>
> I tracked the multiple card problem down to the PCI identification
> code where a new variable, "found", was introduced and at the end of the
> pci scanning loop was the line "if( !found ) break;". I don't know
> what you were trying to accomplish with that, but commenting it out,
> restored recognition for multiple 3c590 boards. If I read this code right,
> I get the impression that it would stop scanning the PCI list as soon as
> it found a card it didn't recognize. Since it only looks for 3c59x cards,
> this would include PCI video cards (which I have) or other sundry PCI
> peripherals. I may be wrong - but I don't think that code is a good idea.

Actually I detected this trouble and immediately fixed it. I sent a message
to linux-kernel with the latest patch (and with all my apologies...), so
try with the new patch. I'll append at the end of this message.

The function vortex_scan() scans PCI peripherals in a way I didn't understand
in detail. I thought of variable "pci_index" as an index for "virtual
slots", and I realized that:

1) Multiple cards of different vendor/model are "allocated" on the same
"virtual slot"

2) Cards of the same vendor and model are allocated on adjacent slots.

Variable "index" scans each "virtual slot" for known cards. Scanning is
based on a "vendor id" and a "model id", so it should'n run into any trouble
when a video card of the same vendor is used --- the model is different.
>From 2) above, it follows that when no known card is found at a given
"virtual slot", it is useless to continue the search. Hence the check on
variable "found". But "found" was initialized in the wrong place...sorry!

I'd like you to tell me:

a) the buffer capacity of your 3c590 cards (8K? 32K? Other?)
b) the release of your Linux.

Enjoy

|----------------------------------------------------------------------|
| Giuseppe Ciaccio e-mail : ciaccio@disi.unige.it |
| phone, address: finger ciaccio@disi.unige.it |
|----------------------------------------------------------------------|

--- 3c59xold.c Thu Mar 14 15:01:15 1996
+++ 3c59xnew.c Tue Mar 19 18:57:59 1996
@@ -2,6 +2,8 @@
/*
Written 1995 by Donald Becker.

+ Revised 3/19/1996 by Giuseppe Ciaccio ciaccio@disi.unige.it
+
This software may be used and distributed according to the terms
of the GNU Public License, incorporated herein by reference.

@@ -14,7 +16,7 @@
Code 930.5, Goddard Space Flight Center, Greenbelt MD 20771
*/

-static char *version = "3c59x.c:v0.13 2/13/96 becker@cesdis.gsfc.nasa.gov\n";
+static char *version = "3c59x.c:v0.13 2/13/96 becker@cesdis.gsfc.nasa.gov\nrevised: 3/19/96 ciaccio@disi.unige.it\n";

/* "Knobs" that turn on special features. */
/* Allow the use of bus master transfers instead of programmed-I/O for the
@@ -25,6 +27,18 @@
/* Put out somewhat more debugging messages. (0 - no msg, 1 minimal msgs). */
#define VORTEX_DEBUG 1

+
+/* Sometimes it is safe to wait a moment... */
+#define WAIT_LAPSE 20
+#define LONG_WAIT_LAPSE 200
+#define WAIT_TX_AVAIL 200
+#define MAX_TICKS_SO_FAR 40
+
+
+/* Max packet size is 1536, but can change in the future... */
+#define MAX_PACKET_SIZE 1536
+
+
#include <linux/module.h>

#include <linux/kernel.h>
@@ -66,7 +80,11 @@
int vortex_debug = 1;
#endif

-static int product_ids[] = {0x5900, 0x5950, 0x5951, 0x5952, 0, 0};
+/* These are for PCI only */
+static int product_ids[] = {0x5900, 0x5950, 0x5951, 0x5952, 0};
+#define HOW_MANY_PCI_PRODUCTS 4
+
+/* These are for both PCI and EISA */
static const char *product_names[] = {
"3c590 Vortex 10Mbps",
"3c595 Vortex 100baseTX",
@@ -74,7 +92,9 @@
"3c595 Vortex 100base-MII",
"EISA Vortex 3c597",
};
-#define DEMON_INDEX 5 /* Caution! Must be consistent with above! */
+
+/* Caution! Must be consistent with above! */
+#define DEMON_INDEX HOW_MANY_PCI_PRODUCTS+1

/*
Theory of Operation
@@ -123,6 +143,16 @@
The new chips support both ethernet (1.5K) and FDDI (4.5K) packet sizes!
This driver only supports ethernet packets because of the skbuff allocation
limit of 4K.
+
+In order to signal room availability in the Tx buffer, the network adapter
+is programmed to send an interrupt, which causes the flag dev->tbusy
+to be zeroed when catched (function vortex_interrupt). However, it seems
+that 3c590 boards sometimes forget to send that interrupt. As a result,
+the ongoing communication is blocked.
+The problem is solved by busy-waiting room availability each time a new
+transmission is to start and dev->tbusy is not zero
+(function vortex_start_xmit).
+
*/

#define TCOM_VENDOR_ID 0x10B7 /* 3Com's manufacturer's ID. */
@@ -301,56 +331,60 @@
int cards_found = 0;

if (pcibios_present()) {
- static int pci_index = 0;
+ static int pci_index;
for (; pci_index < 8; pci_index++) {
unsigned char pci_bus, pci_device_fn, pci_irq_line, pci_latency;
unsigned int pci_ioaddr;
unsigned short pci_command;
int index;
+ int found = 0;

- for (index = 0; product_ids[index]; index++) {
- if ( ! pcibios_find_device(TCOM_VENDOR_ID, product_ids[index],
+ for (index = 0; index < 5; index++) {
+ if ( pcibios_find_device(TCOM_VENDOR_ID, product_ids[index],
pci_index, &pci_bus,
&pci_device_fn))
- break;
- }
- if ( ! product_ids[index])
- break;
-
- pcibios_read_config_byte(pci_bus, pci_device_fn,
- PCI_INTERRUPT_LINE, &pci_irq_line);
- pcibios_read_config_dword(pci_bus, pci_device_fn,
- PCI_BASE_ADDRESS_0, &pci_ioaddr);
- /* Remove I/O space marker in bit 0. */
- pci_ioaddr &= ~3;
+ continue;

+ /* A known PCI card was found */
+ found = 1;
+
+ pcibios_read_config_byte(pci_bus, pci_device_fn,
+ PCI_INTERRUPT_LINE, &pci_irq_line);
+ pcibios_read_config_dword(pci_bus, pci_device_fn,
+ PCI_BASE_ADDRESS_0, &pci_ioaddr);
+ /* Remove I/O space marker in bit 0. */
+ pci_ioaddr &= ~3;
+
#ifdef VORTEX_BUS_MASTER
- /* Get and check the bus-master and latency values.
- Some PCI BIOSes fail to set the master-enable bit, and
- the latency timer must be set to the maximum value to avoid
- data corruption that occurs when the timer expires during
- a transfer. Yes, it's a bug. */
- pcibios_read_config_word(pci_bus, pci_device_fn,
- PCI_COMMAND, &pci_command);
- if ( ! (pci_command & PCI_COMMAND_MASTER)) {
- printk(" PCI Master Bit has not been set! Setting...\n");
- pci_command |= PCI_COMMAND_MASTER;
- pcibios_write_config_word(pci_bus, pci_device_fn,
- PCI_COMMAND, pci_command);
- }
- pcibios_read_config_byte(pci_bus, pci_device_fn,
- PCI_LATENCY_TIMER, &pci_latency);
- if (pci_latency != 255) {
- printk(" Overriding PCI latency timer (CFLT) setting of %d, new value is 255.\n", pci_latency);
- pcibios_write_config_byte(pci_bus, pci_device_fn,
- PCI_LATENCY_TIMER, 255);
- }
+ /* Get and check the bus-master and latency values.
+ Some PCI BIOSes fail to set the master-enable bit, and
+ the latency timer must be set to the maximum value to avoid
+ data corruption that occurs when the timer expires during
+ a transfer. Yes, it's a bug. */
+ pcibios_read_config_word(pci_bus, pci_device_fn,
+ PCI_COMMAND, &pci_command);
+ if ( ! (pci_command & PCI_COMMAND_MASTER)) {
+ printk(" PCI Master Bit has not been set! Setting...\n");
+ pci_command |= PCI_COMMAND_MASTER;
+ pcibios_write_config_word(pci_bus, pci_device_fn,
+ PCI_COMMAND, pci_command);
+ }
+ pcibios_read_config_byte(pci_bus, pci_device_fn,
+ PCI_LATENCY_TIMER, &pci_latency);
+ if (pci_latency != 255) {
+ printk(" Overriding PCI latency timer (CFLT) setting of %d, new value is 255.\n", pci_latency);
+ pcibios_write_config_byte(pci_bus, pci_device_fn,
+ PCI_LATENCY_TIMER, 255);
+ }
#endif /* VORTEX_BUS_MASTER */
- vortex_found_device(dev, pci_ioaddr, pci_irq_line, index,
- dev && dev->mem_start ? dev->mem_start
- : options[cards_found]);
- dev = 0;
- cards_found++;
+ vortex_found_device(dev, pci_ioaddr, pci_irq_line, index,
+ dev && dev->mem_start ? dev->mem_start
+ : options[cards_found]);
+ dev = 0;
+ cards_found++;
+ }
+ /* If no known card was found, stop scanning. */
+ if (!found) break;
}
}

@@ -530,13 +564,13 @@
}

outw(TxReset, ioaddr + EL3_CMD);
- for (i = 20; i >= 0 ; i--)
+ for (i = WAIT_LAPSE; i >= 0 ; i--)
if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
break;

outw(RxReset, ioaddr + EL3_CMD);
/* Wait a few ticks for the RxReset command to complete. */
- for (i = 20; i >= 0 ; i--)
+ for (i = WAIT_LAPSE; i >= 0 ; i--)
if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
break;

@@ -638,26 +672,28 @@
struct vortex_private *vp = (struct vortex_private *)dev->priv;
int ioaddr = dev->base_addr;

- /* Transmitter timeout, serious problems. */
if (dev->tbusy) {
- int tickssofar = jiffies - dev->trans_start;
- if (tickssofar < 40)
- return 1;
- printk("%s: transmit timed out, tx_status %2.2x status %4.4x.\n",
- dev->name, inb(ioaddr + TxStatus), inw(ioaddr + EL3_STATUS));
- vp->stats.tx_errors++;
- /* Issue TX_RESET and TX_START commands. */
- outw(TxReset, ioaddr + EL3_CMD);
- {
- int i;
- for (i = 20; i >= 0 ; i--)
- if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
- break;
- }
- outw(TxEnable, ioaddr + EL3_CMD);
- dev->trans_start = jiffies;
- dev->tbusy = 0;
- return 0;
+ /* Missing interrupt of TxAvailable ? */
+ /* Let's have a little busy-waiting for */
+ /* room available in Tx buffer */
+ int i = WAIT_TX_AVAIL;
+ while ((inw(ioaddr + TxFree) <= MAX_PACKET_SIZE) && --i);
+ if (!i) {
+ if ((jiffies - dev->trans_start) < MAX_TICKS_SO_FAR)
+ return 1;
+ printk("%s: transmit timed out, tx_status %2.2x status %4.4x.\n",
+ dev->name, inb(ioaddr + TxStatus), inw(ioaddr + EL3_STATUS));
+ vp->stats.tx_errors++;
+ /* Issue TX_RESET and TX_START commands. */
+ outw(TxReset, ioaddr + EL3_CMD);
+ for (i = 20; i >= 0 ; i--)
+ if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress) break;
+ outw(TxEnable, ioaddr + EL3_CMD);
+ dev->trans_start = jiffies;
+ dev->tbusy = 0;
+ return 0;
+ }
+ dev->tbusy = 0;
}

if (skb == NULL || skb->len <= 0) {
@@ -688,21 +724,19 @@
/* ... and the packet rounded to a doubleword. */
outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
dev_kfree_skb (skb, FREE_WRITE);
- if (inw(ioaddr + TxFree) > 1536) {
- dev->tbusy = 0;
- } else
+ if (inw(ioaddr + TxFree) > MAX_PACKET_SIZE) dev->tbusy = 0;
+ else
/* Interrupt us when the FIFO has room for max-sized packet. */
- outw(SetTxThreshold + 1536, ioaddr + EL3_CMD);
+ outw(SetTxThreshold + MAX_PACKET_SIZE, ioaddr + EL3_CMD);
}
#else
/* ... and the packet rounded to a doubleword. */
outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
dev_kfree_skb (skb, FREE_WRITE);
- if (inw(ioaddr + TxFree) > 1536) {
- dev->tbusy = 0;
- } else
+ if (inw(ioaddr + TxFree) > MAX_PACKET_SIZE) dev->tbusy = 0;
+ else
/* Interrupt us when the FIFO has room for max-sized packet. */
- outw(SetTxThreshold + 1536, ioaddr + EL3_CMD);
+ outw(SetTxThreshold + MAX_PACKET_SIZE, ioaddr + EL3_CMD);
#endif /* bus master */

dev->trans_start = jiffies;
@@ -722,7 +756,7 @@
if (tx_status & 0x30) {
int j;
outw(TxReset, ioaddr + EL3_CMD);
- for (j = 20; j >= 0 ; j--)
+ for (j = WAIT_LAPSE; j >= 0 ; j--)
if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
break;
}
@@ -802,11 +836,11 @@
#endif
if (status & (AdapterFailure | RxEarly | StatsFull)) {
/* Handle all uncommon interrupts at once. */
- if (status & RxEarly) { /* Rx early is unused. */
+ if (status & RxEarly) { /* Rx early is unused. */
vortex_rx(dev);
outw(AckIntr | RxEarly, ioaddr + EL3_CMD);
}
- if (status & StatsFull) { /* Empty statistics. */
+ if (status & StatsFull) { /* Empty statistics. */
static int DoneDidThat = 0;
if (vortex_debug > 4)
printk("%s: Updating stats.\n", dev->name);
@@ -842,6 +876,9 @@
}
}

+ if (!(status & (RxComplete | TxAvailable | DMADone | AdapterFailure | RxEarly | StatsFull)))
+ printk("%s: vortex_interrupt(): irq %d with unknown status.\n", dev->name, irq);
+
if (++i > 10) {
printk("%s: Infinite loop in interrupt, status %4.4x. "
"Disabling functions (%4.4x).\n",
@@ -904,7 +941,7 @@
netif_rx(skb);
outw(RxDiscard, ioaddr + EL3_CMD); /* Pop top Rx packet. */
/* Wait a limited time to go to next packet. */
- for (i = 200; i >= 0; i--)
+ for (i = LONG_WAIT_LAPSE; i >= 0; i--)
if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
break;
vp->stats.rx_packets++;
@@ -916,7 +953,7 @@
vp->stats.rx_dropped++;
outw(RxDiscard, ioaddr + EL3_CMD);
/* Wait a limited time to skip this packet. */
- for (i = 200; i >= 0; i--)
+ for (i = LONG_WAIT_LAPSE; i >= 0; i--)
if ( ! inw(ioaddr + EL3_STATUS) & CmdInProgress)
break;
}