Small fixes to sis(4):

- Remove sis_unit and use device_printf() and if_printf() instead.
- Use callout_init_mtx() for the callout.
- Remove spls.
- Fix locking for ifmedia to happen in the ifmedia handlers rather than in
  sis_ioctl().
- Log an error message if we fail to allocate any resources.  Perform
  cleanup if we fail to allocate any resources so that we don't leave
  a mutex hanging around.

Tested by:	Jason Tsai jason dot tsai at newcyberian dot com (1-4)
MFC after:	3 days

2 files changed, 33 insertions, 44 deletions

diff --git a/sys/pci/if_sis.c b/sys/pci/if_sis.c
index 4d3363b..117a46a 100644
--- a/sys/pci/if_sis.c
+++ b/sys/pci/if_sis.c
@@ -465,9 +465,7 @@ sis_mii_send(struct sis_softc *sc, uint32_t bits, int cnt)
 static int
 sis_mii_readreg(struct sis_softc *sc, struct sis_mii_frame *frame)
 {
-	int			i, ack, s;
- 
-	s = splimp();
+	int			i, ack;
  
 	/*
 	 * Set up frame for RX.
@@ -541,8 +539,6 @@ fail:
 	SIO_SET(SIS_MII_CLK);
 	DELAY(1);
 
-	splx(s);
-
 	if (ack)
 		return(1);
 	return(0);
@@ -554,9 +550,7 @@ fail:
 static int
 sis_mii_writereg(struct sis_softc *sc, struct sis_mii_frame *frame)
 {
-	int			s;
  
-	 s = splimp();
  	/*
  	 * Set up frame for TX.
  	 */
@@ -590,8 +584,6 @@ sis_mii_writereg(struct sis_softc *sc, struct sis_mii_frame *frame)
  	 */
  	SIO_CLR(SIS_MII_DIR);
  
- 	splx(s);
- 
  	return(0);
 }
 
@@ -643,8 +635,7 @@ sis_miibus_readreg(device_t dev, int phy, int reg)
 		}
 
 		if (i == SIS_TIMEOUT) {
-			printf("sis%d: PHY failed to come ready\n",
-			    sc->sis_unit);
+			if_printf(sc->sis_ifp, "PHY failed to come ready\n");
 			return(0);
 		}
 
@@ -702,8 +693,7 @@ sis_miibus_writereg(device_t dev, int phy, int reg, int data)
 		}
 
 		if (i == SIS_TIMEOUT)
-			printf("sis%d: PHY failed to come ready\n",
-			    sc->sis_unit);
+			if_printf(sc->sis_ifp, "PHY failed to come ready\n");
 	} else {
 		bzero((char *)&frame, sizeof(frame));
 
@@ -869,7 +859,7 @@ sis_reset(struct sis_softc *sc)
 	}
 
 	if (i == SIS_TIMEOUT)
-		printf("sis%d: reset never completed\n", sc->sis_unit);
+		if_printf(sc->sis_ifp, "reset never completed\n");
 
 	/* Wait a little while for the chip to get its brains in order. */
 	DELAY(1000);
@@ -919,16 +909,16 @@ sis_attach(device_t dev)
 	u_char			eaddr[ETHER_ADDR_LEN];
 	struct sis_softc	*sc;
 	struct ifnet		*ifp;
-	int			unit, error = 0, waittime = 0;
+	int			error = 0, waittime = 0;
 
 	waittime = 0;
 	sc = device_get_softc(dev);
-	unit = device_get_unit(dev);
 
 	sc->sis_self = dev;
 
 	mtx_init(&sc->sis_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
 	    MTX_DEF);
+	callout_init_mtx(&sc->sis_stat_ch, &sc->sis_mtx, 0);
 
 	if (pci_get_device(dev) == SIS_DEVICEID_900)
 		sc->sis_type = SIS_TYPE_900;
@@ -944,8 +934,10 @@ sis_attach(device_t dev)
 	pci_enable_busmaster(dev);
 
 	error = bus_alloc_resources(dev, sis_res_spec, sc->sis_res);
-	if (error)
-		return (error);
+	if (error) {
+		device_printf(dev, "couldn't allocate resources\n");
+		goto fail;
+	}
 
 	/* Reset the adapter. */
 	sis_reset(sc);
@@ -1065,12 +1057,6 @@ sis_attach(device_t dev)
 		break;
 	}
 
