Subject: Re: bge(4) (DEGXA-TX) no-go on the AlphaServer ES40
To: NetBSD/alpha Discussion List <port-alpha@NetBSD.ORG>
From: Jason Thorpe <thorpej@shagadelic.org>
List: tech-net
Date: 11/27/2004 23:24:04
--Apple-Mail-4-912876596
Content-Type: multipart/mixed; boundary=Apple-Mail-3-912876588


--Apple-Mail-3-912876588
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	charset=US-ASCII;
	format=flowed


On Nov 25, 2004, at 1:19 PM, Greg A. Woods wrote:

> I'll take #1 but I think I like #2 best too!  ;-)

Ok, please try the following patch against -current.  It implements 
deferred-until-UP creation of DMA maps for the wm driver, and fixes a 
DMA resource leak in the bge driver.

Note: The bge driver is horribly structured, and really needs an 
overhaul, but I did the minimum work on it for now.

         -- Jason R. Thorpe <thorpej@shagadelic.org>

--Apple-Mail-3-912876588
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
	x-unix-mode=0644;
	name="wm-bge-patch.txt"
Content-Disposition: attachment;
	filename=wm-bge-patch.txt

Index: if_bge.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_bge.c,v
retrieving revision 1.79
diff -u -p -r1.79 if_bge.c
--- if_bge.c	19 Nov 2004 17:59:09 -0000	1.79
+++ if_bge.c	28 Nov 2004 07:22:19 -0000
@@ -891,10 +891,6 @@ bge_newbuf_std(sc, i, m, dmamap)
 		m_new->m_len = m_new->m_pkthdr.len = MCLBYTES;
 		if (!sc->bge_rx_alignment_bug)
 		    m_adj(m_new, ETHER_ALIGN);
-
-		if (bus_dmamap_load_mbuf(sc->bge_dmatag, dmamap, m_new,
-		    BUS_DMA_READ|BUS_DMA_NOWAIT))
-			return(ENOBUFS);
 	} else {
 		m_new = m;
 		m_new->m_len = m_new->m_pkthdr.len = MCLBYTES;
@@ -903,6 +899,14 @@ bge_newbuf_std(sc, i, m, dmamap)
 		    m_adj(m_new, ETHER_ALIGN);
 	}
 
