NetBSD Problem Report #46576

From www@NetBSD.org  Sun Jun 10 14:52:52 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 B9D9363B89C
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 10 Jun 2012 14:52:52 +0000 (UTC)
Message-Id: <20120610145251.70D0463B882@www.NetBSD.org>
Date: Sun, 10 Jun 2012 14:52:51 +0000 (UTC)
From: webpages@sprow.co.uk
Reply-To: webpages@sprow.co.uk
To: gnats-bugs@NetBSD.org
Subject: Shutdown doesn't disable TX/RX interrupts in SEEQ8005 driver
X-Send-Pr-Version: www-1.0

>Number:         46576
>Category:       port-acorn32
>Synopsis:       Shutdown doesn't disable TX/RX interrupts in SEEQ8005 driver
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    christos
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jun 10 14:55:00 +0000 2012
>Closed-Date:    Sun Jun 10 15:20:58 +0000 2012
>Last-Modified:  Tue Jun 12 19:50:01 +0000 2012
>Originator:     Robert Sprowson
>Release:        
>Organization:
>Environment:
>Description:
In porting the SEEQ8005 I've spotted a few problems and a few bits of code cleanup. The "fix" text is supplied in the form of a unified diff against "seeq8005.c,v 1.47" in src/sys/dev/ic, if the web form messes it up it can be mailed.

Line 477:
A missing '~' means that the RX and TX interrupts aren't actually disabled on shutdown. This is problematic in my port where the driver can be restarted at runtime rather than just once on boot.

Line 493; 1160; 
Odd tab/space made consistent with rest of the source file.

Line 833; 981; 1485;
Typo/spelling mistake correction.

Line 888; 1111: 
Function declared as static at the top of the file, but has external linkage further down.

Line 1238; 1267; 1296:
Assignment of integer to pointer type. Changed from '0' to 'NULL'.
>How-To-Repeat:

>Fix:
--- orgseeq8005.c	Sun Jun 10 15:36:08 2012
+++ updseeq8005.c	Sun Jun 10 15:33:54 2012
@@ -477,7 +479,7 @@
 	ea_stoprx(sc);

 	/* Disable rx and tx interrupts */
-	sc->sc_command &= (SEEQ_CMD_RX_INTEN | SEEQ_CMD_TX_INTEN);
+	sc->sc_command &= ~(SEEQ_CMD_RX_INTEN | SEEQ_CMD_TX_INTEN);

 	/* Clear any pending interrupts */
 	SEEQ_WRITE16(sc, iot, ioh, SEEQ_COMMAND,
@@ -493,7 +495,7 @@
 	}

 	/* Cancel any watchdog timer */
-       	sc->sc_ethercom.ec_if.if_timer = 0;
+	sc->sc_ethercom.ec_if.if_timer = 0;
 }


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

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


 	/* Set flags appropriately. */
@@ -888,7 +890,7 @@
  * Called at splnet()
  */

-void
+static void
 eatxpacket(struct seeq8005_softc *sc)
 {
 	bus_space_tag_t iot = sc->sc_iot;
@@ -981,7 +983,7 @@
 	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 update by hardware. */
+	hdr[3] = 0; /* Status byte -- will be updated by hardware. */
 	ea_writebuf(sc, hdr, 0x0000, 4);

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

-void
+static void
 ea_rxint(struct seeq8005_softc *sc)
 {
 	bus_space_tag_t iot = sc->sc_iot;
@@ -1160,7 +1162,7 @@
 		}

 		/* Get packet length */
-       		len = (ptr - addr) - 4;
+		len = (ptr - addr) - 4;

 		if (len < 0)
 			len += sc->sc_rx_bufsize;
@@ -1238,7 +1240,7 @@

 	/* Pull packet off interface. */
 	m = ea_get(sc, addr, len, ifp);
-	if (m == 0)
+	if (m == NULL)
 		return;

 	/*
@@ -1267,20 +1269,20 @@
         epkt = cp + totlen;

         MGETHDR(m, M_DONTWAIT, MT_DATA);
-        if (m == 0)
-                return 0;
+        if (m == NULL)
+                return NULL;
         m->m_pkthdr.rcvif = ifp;
         m->m_pkthdr.len = totlen;
         m->m_len = MHLEN;
-        top = 0;
+        top = NULL;
         mp = &top;

         while (totlen > 0) {
                 if (top) {
                         MGET(m, M_DONTWAIT, MT_DATA);
-                        if (m == 0) {
+                        if (m == NULL) {
                                 m_freem(top);
-                                return 0;
+                                return NULL;
                         }
                         m->m_len = MLEN;
                 }
@@ -1296,13 +1298,13 @@
                          * Place initial small packet/header at end of mbuf.
                          */
                         if (len < m->m_len) {
-                                if (top == 0 && len + max_linkhdr <= m->m_len)
+                                if (top == NULL && len + max_linkhdr <= m->m_len)
                                         m->m_data += max_linkhdr;
                                 m->m_len = len;
                         } else
                                 len = m->m_len;
                 }