-	sc->sis_unit = unit;
-	if (debug_mpsafenet)
-		callout_init(&sc->sis_stat_ch, CALLOUT_MPSAFE);
-	else
-		callout_init(&sc->sis_stat_ch, 0);
-
 	/*
 	 * Allocate the parent bus DMA tag appropriate for PCI.
 	 */
@@ -1114,7 +1100,7 @@ sis_attach(device_t dev)
 	    &sc->sis_rx_dmamap);
 
 	if (error) {
-		printf("sis%d: no memory for rx list buffers!\n", unit);
+		device_printf(dev, "no memory for rx list buffers!\n");
 		bus_dma_tag_destroy(sc->sis_rx_tag);
 		sc->sis_rx_tag = NULL;
 		goto fail;
@@ -1126,7 +1112,7 @@ sis_attach(device_t dev)
 	    &sc->sis_rx_paddr, 0);
 
 	if (error) {
-		printf("sis%d: cannot get address of the rx ring!\n", unit);
+		device_printf(dev, "cannot get address of the rx ring!\n");
 		bus_dmamem_free(sc->sis_rx_tag,
 		    sc->sis_rx_list, sc->sis_rx_dmamap);
 		bus_dma_tag_destroy(sc->sis_rx_tag);
@@ -1153,7 +1139,7 @@ sis_attach(device_t dev)
 	    &sc->sis_tx_dmamap);
 
 	if (error) {
-		printf("sis%d: no memory for tx list buffers!\n", unit);
+		device_printf(dev, "no memory for tx list buffers!\n");
 		bus_dma_tag_destroy(sc->sis_tx_tag);
 		sc->sis_tx_tag = NULL;
 		goto fail;
@@ -1165,7 +1151,7 @@ sis_attach(device_t dev)
 	    &sc->sis_tx_paddr, 0);
 
 	if (error) {
-		printf("sis%d: cannot get address of the tx ring!\n", unit);
+		device_printf(dev, "cannot get address of the tx ring!\n");
 		bus_dmamem_free(sc->sis_tx_tag,
 		    sc->sis_tx_list, sc->sis_tx_dmamap);
 		bus_dma_tag_destroy(sc->sis_tx_tag);
@@ -1194,7 +1180,7 @@ sis_attach(device_t dev)
 
 	ifp = sc->sis_ifp = if_alloc(IFT_ETHER);
 	if (ifp == NULL) {
-		printf("sis%d: can not if_alloc()\n", sc->sis_unit);
+		device_printf(dev, "can not if_alloc()\n");
 		error = ENOSPC;
 		goto fail;
 	}
@@ -1216,7 +1202,7 @@ sis_attach(device_t dev)
 	 */
 	if (mii_phy_probe(dev, &sc->sis_miibus,
 	    sis_ifmedia_upd, sis_ifmedia_sts)) {
-		printf("sis%d: MII without any PHY!\n", sc->sis_unit);
+		device_printf(dev, "MII without any PHY!\n");
 		error = ENXIO;
 		goto fail;
 	}
@@ -1242,7 +1228,7 @@ sis_attach(device_t dev)
 	    sis_intr, sc, &sc->sis_intrhand);
 
 	if (error) {
-		printf("sis%d: couldn't set up irq\n", unit);
+		device_printf(dev, "couldn't set up irq\n");
 		ether_ifdetach(ifp);
 		goto fail;
 	}
@@ -1269,13 +1255,15 @@ sis_detach(device_t dev)
 
 	sc = device_get_softc(dev);
 	KASSERT(mtx_initialized(&sc->sis_mtx), ("sis mutex not initialized"));
-	SIS_LOCK(sc);
 	ifp = sc->sis_ifp;
 
 	/* These should only be active if attach succeeded. */
 	if (device_is_attached(dev)) {
+		SIS_LOCK(sc);
 		sis_reset(sc);
 		sis_stop(sc);
+		SIS_UNLOCK(sc);
+		callout_drain(&sc->sis_stat_ch);
 		ether_ifdetach(ifp);
 	}
 	if (ifp)
@@ -1307,7 +1295,6 @@ sis_detach(device_t dev)
 	if (sc->sis_tag)
 		bus_dma_tag_destroy(sc->sis_tag);
 
-	SIS_UNLOCK(sc);
 	mtx_destroy(&sc->sis_mtx);
 
 	return(0);