+	error = bus_dmamap_load_mbuf(sc->bge_dmatag, dmamap, m_new,
+	    BUS_DMA_READ|BUS_DMA_NOWAIT);
+	if (error) {
+		printf("%s: unable to load std Rx DMA map (idx %d), "
+		    "error = %d\n", sc->bge_dev.dv_xname, i, error);
+		panic("bge_newbuf_std");
+	}
+
 	sc->bge_cdata.bge_rx_std_chain[i] = m_new;
 	r = &sc->bge_rdata->bge_rx_std_ring[i];
 	bge_set_hostaddr(&r->bge_addr,
@@ -2804,6 +2808,7 @@ bge_rxeof(sc)
 			stdcnt++;
 			dmamap = sc->bge_cdata.bge_rx_std_map[rxidx];
 			sc->bge_cdata.bge_rx_std_map[rxidx] = 0;
+			bus_dmamap_unload(sc->bge_dmatag, dmamap);
 			if (cur_rx->bge_flags & BGE_RXBDFLAG_ERROR) {
 				ifp->if_ierrors++;
 				bge_newbuf_std(sc, sc->bge_std, m, dmamap);
Index: if_wm.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_wm.c,v
retrieving revision 1.88
diff -u -p -r1.88 if_wm.c
--- if_wm.c	24 Nov 2004 15:14:13 -0000	1.88
+++ if_wm.c	28 Nov 2004 07:22:22 -0000
@@ -364,6 +364,7 @@ do {									\
 #define	WM_F_BUS64		0x20	/* bus is 64-bit */
 #define	WM_F_PCIX		0x40	/* bus is PCI-X */
 #define	WM_F_CSA		0x80	/* bus is CSA */
+#define	WM_F_HAVE_MAPS		0x100	/* Tx and Rx maps exist */
 
 #ifdef WM_EVENT_COUNTERS
 #define	WM_EVCNT_INCR(ev)	(ev)->ev_count++
@@ -1003,36 +1004,10 @@ wm_attach(struct device *parent, struct 
 		goto fail_3;
 	}
 
-
-	/*
-	 * Create the transmit buffer DMA maps.
-	 */
+	/* Enqueue fewer Tx buffers on the 82547. */
 	WM_TXQUEUELEN(sc) =
 	    (sc->sc_type == WM_T_82547 || sc->sc_type == WM_T_82547_2) ?
 	    WM_TXQUEUELEN_MAX_82547 : WM_TXQUEUELEN_MAX;
-	for (i = 0; i < WM_TXQUEUELEN(sc); i++) {
-		if ((error = bus_dmamap_create(sc->sc_dmat, WM_MAXTXDMA,
-					       WM_NTXSEGS, WTX_MAX_LEN, 0, 0,
-					  &sc->sc_txsoft[i].txs_dmamap)) != 0) {
-			aprint_error("%s: unable to create Tx DMA map %d, "
-			    "error = %d\n", sc->sc_dev.dv_xname, i, error);
-			goto fail_4;
-		}
-	}
-
-	/*
-	 * Create the receive buffer DMA maps.
-	 */
-	for (i = 0; i < WM_NRXDESC; i++) {
-		if ((error = bus_dmamap_create(sc->sc_dmat, MCLBYTES, 1,
-					       MCLBYTES, 0, 0,
-					  &sc->sc_rxsoft[i].rxs_dmamap)) != 0) {
-			aprint_error("%s: unable to create Rx DMA map %d, "
-			    "error = %d\n", sc->sc_dev.dv_xname, i, error);
-			goto fail_5;
-		}
-		sc->sc_rxsoft[i].rxs_mbuf = NULL;
-	}
 
 	/*
 	 * Reset the chip to a known state.
@@ -1300,19 +1275,6 @@ wm_attach(struct device *parent, struct 
 	 * Free any resources we've allocated during the failed attach
 	 * attempt.  Do this in reverse order and fall through.
 	 */
- fail_5:
-	for (i = 0; i < WM_NRXDESC; i++) {
-		if (sc->sc_rxsoft[i].rxs_dmamap != NULL)
-			bus_dmamap_destroy(sc->sc_dmat,
-			    sc->sc_rxsoft[i].rxs_dmamap);
-	}
- fail_4:
-	for (i = 0; i < WM_TXQUEUELEN(sc); i++) {
-		if (sc->sc_txsoft[i].txs_dmamap != NULL)
-			bus_dmamap_destroy(sc->sc_dmat,
-			    sc->sc_txsoft[i].txs_dmamap);
-	}
-	bus_dmamap_unload(sc->sc_dmat, sc->sc_cddmamap);
  fail_3:
 	bus_dmamap_destroy(sc->sc_dmat, sc->sc_cddmamap);
  fail_2:
@@ -2475,6 +2437,80 @@ wm_reset(struct wm_softc *sc)
 }
 
 /*
+ * wm_destroy_maps:
+ *
+ *	Tear down Tx and Rx DMA maps.
+ */
+static void
+wm_destroy_maps(struct wm_softc *sc)
+{
+	int i;
+
+	/* Transmit maps. */
+	for (i = 0; i < WM_TXQUEUELEN(sc); i++) {
+		if (sc->sc_txsoft[i].txs_dmamap != NULL) {
+			bus_dmamap_destroy(sc->sc_dmat,
+					   sc->sc_txsoft[i].txs_dmamap);
+			sc->sc_txsoft[i].txs_dmamap = NULL;
+		}
+	}
+
+	/* Receive maps. */
+	for (i = 0; i < WM_NRXDESC; i++) {
+		if (sc->sc_rxsoft[i].rxs_dmamap != NULL) {
+			bus_dmamap_destroy(sc->sc_dmat,
+					   sc->sc_rxsoft[i].rxs_dmamap);
+			sc->sc_rxsoft[i].rxs_dmamap = NULL;
+		}
+	}
+
+	sc->sc_flags &= ~WM_F_HAVE_MAPS;
+}
+
+/*
+ * wm_create_maps:
+ *
+ *	Create Tx and Rx DMA maps.
+ */
+static int
+wm_create_maps(struct wm_softc *sc)
+{
+	int error, i;
+
+	if (sc->sc_flags & WM_F_HAVE_MAPS)
+		return (0);
+	
+	/* Transmit maps. */
+	for (i = 0; i < WM_TXQUEUELEN(sc); i++) {
+		if ((error = bus_dmamap_create(sc->sc_dmat, WM_MAXTXDMA,
+					       WM_NTXSEGS, WTX_MAX_LEN, 0,
+					       BUS_DMA_ALLOCNOW,
+				&sc->sc_txsoft[i].txs_dmamap)) != 0) {
+			log(LOG_ERR, "%s: unable to create Tx DMA map %d, "
+			    "error = %d\n", sc->sc_dev.dv_xname, i, error);
+			wm_destroy_maps(sc);
+			return (error);
+		}
+	}
+
+	/* Receive maps. */
+	for (i = 0; i < WM_NRXDESC; i++) {
+		if ((error = bus_dmamap_create(sc->sc_dmat, MCLBYTES, 1,
+					       MCLBYTES, 0, BUS_DMA_ALLOCNOW,
+				&sc->sc_rxsoft[i].rxs_dmamap)) != 0) {
+			log(LOG_ERR, "%s: unable to create Rx DMA map %d, "
+			    "error = %d\n", sc->sc_dev.dv_xname, i, error);
+			wm_destroy_maps(sc);
+			return (error);
+		}
+		sc->sc_rxsoft[i].rxs_mbuf = NULL;
+	}
+
+	sc->sc_flags |= WM_F_HAVE_MAPS;
+	return (0);
+}
+
+/*
  * wm_init:		[ifnet interface function]
  *
  *	Initialize the interface.  Must be called at splnet().
@@ -2513,6 +2549,10 @@ wm_init(struct ifnet *ifp)
 	/* Reset the chip to a known state. */
 	wm_reset(sc);
 
+	/* Create the Tx and Rx DMA maps. */
+	if ((error = wm_create_maps(sc)) != 0)
+		goto out;
+
 	/* Initialize the transmit descriptor ring. */
 	memset(sc->sc_txdescs, 0, WM_TXDESCSIZE(sc));
 	WM_CDTXSYNC(sc, 0, WM_NTXDESC(sc),
@@ -2696,10 +2736,10 @@ wm_init(struct ifnet *ifp)
 	sc->sc_rctl = RCTL_EN | RCTL_LBM_NONE | RCTL_RDMTS_1_2 | RCTL_LPE |
 	    RCTL_DPF | RCTL_MO(sc->sc_mchash_type);
 
-	if(MCLBYTES == 2048) {
+	if (MCLBYTES == 2048) {
 		sc->sc_rctl |= RCTL_2k;
 	} else {
-		if(sc->sc_type >= WM_T_82543) {
+		if (sc->sc_type >= WM_T_82543) {
 			switch(MCLBYTES) {
 			case 4096:
 				sc->sc_rctl |= RCTL_BSEX | RCTL_BSEX_4k;
@@ -2715,7 +2755,8 @@ wm_init(struct ifnet *ifp)
 				    MCLBYTES);
 				break;
 			}
-		} else panic("wm_init: i82542 requires MCLBYTES = 2048");
+		} else
+			panic("wm_init: i82542 requires MCLBYTES = 2048");
 	}
 
 	/* Set the receive filter. */
@@ -2729,9 +2770,11 @@ wm_init(struct ifnet *ifp)
 	ifp->if_flags &= ~IFF_OACTIVE;
 
  out:
-	if (error)
+	if (error) {
+		wm_destroy_maps(sc);
 		log(LOG_ERR, "%s: interface not running\n",
 		    sc->sc_dev.dv_xname);
+	}
 	return (error);
 }
 
@@ -2794,8 +2837,10 @@ wm_stop(struct ifnet *ifp, int disable)
 		}
 	}
 
