From d0b9b8460db41f0b5895f15cab33115a5e6d2480 Mon Sep 17 00:00:00 2001 From: yongari Date: Tue, 13 Dec 2011 18:11:25 +0000 Subject: 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 --- sys/dev/bce/if_bce.c | 107 ++++++++++++++++++++++++------------------------ sys/dev/bce/if_bcereg.h | 1 + 2 files changed, 55 insertions(+), 53 deletions(-) (limited to 'sys/dev/bce') 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; -- cgit v1.1