NetBSD Problem Report #46998

From www@NetBSD.org  Sun Sep 23 12:11:09 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 7899E63E29B
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 23 Sep 2012 12:11:09 +0000 (UTC)
Message-Id: <20120923121108.1A08463B907@www.NetBSD.org>
Date: Sun, 23 Sep 2012 12:11:08 +0000 (UTC)
From: webpages@sprow.co.uk
Reply-To: webpages@sprow.co.uk
To: gnats-bugs@NetBSD.org
Subject: Enhancements/fixes to seeq8005 driver
X-Send-Pr-Version: www-1.0

>Number:         46998
>Category:       port-acorn32
>Synopsis:       Enhancements/fixes to seeq8005 driver
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    port-acorn32-maintainer
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 23 12:15:00 +0000 2012
>Closed-Date:    Wed Oct 10 22:42:48 +0000 2012
>Last-Modified:  Wed Oct 10 22:45:01 +0000 2012
>Originator:     Robert Sprowson
>Release:        
>Organization:
Self
>Environment:
>Description:
See diffs for proposed patch, and rationale here. Can supply patch by email if the web form mangles it.

@@ -106,7 +106,12 @@
The TX buffer size is not a function of the SEEQ chip, it is an arbitrary driver threshold, so I've renamed the define accordingly and added a new one to specify how many buffers are used (in my port I allow multiple packets to be in flight at once, so have used #ifndef, such that the makefile can override).

@@ -225,12 +230,11 @@
Comment corrected, and make use of the above defines.

@@ -443,6 +447,7 @@
Missing delay(1) added, otherwise the 20,000 timeout loop is dependent on the speed of your processor. Matches ea_stoptx logic now.

@@ -145,7 +150,7 @@
@@ -832,7 +840,7 @@
@@ -864,7 +872,7 @@
@@ -876,7 +884,7 @@
@@ -1107,7 +1113,7 @@
Renamed eatxpacket in line with other ea_* functions.

@@ -604,6 +609,9 @@
The FIFO empty check does nothing if the previous mode was 'read', but is required before changing the BUFCODE (per 80C04 datasheet page 19, note [2]). Then the mode is set to write, so a second FIFO empty check is needed incase the previous mode was read. 

@@ -901,7 +909,7 @@
Treat m0 as a pointer not an integer.

@@ -968,20 +976,18 @@
Remove double write of the NULL packet header. Either do memset/ea_writebuf or two writes to SEEQ_BUFWIN, but not both.
The calculation of nextpacket (for hdr[]) assumes bufstart = 0, and puts the packet header pointing in the wrong place when it isn't.

@@ -1195,9 +1201,6 @@
The setting of CFG2_OUTPUT is done in ea_init(), so this is duplicated code.
>How-To-Repeat:

>Fix:
--- seeq8005_148.c	Mon Jun 11 00:01:56 2012
+++ seeq8005_new.c	Sun Sep 23 11:56:42 2012
@@ -106,7 +106,12 @@
 #define DPRINTF(f, x)
 #endif