-	if (disable)
+	if (disable) {
 		wm_rxdrain(sc);
+		wm_destroy_maps(sc);
+	}
 
 	/* Mark the interface as down and cancel the watchdog timer. */
 	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
@@ -3072,9 +3117,12 @@ wm_add_rxbuf(struct wm_softc *sc, int id
 	error = bus_dmamap_load_mbuf(sc->sc_dmat, rxs->rxs_dmamap, m,
 	    BUS_DMA_READ|BUS_DMA_NOWAIT);
 	if (error) {
-		/* XXX XXX XXX */
-		printf("%s: unable to load rx DMA map %d, error = %d\n",
-		    sc->sc_dev.dv_xname, idx, error);
+		/*
+		 * It's OK to panic here; the Rx map is created with
+		 * BUS_DMA_ALLOCNOW, so this should never really fail.
+		 */
+		printf("%s: unable to load Rx DMA map (idx %d), "
+		    "error = %d\n", sc->sc_dev.dv_xname, idx, error);
 		panic("wm_add_rxbuf");
 	}
 

--Apple-Mail-3-912876588--

--Apple-Mail-4-912876596
content-type: application/pgp-signature; x-mac-type=70674453;
	name=PGP.sig
content-description: This is a digitally signed message part
content-disposition: inline; filename=PGP.sig
content-transfer-encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (Darwin)

iD8DBQFBqX0UOpVKkaBm8XkRAjSZAJ9ijYdiPd22ZxwOYF3iiZFDAMrSSgCfV3xU
ukAVOoi1fN0WBj6F2x65dtc=
=yLWV
-----END PGP SIGNATURE-----

--Apple-Mail-4-912876596--