[2315] in linux-net channel archive

home help back first fref pref prev next nref lref last post

3c505 driver

daemon@ATHENA.MIT.EDU (Philip Blundell)
Sun Mar 31 11:41:38 1996

Date: 	Sun, 31 Mar 1996 16:05:15 +0100 (BST)
From: Philip Blundell <phil@tazenda.demon.co.uk>
To: Jula Laiho <jlaiho@ichaos.nullnet.fi>
cc: linuxnet <linux-net@vger.rutgers.edu>

I have made some more changes to the 3c505 driver to make it more
reliable.  The patch is attached.

The current driver has some quite severe re-entrancy problems, because
large chunks of it run with interrupts enabled and no protection.  In
particular, if send_pcb() is running (eg to transmit a packet) and a frame
comes in, the interrupt code will call down to send_pcb() again to restart
the receiver, which brings the driver to a sticky end.  I have rewritten
send_pcb() to be less messy, and added code to defer restarting the
receiver until after the current PCB send (and data download, if one is
needed) has completed.

On my machine, this at least stops the driver falling over completely
under load, which makes it usable though I do still see timeouts fairly
often and throughput is fairly unimpressive.

I have also removed all the calls to CHECK_NULL(), as they didn't seem to
be contributing a great deal, and added an additional call to
autoirq_report() so that if the driver fails to initialise, it releases
all the free IRQs so you can try again.

Phil
--
.signature: Permission denied

--- linux/drivers/net/3c505.c.81	Sat Mar  2 13:42:14 1996
+++ linux/drivers/net/3c505.c	Sun Mar 31 15:47:41 1996
@@ -30,6 +30,7 @@
  *                      Terry Murphy, of 3Com Network Adapter Division
  *		Linux 1.3.0 changes by
  *			Alan Cox <Alan.Cox@linux.org>
+ *              More debugging by Philip Blundell <pjb27@cam.ac.uk>
  *
  */

@@ -61,10 +62,6 @@

 static const char * filename = __FILE__;

-static const char * null_msg = "*** NULL at %s:%s (line %d) ***\n";
-#define CHECK_NULL(p) \
-	if (!p) printk(null_msg, filename,__FUNCTION__,__LINE__)
-
 static const char * timeout_msg = "*** timeout at %s:%s (line %d) ***\n";
 #define TIMEOUT_MSG(lineno) \
 	printk(timeout_msg, filename,__FUNCTION__,(lineno))
@@ -230,34 +227,6 @@
 	sti();
 }

-#define WAIT_HCRE(addr,toval) wait_hcre((addr),(toval),__LINE__)
-static inline int
-wait_hcre (unsigned int base_addr, int toval, int lineno)
-{
-	int timeout = jiffies + toval;
-	while (((inb_status(base_addr)&HCRE)==0) && (jiffies <= timeout))
-		;
-	if (jiffies >= timeout) {
-		TIMEOUT_MSG(lineno);
-		return FALSE;
-	}
-	return TRUE;
-}
-
-static inline int
-wait_fast_hcre (unsigned int base_addr, int toval, int lineno)
-{
-	int timeout = 0;
-	while (((inb_status(base_addr)&HCRE)==0) && (timeout++ < toval))
-		;
-	if (timeout >= toval) {
-		sti();
-		TIMEOUT_MSG(lineno);
-		return FALSE;
-	}
-	return TRUE;
-}
-
 static int start_receive (struct device *, pcb_struct *);
 static void adapter_hard_reset (struct device *);

@@ -303,6 +272,46 @@
 		printk("%s: start receive command failed \n", dev->name);
 }

+/* These should be moved to the private data area */
+static int send_pcb_semaphore = 0;
+static int rx_restart_needed = 0;
+
+/* Primitive functions used by send_pcb() */
+static inline int send_pcb_slow(unsigned int base_addr, unsigned char byte)
+{
+	int timeout;
+	outb_command(byte, base_addr);
+	for (timeout = jiffies + 5; jiffies < timeout; ) {
+		if (inb_status(base_addr) & HCRE) return FALSE;
+	}
+	printk("3c505: send_pcb_slow timed out\n");
+	return TRUE;
+}
+
+static inline int send_pcb_fast(unsigned int base_addr, unsigned char byte)
+{
+	int timeout;
+	outb_command(byte, base_addr);
+	for (timeout = 0; timeout < 40000; timeout++) {
+		if (inb_status(base_addr) & HCRE) return FALSE;
+	}
+	printk("3c505: send_pcb_fast timed out\n");
+	return TRUE;
+}
+
+static int
+start_receive (struct device * dev, pcb_struct * tx_pcb);
+
+/* Check to see if the receiver needs restarting, and kick it if so */
+static inline void prime_rx(struct device *dev)
+{
+	if (rx_restart_needed) {
+		elp_device *adapter = dev->priv;
+		rx_restart_needed = 0;
+		start_receive(dev, &adapter->itx_pcb);
+	}
+}
+
 /*****************************************************************
  *
  * send_pcb
@@ -318,12 +327,28 @@
  *
  *****************************************************************/