@@ -1560,7 +1547,7 @@ sis_tick(void *xsc)
 	struct ifnet		*ifp;
 
 	sc = xsc;
-	SIS_LOCK(sc);
+	SIS_LOCK_ASSERT(sc);
 	sc->in_tick = 1;
 	ifp = sc->sis_ifp;
 
@@ -1576,7 +1563,6 @@ sis_tick(void *xsc)
 
 	callout_reset(&sc->sis_stat_ch, hz,  sis_tick, sc);
 	sc->in_tick = 0;
-	SIS_UNLOCK(sc);
 }
 
 #ifdef DEVICE_POLLING
@@ -1905,8 +1891,8 @@ sis_initl(struct sis_softc *sc)
 
 	/* Init circular TX/RX lists. */
 	if (sis_ring_init(sc) != 0) {
-		printf("sis%d: initialization failed: no "
-			"memory for rx buffers\n", sc->sis_unit);
+		if_printf(ifp,
+		    "initialization failed: no memory for rx buffers\n");
 		sis_stop(sc);
 		return;
 	}
@@ -2079,6 +2065,7 @@ sis_ifmedia_upd(struct ifnet *ifp)
 
 	sc = ifp->if_softc;
 
+	SIS_LOCK(sc);
 	mii = device_get_softc(sc->sis_miibus);
 	sc->sis_link = 0;
 	if (mii->mii_instance) {
@@ -2087,6 +2074,7 @@ sis_ifmedia_upd(struct ifnet *ifp)
 			mii_phy_reset(miisc);
 	}
 	mii_mediachg(mii);
+	SIS_UNLOCK(sc);
 
 	return(0);
 }
@@ -2102,8 +2090,10 @@ sis_ifmedia_sts(struct ifnet *ifp, struct ifmediareq *ifmr)
 
 	sc = ifp->if_softc;
 
+	SIS_LOCK(sc);
 	mii = device_get_softc(sc->sis_miibus);
 	mii_pollstat(mii);
+	SIS_UNLOCK(sc);
 	ifmr->ifm_active = mii->mii_media_active;
 	ifmr->ifm_status = mii->mii_media_status;
 }
@@ -2118,13 +2108,13 @@ sis_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
 
 	switch(command) {
 	case SIOCSIFFLAGS:
+		SIS_LOCK(sc);
 		if (ifp->if_flags & IFF_UP) {
-			sis_init(sc);
+			sis_initl(sc);
 		} else if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
-			SIS_LOCK(sc);
 			sis_stop(sc);
-			SIS_UNLOCK(sc);
 		}
+		SIS_UNLOCK(sc);
 		error = 0;
 		break;
 	case SIOCADDMULTI:
@@ -2140,13 +2130,13 @@ sis_ioctl(struct ifnet *ifp, u_long command, caddr_t data)
 	case SIOCGIFMEDIA:
 	case SIOCSIFMEDIA:
 		mii = device_get_softc(sc->sis_miibus);
-		SIS_LOCK(sc);
 		error = ifmedia_ioctl(ifp, ifr, &mii->mii_media, command);
-		SIS_UNLOCK(sc);
 		break;
 	case SIOCSIFCAP:
+		SIS_LOCK(sc);
 		ifp->if_capenable &= ~IFCAP_POLLING;
 		ifp->if_capenable |= ifr->ifr_reqcap & IFCAP_POLLING;
+		SIS_UNLOCK(sc);
 		break;
 	default:
 		error = ether_ioctl(ifp, command, data);
@@ -2170,7 +2160,7 @@ sis_watchdog(struct ifnet *ifp)
 	}
 
 	ifp->if_oerrors++;
-	printf("sis%d: watchdog timeout\n", sc->sis_unit);
+	if_printf(ifp, "watchdog timeout\n");
 
 	sis_stop(sc);
 	sis_reset(sc);
diff --git a/sys/pci/if_sisreg.h b/sys/pci/if_sisreg.h
index d67678f..4c0a2c0 100644
--- a/sys/pci/if_sisreg.h
+++ b/sys/pci/if_sisreg.h
@@ -435,7 +435,6 @@ struct sis_softc {
 	void			*sis_intrhand;
 	device_t		sis_self;
 	device_t		sis_miibus;
-	u_int8_t		sis_unit;
 	u_int8_t		sis_type;
 	u_int8_t		sis_rev;
 	u_int8_t		sis_link;