-#define	SEEQ_TX_BUFFER_SIZE		0x800		/* (> ETHER_MAX_LEN) */
+#ifndef EA_TX_BUFFER_SIZE
+#define EA_TX_BUFFER_SIZE		0x800		/* (> ETHER_MAX_LEN) */
+#endif
+#ifndef EA_TX_BUFFER_COUNT
+#define EA_TX_BUFFER_COUNT		1		/* (> 0) */
+#endif

 #define SEEQ_READ16(sc, iot, ioh, reg)					\
 	((sc)->sc_flags & SF_8BIT ?					\
@@ -145,7 +150,7 @@
 static struct mbuf *ea_get(struct seeq8005_softc *, int, int, struct ifnet *);
 static void ea_txint(struct seeq8005_softc *);
 static void ea_rxint(struct seeq8005_softc *);
-static void eatxpacket(struct seeq8005_softc *);
+static void ea_txpacket(struct seeq8005_softc *);
 static int ea_writembuf(struct seeq8005_softc *, struct mbuf *, int);
 static void ea_mc_reset(struct seeq8005_softc *);
 static void ea_mc_reset_8004(struct seeq8005_softc *);
@@ -225,12 +230,11 @@
 	/*
 	 * Set up tx and rx buffers.
 	 *
-	 * We use approximately a quarter of the packet memory for TX
+	 * We set aside EA_TX_BUFFER_SIZE * EA_TX_BUFFER_COUNT for TX
 	 * buffers and the rest for RX buffers
 	 */
-	/* sc->sc_tx_bufs = sc->sc_buffersize / SEEQ_TX_BUFFER_SIZE / 4; */
-	sc->sc_tx_bufs = 1;
-	sc->sc_tx_bufsize = sc->sc_tx_bufs * SEEQ_TX_BUFFER_SIZE;
+	sc->sc_tx_bufs = EA_TX_BUFFER_COUNT;
+	sc->sc_tx_bufsize = sc->sc_tx_bufs * EA_TX_BUFFER_SIZE;
 	sc->sc_rx_bufsize = sc->sc_buffersize - sc->sc_tx_bufsize;
 	sc->sc_enabled = 0;

@@ -443,6 +447,7 @@
 	timeout = 20000;
 	do {
 		status = SEEQ_READ16(sc, iot, ioh, SEEQ_STATUS);
+		delay(1);
 	} while ((status & SEEQ_STATUS_RX_ON) && --timeout > 0);
 	if (timeout == 0)
 		log(LOG_ERR, "%s: timeout waiting for rx termination\n",
@@ -604,6 +609,9 @@
 		ea_select_buffer(sc, SEEQ_BUFCODE_LOCAL_MEM);
 		SEEQ_WRITE16(sc, iot, ioh, SEEQ_COMMAND,
 		    sc->sc_command | SEEQ_CMD_FIFO_WRITE);
+
+		ea_await_fifo_empty(sc);
+
 		SEEQ_WRITE16(sc, iot, ioh, SEEQ_DMA_ADDR, addr);
 	}

@@ -832,7 +840,7 @@
 	SEEQ_WRITE16(sc, iot, ioh, SEEQ_COMMAND,
 			  sc->sc_command | SEEQ_CMD_RX_ON);

-	/* TX_ON gets set by eatxpacket when there's something to transmit. */
+	/* TX_ON gets set by ea_txpacket when there's something to transmit. */


 	/* Set flags appropriately. */
@@ -864,7 +872,7 @@

 	/*
 	 * Don't do anything if output is active.  seeq8005intr() will call
-	 * us (actually eatxpacket()) back when the card's ready for more
+	 * us (actually ea_txpacket()) back when the card's ready for more
 	 * frames.
 	 */
 	if (ifp->if_flags & IFF_OACTIVE)
@@ -876,7 +884,7 @@

 	/* tx packets */

-	eatxpacket(sc);
+	ea_txpacket(sc);
 	splx(s);
 }

@@ -888,7 +896,7 @@
  */

 static void