+/* This can be quite slow -- the adapter is allowed to take up to 40ms
+ * to respond to the initial interrupt.
+ *
+ * We run initially with interrupts turned on, but with a semaphore set
+ * so that nobody tries to re-enter this code.  Once the first byte has
+ * gone through, we turn interrupts off and then send the others (the
+ * timeout is reduced to 500us).
+ */
+
 static int
 send_pcb (struct device * dev, pcb_struct * pcb)
 {
 	int i;
 	int timeout;
-	int cont;
+
+        /* Avoid contention */
+	if (set_bit(1, &send_pcb_semaphore)) {
+		if (elp_debug >= 3) {
+			printk("%s: send_pcb entered while threaded\n", dev->name);
+		}
+		return FALSE;
+	}

 	/*
 	 * load each byte into the command register and
@@ -331,55 +356,46 @@
 	 * had read the byte
 	 */
 	set_hsf(dev->base_addr,0);
-	if ((cont = WAIT_HCRE(dev->base_addr,5))) {
-		cli();
-		if (pcb->command==CMD_TRANSMIT_PACKET)
-			outb_control(inb_control(dev->base_addr)&~DIR,dev->base_addr);
-		outb_command(pcb->command, dev->base_addr);
-		sti();
-		cont = WAIT_HCRE(dev->base_addr,5);
-	}

-	if (cont) {
-		outb_command(pcb->length, dev->base_addr);
-		cont = WAIT_HCRE(dev->base_addr,5);
-	}
+	if (send_pcb_slow(dev->base_addr, pcb->command)) goto abort;

 	cli();
-	for (i = 0; cont && (i < pcb->length); i++) {
-		outb_command(pcb->data.raw[i], dev->base_addr);
-		cont = wait_fast_hcre(dev->base_addr,20000,__LINE__);
-	} /* if wait_fast_hcre() failed, has already done sti() */
-
-	/* set the host status bits to indicate end of PCB */
-	/* send the total packet length as well */
-	/* wait for the adapter to indicate that it has read the PCB */
-	if (cont) {
-		set_hsf(dev->base_addr,HSF_PCB_END);
-		outb_command(2+pcb->length, dev->base_addr);
-		sti();
-		timeout = jiffies + 7;
-		while (jiffies < timeout) {
-			i = GET_ASF(dev->base_addr);
-			if ((i == ASF_PCB_ACK) || (i == ASF_PCB_NAK))
-				break;
-		}

-		if (i == ASF_PCB_ACK) {
-			reset_count=0;
+	if (send_pcb_fast(dev->base_addr, pcb->length)) goto sti_abort;
+
+	for (i = 0; i < pcb->length; i++) {
+		if (send_pcb_fast(dev->base_addr, pcb->data.raw[i])) goto sti_abort;
+	}
+
+	outb_control(inb_control(dev->base_addr) | 3, dev->base_addr);  /* signal end of PCB */
+	outb_command(2+pcb->length, dev->base_addr);
+
+        /* now wait for the acknowledgement */
+	sti();
+
+	for (timeout = jiffies+5; jiffies < timeout; ) {
+		switch (GET_ASF(dev->base_addr)) {
+		case ASF_PCB_ACK:
+			send_pcb_semaphore = 0;
+			if (pcb->command != CMD_TRANSMIT_PACKET) {
+				prime_rx(dev);
+			}
 			return TRUE;
+			break;
+		case ASF_PCB_NAK:
+			printk(KERN_INFO "%s: send_pcb got NAK\n", dev->name);
+			goto abort;
+			break;
 		}
-		else if (i == ASF_PCB_NAK) {
-			printk("%s: PCB send was NAKed\n", dev->name);
-		} else {
-			printk("%s: timeout after sending PCB\n", dev->name);
-		}
-	} else {
-		sti();
-		printk("%s: timeout in middle of sending PCB\n", dev->name);
 	}

-	adapter_reset(dev);
+	printk("%s: timeout waiting for PCB acknowledge (status %02x)\n", dev->name, inb_status(dev->base_addr));
+
+sti_abort:
+	sti();
+abort:
+	send_pcb_semaphore = 0;
+	prime_rx(dev);
 	return FALSE;
 }

@@ -404,9 +420,6 @@
 	int stat;
 	int timeout;

-	CHECK_NULL(pcb);
-	CHECK_NULL(dev);
-
 	set_hsf(dev->base_addr,0);

 	/* get the command code */
@@ -426,6 +439,7 @@
 		;
 	if (jiffies >= timeout) {
 		TIMEOUT_MSG(__LINE__);
+		printk("%s: status %02x\n", dev->name, stat);
 		return FALSE;
 	}
 	pcb->length = inb_command(dev->base_addr);
@@ -475,8 +489,6 @@
 	int timeout;
 	long flags;

-	CHECK_NULL(dev);
-
 	save_flags(flags);
 	sti();

@@ -524,9 +536,6 @@
 static int
 start_receive (struct device * dev, pcb_struct * tx_pcb)
 {
-	CHECK_NULL(dev);
-	CHECK_NULL(tx_pcb);
-
 	if (elp_debug >= 3)
 		printk("%s: restarting receiver\n", dev->name);
 	tx_pcb->command = CMD_RECEIVE_PACKET;
@@ -557,7 +566,6 @@
 	struct sk_buff *skb;
 	elp_device * adapter;

-	CHECK_NULL(dev);
 	adapter=dev->priv;

 	if (len <= 0 || ((len & ~1) != len))
@@ -581,6 +589,7 @@
 	 * if buffer could not be allocated, swallow it
 	 */
 	if (skb == NULL) {
+		printk(KERN_INFO "%s: memory squeeze, dropping packet\n", dev->name);
 		for (i = 0; i < (rlen/2); i++) {
 			timeout = 0;
 			while ((inb_status(dev->base_addr)&HRDY) == 0 && timeout++ < 20000)
@@ -657,8 +666,6 @@

 	adapter = (elp_device *) dev->priv;

-	CHECK_NULL(adapter);
-
 	if (dev->interrupt)
 		if (elp_debug >= 2)
 			printk("%s: Re-entering the interrupt handler.\n", dev->name);
@@ -687,9 +694,6 @@
 					if (dev->start == 0)
 						break;
 					cli();
-					/* Set direction of adapter FIFO */
-					outb_control(inb_control(dev->base_addr)|DIR,
-					             dev->base_addr);
 					len = adapter->irx_pcb.data.rcv_resp.pkt_len;
 					dlen = adapter->irx_pcb.data.rcv_resp.buf_len;
 					if (adapter->irx_pcb.data.rcv_resp.timeout != 0) {
@@ -706,9 +710,11 @@
 						if (elp_debug >= 3)
 							printk("%s: packet received\n", dev->name);
 					}
-					if (dev->start && !start_receive(dev, &adapter->itx_pcb))
-						if (elp_debug >= 2)
-							printk("%s: interrupt - failed to send receive start PCB\n", dev->name);
+					if (dev->start && !start_receive(dev, &adapter->itx_pcb)) {
+					  if (elp_debug >= 2)
+					    printk("%s: interrupt - receiver start deferred\n", dev->name);
+					  rx_restart_needed = 1;
+					}
 					if (elp_debug >= 3)
 					printk("%s: receive procedure complete\n", dev->name);

@@ -815,8 +821,6 @@
 {
 	elp_device * adapter;

-	CHECK_NULL(dev);
-
 	adapter = dev->priv;

 	if (elp_debug >= 3)
@@ -923,6 +927,12 @@
 	 */
 	if (!start_receive(dev, &adapter->tx_pcb))
 		printk("%s: start receive command failed \n", dev->name);
+	if (!start_receive(dev, &adapter->tx_pcb))
+		printk("%s: start receive command failed \n", dev->name);
+	if (!start_receive(dev, &adapter->tx_pcb))
+		printk("%s: start receive command failed \n", dev->name);
+	if (!start_receive(dev, &adapter->tx_pcb))
+		printk("%s: start receive command failed \n", dev->name);
 	if (elp_debug >= 3)
 		printk("%s: start receive command sent\n", dev->name);

@@ -950,9 +960,6 @@
 	 */
 	unsigned int nlen = (((len < 60) ? 60 : len) + 1) & (~1);

-	CHECK_NULL(dev);
-	CHECK_NULL(ptr);
-
 	adapter = dev->priv;

 	if (nlen < len)
@@ -991,6 +998,8 @@
 	}
 	sti();

+	prime_rx(dev);
+
 	return TRUE;
 }

@@ -1004,19 +1013,28 @@
 static int
 elp_start_xmit (struct sk_buff *skb, struct device *dev)
 {
-	CHECK_NULL(dev);
-
 	/*
-	 * not sure what this does, but the 3c509 driver does it, so...
+	 * if the transmitter is still busy, we have a transmit timeout...
 	 */
+	if (dev->tbusy) {
+		int tickssofar = jiffies - dev->trans_start;
+		int stat;
+		if (tickssofar < 40)
+			return 1;
+		stat = inb_status(dev->base_addr);
+		printk("%s: transmit timed out, %s?\n", dev->name, (stat & ACRF)?"IRQ conflict":"network cable problem");
+		if (elp_debug >= 1)
+		  printk("%s: status %#02x\n", dev->name, stat);
+		dev->trans_start = jiffies;
+		dev->tbusy = 0;
+	}
+
+	/* Some upper layer thinks we've missed a tx-done interrupt */
 	if (skb == NULL) {
 		dev_tint(dev);
 		return 0;
 	}

-	/*
-	 * if we ended up with a munged length, don't send it
-	 */
 	if (skb->len <= 0)
 		return 0;

@@ -1024,26 +1042,10 @@
 		printk("%s: request to send packet of length %d\n", dev->name, (int)skb->len);

 	/*
-	 * if the transmitter is still busy, we have a transmit timeout...
-	 */
-	if (dev->tbusy) {
-		int tickssofar = jiffies - dev->trans_start;
-		int stat;
-		if (tickssofar < 50) /* was 500, AJT */
-			return 1;
-		printk("%s: transmit timed out, not resetting adapter\n", dev->name);
-		if (((stat=inb_status(dev->base_addr))&ACRF) != 0)
-			printk("%s: hmmm...seemed to have missed an interrupt!\n", dev->name);
-		printk("%s: status %#02x\n", dev->name, stat);
-		dev->trans_start = jiffies;
-		dev->tbusy = 0;
-	}
-
-	/*
 	 * send the packet at skb->data for skb->len
 	 */
 	if (!send_packet(dev, skb->data, skb->len)) {
-		printk("%s: send packet PCB failed\n", dev->name);
+		printk("%s: failed to transmit packet\n", dev->name);
 		return 1;
 	}

@@ -1119,9 +1121,7 @@
 {
 	elp_device * adapter;

-	CHECK_NULL(dev);
 	adapter = dev->priv;
-	CHECK_NULL(adapter);

 	if (elp_debug >= 3)
 		printk("%s: request to close device\n", dev->name);
@@ -1242,8 +1242,6 @@
 {
 	elp_device * adapter;

-	CHECK_NULL(dev);
-
 	/*
 	 * set ptrs to various functions
 	 */
@@ -1260,7 +1258,6 @@
 	 * setup ptr to adapter specific information
 	 */
 	adapter = (elp_device *)(dev->priv = kmalloc(sizeof(elp_device), GFP_KERNEL));
-	CHECK_NULL(adapter);
 	if (adapter == NULL)
 		return;
 	memset(&(adapter->stats), 0, sizeof(struct enet_statistics));
@@ -1392,8 +1389,6 @@
 	elp_device adapter;
 	int i;

-	CHECK_NULL(dev);
-
 	/*
 	 *  setup adapter structure
 	 */
@@ -1420,6 +1415,7 @@
 	    (adapter.rx_pcb.command != CMD_ADDRESS_RESPONSE) ||
 	    (adapter.rx_pcb.length != 6)) {
 		printk("%s: not responding to first PCB\n", dev->name);
+		autoirq_report(0);
 		return -ENODEV;
 	}

--- linux/drivers/net/README.3c505.81	Wed Jun  7 09:39:06 1995
+++ linux/drivers/net/README.3c505	Sun Mar 31 15:56:01 1996
@@ -16,17 +16,21 @@
  to 1 may help. As of 3c505.c v0.8 the driver should be able to find
  out whether of not this is needed, but I'm not completely sure.
 ELP_DEBUG
- The driver debug level. 1 is ok for most everything, 0 will provide
- less verbose bootup messages, and 2 and 3 are usually too verbose
- for anything.
+ The driver debug level.  It's probably best to leave it at 0 most of the time.
+ If you are having trouble, setting it to 1 may give you more information.
+ Any higher setting is too verbose for most purposes.

 Known problems:
- During startup the driver shows the following two messages:
- *** timeout at 3c505.c:elp_set_mc_list (line 1158) ***
- *** timeout at 3c505.c:elp_set_mc_list (line 1183) ***
- These are because upper parts of the networking code attempt
- to load multicast address lists to the adapter before the
- adapter is properly up and running.
+ The 3c505 is a slow card, mostly because of the way it talks to the host.
+ Don't expect any great performance from it.
+
+ I am seeing periodic "transmit timed out" and "timeout waiting for PCB
+ acknowledge" messages under high load.  I'm not sure what's causing these -
+ it seems that the 3c505 occasionally just loses a command.  They seem not to
+ be fatal, anyway.
+
+ There may be some initialisation problems still lurking, particularly when
+ warm-booting from DOS (ELP_NEED_HARD_RESET seems not to help).

 Authors:
  The driver is mainly written by Craig Southeren, email
@@ -34,3 +38,4 @@
  Parts of the driver (adapting the driver to 1.1.4+ kernels,
  IRQ/address detection, some changes) and this README by
  Juha Laiho <jlaiho@ichaos.nullnet.fi>.
+ Philip Blundell <pjb27@cam.ac.uk> made some more changes.



home help back first fref pref prev next nref lref last post