From owner-p4-projects@FreeBSD.ORG Sun Aug 6 11:16:57 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 2E8DB16A4E5; Sun, 6 Aug 2006 11:16:57 +0000 (UTC) X-Original-To: perforce@FreeBSD.org Delivered-To: perforce@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 09A9316A4E1 for ; Sun, 6 Aug 2006 11:16:57 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2E6A343D49 for ; Sun, 6 Aug 2006 11:16:56 +0000 (GMT) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id k76BGu0l043292 for ; Sun, 6 Aug 2006 11:16:56 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id k76BGtmo043289 for perforce@freebsd.org; Sun, 6 Aug 2006 11:16:55 GMT (envelope-from hselasky@FreeBSD.org) Date: Sun, 6 Aug 2006 11:16:55 GMT Message-Id: <200608061116.k76BGtmo043289@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 103330 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 06 Aug 2006 11:16:57 -0000 http://perforce.freebsd.org/chv.cgi?CH=103330 Change 103330 by hselasky@hselasky_mini_itx on 2006/08/06 11:16:46 Try to get the mii-locking right. Until further lock the driver's private lock, sc->sc_mtx, from all mii-callbacks, hence during attach there are some problems, and it is not possible to hold sc->sc_mtx when calling "mii_phy_probe()", because this function calls memory allocation functions that can sleep. Add some more debugging statements. Cleanup some comments. Move an include file further up. And some other small things. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/if_aue.c#5 edit .. //depot/projects/usb/src/sys/dev/usb/if_auereg.h#4 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/if_aue.c#5 (text+ko) ==== @@ -101,6 +101,9 @@ #include +/* "device miibus" required. See GENERIC if you get errors here. */ +#include "miibus_if.h" + __FBSDID("$FreeBSD: src/sys/dev/usb/if_aue.c,v 1.96 2006/02/14 12:44:55 glebius Exp $"); MODULE_DEPEND(aue, usb, 1, 1, 1); @@ -121,9 +124,6 @@ #define DPRINTF(...) #endif -/* "device miibus" required. See GENERIC if you get errors here. */ -#include "miibus_if.h" - /* * Various supported device vendors/products. */ @@ -550,6 +550,8 @@ struct aue_softc * sc = device_get_softc(dev); u_int16_t i; + mtx_lock(&(sc->sc_mtx)); /* XXX */ + /* * The Am79C901 HomePNA PHY actually contains * two transceivers: a 1Mbps HomePNA PHY and a @@ -564,12 +566,14 @@ (sc->sc_product == USB_PRODUCT_ADMTEK_PEGASUS)) { if (phy == 3) { - return (0); + i = 0; + goto done; } #ifdef notdef if (phy != 1) { - return (0); + i = 0; + goto done; } #endif } @@ -596,6 +600,9 @@ i = aue_cfg_csr_read_2(sc, AUE_PHY_DATA); + done: + mtx_unlock(&(sc->sc_mtx)); /* XXX */ + return i; } @@ -609,6 +616,8 @@ return (0); } + mtx_lock(&(sc->sc_mtx)); /* XXX */ + aue_cfg_csr_write_2(sc, AUE_PHY_DATA, data); aue_cfg_csr_write_1(sc, AUE_PHY_ADDR, phy); aue_cfg_csr_write_1(sc, AUE_PHY_CTL, reg | AUE_PHYCTL_WRITE); @@ -629,6 +638,8 @@ } } + mtx_unlock(&(sc->sc_mtx)); /* XXX */ + return(0); } @@ -638,6 +649,8 @@ struct aue_softc * sc = device_get_softc(dev); struct mii_data * mii = GET_MII(sc); + mtx_lock(&(sc->sc_mtx)); /* XXX */ + AUE_CFG_CLRBIT(sc, AUE_CTL0, AUE_CTL0_RX_ENB | AUE_CTL0_TX_ENB); if (IFM_SUBTYPE(mii->mii_media_active) == IFM_100_TX) { @@ -665,6 +678,8 @@ aue_cfg_miibus_writereg(dev, 0, 0x1b, auxmode | 0x04); } + mtx_unlock(&(sc->sc_mtx)); /* XXX */ + return; } @@ -672,7 +687,7 @@ aue_cfg_setmulti(struct aue_softc *sc, struct aue_config_copy *cc, u_int16_t refcount) { - u_int16_t i; + u_int16_t i; if (cc == NULL) { /* nothing to do */ @@ -821,7 +836,9 @@ __callout_init_mtx(&(sc->sc_watchdog), &(sc->sc_mtx), CALLOUT_RETURNUNLOCKED); - if (usbd_set_config_no(uaa->device, AUE_CONFIG_NO, 0)) { + error = usbd_set_config_no(uaa->device, AUE_CONFIG_NO, 0); + + if (error) { device_printf(dev, "setting config " "number failed!\n"); goto detach; @@ -870,11 +887,15 @@ struct aue_config_copy *cc, u_int16_t refcount) { struct ifnet * ifp; - u_int8_t eaddr[ETHER_ADDR_LEN]; + int error; + u_int8_t eaddr[min(ETHER_ADDR_LEN,6)]; /* reset the adapter */ aue_cfg_reset(sc); + /* set default value */ + bzero(eaddr, sizeof(eaddr)); + /* get station address from the EEPROM */ aue_cfg_read_eeprom(sc, eaddr, 0, 3); @@ -900,17 +921,20 @@ ifp->if_init = aue_init_cb; ifp->if_snd.ifq_maxlen = IFQ_MAXLEN; - /* XXX: we need Giant when we - * clobber with the bus: - * - * FIXME: right here we are locking - * in the wrong order: + /* XXX need Giant when accessing + * the device structures ! */ mtx_unlock(&(sc->sc_mtx)); mtx_lock(&Giant); + error = mii_phy_probe(sc->sc_dev, &(sc->sc_miibus), + &aue_ifmedia_upd_cb, + &aue_ifmedia_sts_cb); + + mtx_unlock(&Giant); + mtx_lock(&(sc->sc_mtx)); /* @@ -926,18 +950,13 @@ * end up getting the children deleted twice, which will crash * the system. */ - if (mii_phy_probe(sc->sc_dev, &(sc->sc_miibus), - &aue_ifmedia_upd_cb, - &aue_ifmedia_sts_cb)) { - printf("%s: MII without any PHY!\n", - sc->sc_name); - if_free(ifp); - mtx_unlock(&Giant); - goto done; + if (error) { + printf("%s: MII without any PHY!\n", + sc->sc_name); + if_free(ifp); + goto done; } - mtx_unlock(&Giant); - sc->sc_ifp = ifp; /* @@ -1081,10 +1100,6 @@ return; } -/* - * A frame has been uploaded: pass the resulting mbuf chain up to - * the higher level protocols. - */ static void aue_bulk_read_callback(struct usbd_xfer *xfer) { @@ -1100,14 +1115,12 @@ sc->sc_flags |= AUE_FLAG_READ_STALL; usbd_transfer_start(sc->sc_xfer[3]); } + DPRINTF(sc, 0, "bulk read error, %s\n", + usbd_errstr(xfer->error)); return; tr_transferred: - if (!(ifp->if_drv_flags & IFF_DRV_RUNNING)) { - goto tr_setup; - } - if (xfer->actlen <= (4 + ETHER_CRC_LEN)) { ifp->if_ierrors++; goto tr_setup; @@ -1221,8 +1234,8 @@ } if (sc->sc_flags & AUE_FLAG_WAIT_LINK) { - /* don't send anything while a command - * is pending, or there is no link ! + /* don't send anything + * if there is no link ! */ goto done; } @@ -1277,7 +1290,7 @@ { struct ifnet * ifp = sc->sc_ifp; struct ifmultiaddr * ifma; - u_int16_t h; + u_int8_t h; u_int8_t i; bzero(cc, sizeof(*cc)); @@ -1298,9 +1311,9 @@ continue; } - h = ether_crc32_le - (LLADDR((struct sockaddr_dl *)(ifma->ifma_addr)), - ETHER_ADDR_LEN) & ((1 << AUE_BITS) - 1); + h = (ether_crc32_le + (LLADDR((struct sockaddr_dl *)(ifma->ifma_addr)), + ETHER_ADDR_LEN)) & ((1 << AUE_BITS) - 1); cc->if_hash[(h >> 3)] |= (1 << (h & 7)); } @@ -1563,7 +1576,8 @@ if (mii == NULL) { error = EINVAL; } else { - error = ifmedia_ioctl(ifp, (void *)data, &(mii->mii_media), command); + error = ifmedia_ioctl + (ifp, (void *)data, &(mii->mii_media), command); } break; @@ -1597,6 +1611,8 @@ /* * Stop the adapter and free any mbufs allocated to the * RX and TX lists. + * + * NOTE: can be called when "ifp" is NULL */ static void aue_cfg_stop(struct aue_softc *sc, ==== //depot/projects/usb/src/sys/dev/usb/if_auereg.h#4 (text+ko) ==== @@ -193,10 +193,9 @@ #define AUE_RXSTAT_DRIBBLE 0x10 #define AUE_RXSTAT_MASK 0x1E -#define AUE_INC(x, y) (x) = ((x) + 1) % (y) +#define GET_MII(sc) ((sc)->sc_miibus ? \ + device_get_softc((sc)->sc_miibus) : NULL) -#define GET_MII(sc) ((sc)->sc_miibus ? device_get_softc((sc)->sc_miibus) : NULL) - struct aue_softc { struct usbd_config_td sc_config_td; struct usbd_memory_wait sc_mem_wait; @@ -211,7 +210,6 @@ device_t sc_dev; u_int32_t sc_unit; - u_int32_t sc_media_active; u_int32_t sc_media_status;