-eatxpacket(struct seeq8005_softc *sc)
+ea_txpacket(struct seeq8005_softc *sc)
 {
 	bus_space_tag_t iot = sc->sc_iot;
 	bus_space_handle_t ioh = sc->sc_ioh;
@@ -901,7 +909,7 @@
 	IFQ_DEQUEUE(&ifp->if_snd, m0);

 	/* If there's nothing to send, return. */
-	if (!m0) {
+	if (m0 == NULL) {
 		ifp->if_flags &= ~IFF_OACTIVE;
 		sc->sc_config2 |= SEEQ_CFG2_OUTPUT;
 		SEEQ_WRITE16(sc, iot, ioh, SEEQ_CONFIG2, sc->sc_config2);
@@ -968,20 +976,18 @@
 	/* Follow it with a NULL packet header */
 	memset(hdr, 0, 4);
 	ea_writebuf(sc, hdr, bufstart + 4 + len, 4);
-	SEEQ_WRITE16(sc, sc->sc_iot, sc->sc_ioh, SEEQ_BUFWIN, 0x0000);
-	SEEQ_WRITE16(sc, sc->sc_iot, sc->sc_ioh, SEEQ_BUFWIN, 0x0000);

 	/* Ok we now have a packet len bytes long in our packet buffer */
 	DPRINTF(SEEQ_DEBUG_TX, ("ea_writembuf: length=%d\n", len));

 	/* Write the packet header */
-	nextpacket = len + 4;
+	nextpacket = bufstart + len + 4;
 	hdr[0] = (nextpacket >> 8) & 0xff;
 	hdr[1] = nextpacket & 0xff;
 	hdr[2] = SEEQ_PKTCMD_TX | SEEQ_PKTCMD_DATA_FOLLOWS |
 		SEEQ_TXCMD_XMIT_SUCCESS_INT | SEEQ_TXCMD_COLLISION_INT;
 	hdr[3] = 0; /* Status byte -- will be updated by hardware. */
-	ea_writebuf(sc, hdr, 0x0000, 4);
+	ea_writebuf(sc, hdr, bufstart, 4);

 	return len;
 }
@@ -1107,7 +1113,7 @@

 		/* Tx next packet */

-		eatxpacket(sc);
+		ea_txpacket(sc);
 	}
 }

@@ -1195,9 +1201,6 @@
 			log(LOG_ERR,
 			    "%s: rx packet size error at %04x (len=%d)\n",
 			    device_xname(&sc->sc_dev), addr, len);
-			sc->sc_config2 |= SEEQ_CFG2_OUTPUT;
-			SEEQ_WRITE16(sc, iot, ioh, SEEQ_CONFIG2,
-					  sc->sc_config2);
 			ea_init(ifp);
 			return;
 		}

>Release-Note:

>Audit-Trail:
From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46998 CVS commit: src/sys/dev/ic
Date: Wed, 10 Oct 2012 22:11:32 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Wed Oct 10 22:11:31 UTC 2012

 Modified Files:
 	src/sys/dev/ic: seeq8005.c

 Log Message:
 Rename eatxpacket to ea_txpacket for consistency.

 First part of PR/46998


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/sys/dev/ic/seeq8005.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Wed, 10 Oct 2012 22:42:48 +0000
State-Changed-Why:
diff applied. thanks for the PR.


From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46998 CVS commit: src/sys/dev/ic
Date: Wed, 10 Oct 2012 22:40:33 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Wed Oct 10 22:40:33 UTC 2012

 Modified Files:
 	src/sys/dev/ic: seeq8005.c

 Log Message:
 Second part of PR/46998. The following is taken from the PR with a slight
 edit from me.

 The TX buffer size is not a function of the SEEQ chip, it is an arbitrary
 driver threshold, so I've renamed the define accordingly and added a new
 one to specify how many buffers are used (in my port I allow multiple
 packets to be in flight at once, so have used #ifndef, such that the
 makefile can override).

 Comment corrected, and make use of the above defines.

 Missing delay(1) added, otherwise the 20,000 timeout loop is dependent
 on the speed of your processor. Matches ea_stoptx logic now.

 The FIFO empty check does nothing if the previous mode was 'read', but is
 required before changing the BUFCODE (per 80C04 datasheet page 19,
 note [2]). Then the mode is set to write, so a second FIFO empty check is
 needed incase the previous mode was read.

 Treat m0 as a pointer not an integer.

 Remove double write of the NULL packet header. Either do
 memset/ea_writebuf or two writes to SEEQ_BUFWIN, but not both.

 The calculation of nextpacket (for hdr[]) assumes bufstart = 0, and puts
 the packet header pointing in the wrong place when it isn't.

 The setting of CFG2_OUTPUT is done in ea_init(), so doing it in ea_rxinit
 is duplicated code.


 To generate a diff of this commit:
 cvs rdiff -u -r1.50 -r1.51 src/sys/dev/ic/seeq8005.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search