-		if (top == 0) {
+		if (top == NULL) {
 			/* Make sure the payload is aligned */
 			char *newdata = (char *)
 			    ALIGN((char*)m->m_data + 
@@ -1485,4 +1487,4 @@
 	ifp->if_timer = 0;
 }

-/* End of if_ea.c */
+/* End of seeq8005.c */

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46576 CVS commit: src/sys/dev/ic
Date: Sun, 10 Jun 2012 11:00:49 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sun Jun 10 15:00:49 UTC 2012

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

 Log Message:
 PR/46576: Robert Sprowson: Shutdown doesn't disable TX/RX interrupts in
 SEEQ8005 driver, plus misc white-space and 0->NULL fixes.


 To generate a diff of this commit:
 cvs rdiff -u -r1.47 -r1.48 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.

Responsible-Changed-From-To: port-acorn32-maintainer->christos
Responsible-Changed-By: wiz@NetBSD.org
Responsible-Changed-When: Sun, 10 Jun 2012 15:20:58 +0000
Responsible-Changed-Why:
christos fixed it.


State-Changed-From-To: open->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sun, 10 Jun 2012 15:20:58 +0000
State-Changed-Why:
Fixed, thanks!


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-acorn32/46576 (Shutdown doesn't disable TX/RX interrupts in
 SEEQ8005 driver)
Date: Sun, 10 Jun 2012 17:51:16 +0000

 On Sun, Jun 10, 2012 at 03:20:59PM +0000, wiz@NetBSD.org wrote:
  > Responsible-Changed-From-To: port-acorn32-maintainer->christos
  > Responsible-Changed-By: wiz@NetBSD.org
  > Responsible-Changed-When: Sun, 10 Jun 2012 15:20:58 +0000
  > Responsible-Changed-Why:
  > christos fixed it.
  > 
  > 
  > State-Changed-From-To: open->closed
  > State-Changed-By: wiz@NetBSD.org
  > State-Changed-When: Sun, 10 Jun 2012 15:20:58 +0000
  > State-Changed-Why:
  > Fixed, thanks!

 Shouldn't it be pulled up to -6?

 -- 
 David A. Holland
 dholland@netbsd.org

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	webpages@sprow.co.uk
Cc: 
Subject: Re: port-acorn32/46576 (Shutdown doesn't disable TX/RX interrupts in SEEQ8005 driver)
Date: Sun, 10 Jun 2012 18:22:45 -0400

 On Jun 10,  5:55pm, dholland-bugs@netbsd.org (David Holland) wrote:
 -- Subject: Re: port-acorn32/46576 (Shutdown doesn't disable TX/RX interrupts

 |  Shouldn't it be pulled up to -6?

 I will.

 christos

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46576 CVS commit: [netbsd-6] src/sys/dev/ic
Date: Tue, 12 Jun 2012 19:44:46 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Tue Jun 12 19:44:46 UTC 2012

 Modified Files:
 	src/sys/dev/ic [netbsd-6]: seeq8005.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #325):
 	sys/dev/ic/seeq8005.c: revision 1.48
 PR/46576: Robert Sprowson: Shutdown doesn't disable TX/RX interrupts in
 SEEQ8005 driver, plus misc white-space and 0->NULL fixes.


 To generate a diff of this commit:
 cvs rdiff -u -r1.46 -r1.46.2.1 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