From 036ec4b1ff6b99a6f3c3df922a4eb744d68e1c62 Mon Sep 17 00:00:00 2001 From: yongari Date: Tue, 22 Dec 2009 20:11:56 +0000 Subject: Reimplement miibus_statchg method. Don't rely on link state change interrupt. If we want to use link state change interrupt ste(4) should also implement auto-negotiation complete handler as well as various PHY access handling. Now link state change is handled by mii(4) polling so it will automatically update link state UP/DOWN events which in turn make ste(4) usable with lagg(4). r199559 added a private timer to drive watchdog and the timer also used to drive MAC statistics update. Because the MAC statistics update is called whenever statistics counter reaches near-full, it drove watchdog timer too fast such that it caused false watchdog timeouts under heavy TX traffic conditions. Fix the regression by separating ste_stats_update() from driving watchdog timer and introduce a new function ste_tick() that handles periodic job such as driving watchdog, MAC statistics update and link state check etc. While I'm here clear armed watchdog timer in ste_stop(). --- sys/dev/ste/if_ste.c | 123 +++++++++++++++++++++++++++++------------------- sys/dev/ste/if_stereg.h | 7 ++- 2 files changed, 78 insertions(+), 52 deletions(-) diff --git a/sys/dev/ste/if_ste.c b/sys/dev/ste/if_ste.c index 94d41eb..5eec38a 100644 --- a/sys/dev/ste/if_ste.c +++ b/sys/dev/ste/if_ste.c @@ -124,8 +124,9 @@ static int ste_rxeof(struct ste_softc *, int); static void ste_setmulti(struct ste_softc *); static void ste_start(struct ifnet *); static void ste_start_locked(struct ifnet *); -static void ste_stats_update(void *); +static void ste_stats_update(struct ste_softc *); static void ste_stop(struct ste_softc *); +static void ste_tick(void *); static void ste_txeoc(struct ste_softc *); static void ste_txeof(struct ste_softc *); static void ste_wait(struct ste_softc *); @@ -404,15 +405,49 @@ ste_miibus_statchg(device_t dev) { struct ste_softc *sc; struct mii_data *mii; + struct ifnet *ifp; + uint16_t cfg; sc = device_get_softc(dev); mii = device_get_softc(sc->ste_miibus); + ifp = sc->ste_ifp; + if (mii == NULL || ifp == NULL || + (ifp->if_drv_flags & IFF_DRV_RUNNING) == 0) + return; - if ((mii->mii_media_active & IFM_GMASK) == IFM_FDX) { - STE_SETBIT2(sc, STE_MACCTL0, STE_MACCTL0_FULLDUPLEX); - } else { - STE_CLRBIT2(sc, STE_MACCTL0, STE_MACCTL0_FULLDUPLEX); + sc->ste_flags &= ~STE_FLAG_LINK; + if ((mii->mii_media_status & (IFM_ACTIVE | IFM_AVALID)) == + (IFM_ACTIVE | IFM_AVALID)) { + switch (IFM_SUBTYPE(mii->mii_media_active)) { + case IFM_10_T: + case IFM_100_TX: + case IFM_100_FX: + case IFM_100_T4: + sc->ste_flags |= STE_FLAG_LINK; + default: + break; + } + } + + /* Program MACs with resolved speed/duplex/flow-control. */ + if ((sc->ste_flags & STE_FLAG_LINK) != 0) { + cfg = CSR_READ_2(sc, STE_MACCTL0); + cfg &= ~(STE_MACCTL0_FLOWCTL_ENABLE | STE_MACCTL0_FULLDUPLEX); + if ((IFM_OPTIONS(mii->mii_media_active) & IFM_FDX) != 0) { + /* + * ST201 data sheet says driver should enable receiving + * MAC control frames bit of receive mode register to + * receive flow-control frames but the register has no + * such bits. In addition the controller has no ability + * to send pause frames so it should be handled in + * driver. Implementing pause timer handling in driver + * layer is not trivial, so don't enable flow-control + * here. + */ + cfg |= STE_MACCTL0_FULLDUPLEX; + } + CSR_WRITE_2(sc, STE_MACCTL0, cfg); } } @@ -613,13 +648,8 @@ ste_poll_locked(struct ifnet *ifp, enum poll_cmd cmd, int count) if (status & STE_ISR_TX_DONE) ste_txeoc(sc); - if (status & STE_ISR_STATS_OFLOW) { - callout_stop(&sc->ste_stat_callout); + if (status & STE_ISR_STATS_OFLOW) ste_stats_update(sc); - } - - if (status & STE_ISR_LINKEVENT) - mii_pollstat(device_get_softc(sc->ste_miibus)); if (status & STE_ISR_HOSTERR) { ste_reset(sc); @@ -669,14 +699,8 @@ ste_intr(void *xsc) if (status & STE_ISR_TX_DONE) ste_txeoc(sc); - if (status & STE_ISR_STATS_OFLOW) { - callout_stop(&sc->ste_stat_callout); + if (status & STE_ISR_STATS_OFLOW) ste_stats_update(sc); - } - - if (status & STE_ISR_LINKEVENT) - mii_pollstat(device_get_softc(sc->ste_miibus)); - if (status & STE_ISR_HOSTERR) { ste_reset(sc); @@ -813,6 +837,30 @@ ste_txeoc(struct ste_softc *sc) } static void +ste_tick(void *arg) +{ + struct ste_softc *sc; + struct mii_data *mii; + + sc = (struct ste_softc *)arg; + + STE_LOCK_ASSERT(sc); + + mii = device_get_softc(sc->ste_miibus); + mii_tick(mii); + /* + * ukphy(4) does not seem to generate CB that reports + * resolved link state so if we know we lost a link, + * explicitly check the link state. + */ + if ((sc->ste_flags & STE_FLAG_LINK) == 0) + ste_miibus_statchg(sc->ste_dev); + ste_stats_update(sc); + ste_watchdog(sc); + callout_reset(&sc->ste_callout, hz, ste_tick, sc); +} + +static void ste_txeof(struct ste_softc *sc) { struct ifnet *ifp; @@ -855,43 +903,18 @@ ste_txeof(struct ste_softc *sc) } static void -ste_stats_update(void *xsc) +ste_stats_update(struct ste_softc *sc) { - struct ste_softc *sc; struct ifnet *ifp; - struct mii_data *mii; - sc = xsc; STE_LOCK_ASSERT(sc); ifp = sc->ste_ifp; - mii = device_get_softc(sc->ste_miibus); - ifp->if_collisions += CSR_READ_1(sc, STE_LATE_COLLS) + CSR_READ_1(sc, STE_MULTI_COLLS) + CSR_READ_1(sc, STE_SINGLE_COLLS); - - if ((sc->ste_flags & STE_FLAG_LINK) ==0) { - mii_pollstat(mii); - if (mii->mii_media_status & IFM_ACTIVE && - IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE) { - sc->ste_flags |= STE_FLAG_LINK; - /* - * we don't get a call-back on re-init so do it - * otherwise we get stuck in the wrong link state - */ - ste_miibus_statchg(sc->ste_dev); - if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd)) - ste_start_locked(ifp); - } - } - - if (sc->ste_timer > 0 && --sc->ste_timer == 0) - ste_watchdog(sc); - callout_reset(&sc->ste_stat_callout, hz, ste_stats_update, sc); } - /* * Probe for a Sundance ST201 chip. Check the PCI vendor and device * IDs against our list and return a device name if we find a match. @@ -970,7 +993,7 @@ ste_attach(device_t dev) goto fail; } - callout_init_mtx(&sc->ste_stat_callout, &sc->ste_mtx, 0); + callout_init_mtx(&sc->ste_callout, &sc->ste_mtx, 0); /* Reset the adapter. */ ste_reset(sc); @@ -1076,7 +1099,7 @@ ste_detach(device_t dev) STE_LOCK(sc); ste_stop(sc); STE_UNLOCK(sc); - callout_drain(&sc->ste_stat_callout); + callout_drain(&sc->ste_callout); } if (sc->ste_miibus) device_delete_child(dev, sc->ste_miibus); @@ -1601,7 +1624,7 @@ ste_init_locked(struct ste_softc *sc) ifp->if_drv_flags |= IFF_DRV_RUNNING; ifp->if_drv_flags &= ~IFF_DRV_OACTIVE; - callout_reset(&sc->ste_stat_callout, hz, ste_stats_update, sc); + callout_reset(&sc->ste_callout, hz, ste_tick, sc); } static void @@ -1615,7 +1638,8 @@ ste_stop(struct ste_softc *sc) STE_LOCK_ASSERT(sc); ifp = sc->ste_ifp; - callout_stop(&sc->ste_stat_callout); + callout_stop(&sc->ste_callout); + sc->ste_timer = 0; ifp->if_drv_flags &= ~(IFF_DRV_RUNNING|IFF_DRV_OACTIVE); CSR_WRITE_2(sc, STE_IMR, 0); @@ -1914,6 +1938,9 @@ ste_watchdog(struct ste_softc *sc) ifp = sc->ste_ifp; STE_LOCK_ASSERT(sc); + if (sc->ste_timer == 0 || --sc->ste_timer) + return; + ifp->if_oerrors++; if_printf(ifp, "watchdog timeout\n"); diff --git a/sys/dev/ste/if_stereg.h b/sys/dev/ste/if_stereg.h index 79fc407..f05e555 100644 --- a/sys/dev/ste/if_stereg.h +++ b/sys/dev/ste/if_stereg.h @@ -276,10 +276,9 @@ #define STE_IMR_TX_DMADONE 0x0200 #define STE_IMR_RX_DMADONE 0x0400 -#define STE_INTRS \ +#define STE_INTRS \ (STE_IMR_RX_DMADONE|STE_IMR_TX_DMADONE| \ - STE_IMR_TX_DONE|STE_IMR_HOSTERR| \ - STE_IMR_LINKEVENT) + STE_IMR_TX_DONE|STE_IMR_HOSTERR) #define STE_ISR_INTLATCH 0x0001 #define STE_ISR_HOSTERR 0x0002 @@ -561,7 +560,7 @@ struct ste_softc { int ste_timer; struct ste_list_data ste_ldata; struct ste_chain_data ste_cdata; - struct callout ste_stat_callout; + struct callout ste_callout; struct mtx ste_mtx; }; -- cgit v1.1