Rework link state tracking and remove superfluous link UP/DOWN

messages.
 o Add check for actually resolved speed in miibus_statchg callback
   instead of blindly reprogramming BCE_EMAC_MODE register.  The
   callback may be called multiple times(e.g. link UP, link
   transition, auto-negotiate complete etc) while auto-negotiation
   is in progress.  All unresolved link state changes are ignored
   now and setting BCE_EMAC_MODE after link establishment is done
   once.
 o bce(4) is careful enough not to drive MII_TICK if driver got a
   valid link.  To detect lost link, bce(4) relied on link state
   change interrupt and if driver see the interrupt, it forced to
   drive MII_TICK by calling bce_tick() in interrupt handler.
   Because bce(4) generates multiple link state change interrupts
   while auto-negotiation is in progress, bce_tick() would be
   called multiple times and this resulted in generating multiple
   link UP/DOWN messages.
   With this change, bce_tick() is not called in interrupt handler
   anymore such that miibus_statchg callback handles link state
   changes with consistent manner.

Reviewed by:	davidch

2 files changed, 55 insertions, 53 deletions

diff --git a/sys/dev/bce/if_bce.c b/sys/dev/bce/if_bce.c
index e0014c8..72ba76a 100644
--- a/sys/dev/bce/if_bce.c
+++ b/sys/dev/bce/if_bce.c
@@ -1982,6 +1982,7 @@ static void
 bce_miibus_statchg(device_t dev)
 {
 	struct bce_softc *sc;
+	struct ifnet *ifp;
 	struct mii_data *mii;
 	int val;
 
@@ -1989,42 +1990,57 @@ bce_miibus_statchg(device_t dev)
 
 	DBENTER(BCE_VERBOSE_PHY);
 
+	ifp = sc->bce_ifp;
 	mii = device_get_softc(sc->bce_miibus);
+	if (mii == NULL || ifp == NULL ||
+	    (ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
+		return;
 
+	sc->bce_link_up = FALSE;
 	val = REG_RD(sc, BCE_EMAC_MODE);
 	val &= ~(BCE_EMAC_MODE_PORT | BCE_EMAC_MODE_HALF_DUPLEX |
 	    BCE_EMAC_MODE_MAC_LOOP | BCE_EMAC_MODE_FORCE_LINK |
 	    BCE_EMAC_MODE_25G);
 
 	/* Set MII or GMII interface based on the PHY speed. */
-	switch (IFM_SUBTYPE(mii->mii_media_active)) {
-	case IFM_10_T:
-		if (BCE_CHIP_NUM(sc) != BCE_CHIP_NUM_5706) {
-			DBPRINT(sc, BCE_INFO_PHY,
-			    "Enabling 10Mb interface.\n");
-			val |= BCE_EMAC_MODE_PORT_MII_10;
+	if ((mii->mii_media_status & (IFM_ACTIVE | IFM_AVALID)) ==
+	    (IFM_ACTIVE | IFM_AVALID)) {
+		switch (IFM_SUBTYPE(mii->mii_media_active)) {
+		case IFM_10_T:
+			if (BCE_CHIP_NUM(sc) != BCE_CHIP_NUM_5706) {
+				DBPRINT(sc, BCE_INFO_PHY,
+				    "Enabling 10Mb interface.\n");
+				val |= BCE_EMAC_MODE_PORT_MII_10;
+				sc->bce_link_up = TRUE;
+				break;
+			}
+			/* FALLTHROUGH */
+		case IFM_100_TX:
+			DBPRINT(sc, BCE_INFO_PHY, "Enabling MII interface.\n");
+			val |= BCE_EMAC_MODE_PORT_MII;
+			sc->bce_link_up = TRUE;
+			break;
+		case IFM_2500_SX:
+			DBPRINT(sc, BCE_INFO_PHY, "Enabling 2.5G MAC mode.\n");
+			val |= BCE_EMAC_MODE_25G;
+			/* FALLTHROUGH */
+		case IFM_1000_T:
+		case IFM_1000_SX:
+			DBPRINT(sc, BCE_INFO_PHY, "Enabling GMII interface.\n");
+			val |= BCE_EMAC_MODE_PORT_GMII;
+			sc->bce_link_up = TRUE;
+			if (bce_verbose || bootverbose)
+				BCE_PRINTF("Gigabit link up!\n");
+			break;
+		default:
+			DBPRINT(sc, BCE_INFO_PHY, "Unknown link speed.\n");
 			break;
 		}
-		/* fall-through */
-	case IFM_100_TX:
-		DBPRINT(sc, BCE_INFO_PHY, "Enabling MII interface.\n");
-		val |= BCE_EMAC_MODE_PORT_MII;
-		break;
-	case IFM_2500_SX:
-		DBPRINT(sc, BCE_INFO_PHY, "Enabling 2.5G MAC mode.\n");
-		val |= BCE_EMAC_MODE_25G;
-		/* fall-through */
-	case IFM_1000_T:
-	case IFM_1000_SX:
-		DBPRINT(sc, BCE_INFO_PHY, "Enabling GMII interface.\n");
-		val |= BCE_EMAC_MODE_PORT_GMII;
-		break;
-	default:
-		DBPRINT(sc, BCE_INFO_PHY, "Unknown link speed, enabling "
-		    "default GMII interface.\n");
-		val |= BCE_EMAC_MODE_PORT_GMII;
 	}
 
+	if (sc->bce_link_up == FALSE)
+		return;
+
 	/* Set half or full duplex based on PHY settings. */
 	if ((mii->mii_media_active & IFM_GMASK) == IFM_HDX) {
 		DBPRINT(sc, BCE_INFO_PHY,
@@ -2036,7 +2052,7 @@ bce_miibus_statchg(device_t dev)
 
 	REG_WR(sc, BCE_EMAC_MODE, val);
 
- 	if ((mii->mii_media_active & IFM_ETH_RXPAUSE) != 0) {
+	if ((mii->mii_media_active & IFM_ETH_RXPAUSE) != 0) {
 		DBPRINT(sc, BCE_INFO_PHY,
 		    "%s(): Enabling RX flow control.\n", __FUNCTION__);
 		BCE_SETBIT(sc, BCE_EMAC_RX_MODE, BCE_EMAC_RX_MODE_FLOW_EN);
@@ -2046,7 +2062,7 @@ bce_miibus_statchg(device_t dev)
 		BCE_CLRBIT(sc, BCE_EMAC_RX_MODE, BCE_EMAC_RX_MODE_FLOW_EN);
 	}
 
- 	if ((mii->mii_media_active & IFM_ETH_TXPAUSE) != 0) {
+	if ((mii->mii_media_active & IFM_ETH_TXPAUSE) != 0) {
 		DBPRINT(sc, BCE_INFO_PHY,
 		    "%s(): Enabling TX flow control.\n", __FUNCTION__);
 		BCE_SETBIT(sc, BCE_EMAC_TX_MODE, BCE_EMAC_TX_MODE_FLOW_EN);
@@ -6206,15 +6222,11 @@ bce_phy_intr(struct bce_softc *sc)
 			DBPRINT(sc, BCE_INFO_PHY, "%s(): Link is now DOWN.\n",
 			    __FUNCTION__);
 		}
-
 		/*
-		 * Assume link is down and allow
-		 * tick routine to update the state
-		 * based on the actual media state.
+		 * Link state changed, allow tick routine to update
+		 * the state baased on actual media state.
 		 */
-		sc->bce_link_up = FALSE;
-		callout_stop(&sc->bce_tick_callout);
-		bce_tick(sc);
+		sc->bce_link_tick = TRUE;
 	}
 
 	/* Acknowledge the link change interrupt. */
@@ -6898,12 +6910,13 @@ bce_init_locked(struct bce_softc *sc)
 	/* Enable host interrupts. */
 	bce_enable_intr(sc, 1);
 
-	bce_ifmedia_upd_locked(ifp);
-
 	/* Let the OS know the driver is up and running. */
 	ifp->if_drv_flags |= IFF_DRV_RUNNING;
 	ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 
+	sc->bce_link_tick = TRUE;
+	bce_ifmedia_upd_locked(ifp);
+
 	callout_reset(&sc->bce_tick_callout, hz, bce_tick, sc);
 
 bce_init_locked_exit:
@@ -8199,31 +8212,19 @@ bce_tick(void *xsc)
 	bce_watchdog(sc);
 
 	/* If link is up already up then we're done. */
-	if (sc->bce_link_up == TRUE)
+	if (sc->bce_link_tick == FALSE && sc->bce_link_up == TRUE)
 		goto bce_tick_exit;
 
 	/* Link is down.  Check what the PHY's doing. */
 	mii = device_get_softc(sc->bce_miibus);
 	mii_tick(mii);
 
-	/* Check if the link has come up. */
-	if ((mii->mii_media_status & IFM_ACTIVE) &&
-	    (IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)) {
+	sc->bce_link_tick = FALSE;
+	/* Now that link is up, handle any outstanding TX traffic. */
+	if (sc->bce_link_up == TRUE && !IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
 		DBPRINT(sc, BCE_VERBOSE_MISC,
-		    "%s(): Link up!\n", __FUNCTION__);
-		sc->bce_link_up = TRUE;
-		if ((IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_T ||
-		    IFM_SUBTYPE(mii->mii_media_active) == IFM_1000_SX ||
-		    IFM_SUBTYPE(mii->mii_media_active) == IFM_2500_SX) &&
-		    (bce_verbose || bootverbose))
-			BCE_PRINTF("Gigabit link up!\n");
-
-		/* Now that link is up, handle any outstanding TX traffic. */
-		if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) {
-			DBPRINT(sc, BCE_VERBOSE_MISC, "%s(): Found "
-			    "pending TX traffic.\n", __FUNCTION__);
-			bce_start_locked(ifp);
-		}
+		    "%s(): Found pending TX traffic.\n", __FUNCTION__);
+		bce_start_locked(ifp);
 	}
 
 bce_tick_exit:
diff --git a/sys/dev/bce/if_bcereg.h b/sys/dev/bce/if_bcereg.h
index f74c4d2..65a3424 100644
--- a/sys/dev/bce/if_bcereg.h
+++ b/sys/dev/bce/if_bcereg.h
@@ -6560,6 +6560,7 @@ struct bce_softc
 	u16			pg_prod;
 	u16			pg_cons;
 
+	int			bce_link_tick;
 	int			bce_link_up;
 	struct		callout bce_tick_callout;
 	struct		callout bce_pulse_callout;