[Fwd: Re: bge Ierr rate increase from 5.3R -> 6.1R]
Oleg Bulyzhin
oleg at freebsd.org
Sat Jan 13 02:14:59 PST 2007
On Tue, Jan 09, 2007 at 04:14:16PM -0800, John Polstra wrote:
> I've been using #2 today, and it's working well so far. I
> implemented it like this. (Ignore the version numbers; I'm working
> in a private repository.)
>
> --- if_bge.c 8 Jan 2007 22:46:51 -0000 1.26
> +++ if_bge.c 9 Jan 2007 22:52:43 -0000 1.27
> @@ -3122,8 +3122,8 @@ bge_tick(void *xsc)
>
> if ((sc->bge_flags & BGE_FLAG_TBI) == 0) {
> mii = device_get_softc(sc->bge_miibus);
> - /* Don't mess with the PHY in IPMI/ASF mode */
> - if (!((sc->bge_asf_mode & ASF_STACKUP) && (sc->bge_link)))
> + /* Don't mess with the PHY unless link is down. */
> + if (!sc->bge_link)
> mii_tick(mii);
> } else {
> /*
>
>
> John
Could you please test attached patch? It should fix ierrs issue and should not
break link events (would be fine to test it: unplug/plug cable, try to change
media with ifconfig, change media on other end of wire).
--
Oleg.
================================================================
=== Oleg Bulyzhin -- OBUL-RIPN -- OBUL-RIPE -- oleg at rinet.ru ===
================================================================
-------------- next part --------------
Index: if_bgereg.h
===================================================================
RCS file: /home/ncvs/src/sys/dev/bge/if_bgereg.h,v
retrieving revision 1.66
diff -u -r1.66 if_bgereg.h
--- if_bgereg.h 11 Jan 2007 01:43:24 -0000 1.66
+++ if_bgereg.h 13 Jan 2007 10:05:31 -0000
@@ -2479,8 +2479,13 @@
uint32_t bge_tx_buf_ratio;
int bge_if_flags;
int bge_txcnt;
- int bge_link; /* link state */
- int bge_link_evt; /* pending link event */
+ uint32_t bge_sts;
+#define BGE_STS_LINK 0x00000001 /* MAC link status */
+#define BGE_STS_LINK_EVT 0x00000002 /* pending link event */
+#define BGE_STS_AUTOPOLL 0x00000004 /* PHY auto-polling */
+#define BGE_STS_BIT(sc, x) ((sc)->bge_sts & (x))
+#define BGE_STS_SETBIT(sc, x) ((sc)->bge_sts |= (x))
+#define BGE_STS_CLRBIT(sc, x) ((sc)->bge_sts &= ~(x))
int bge_timer;
struct callout bge_stat_ch;
uint32_t bge_rx_discards;
Index: if_bge.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/bge/if_bge.c,v
retrieving revision 1.172
diff -u -r1.172 if_bge.c
--- if_bge.c 26 Dec 2006 18:33:55 -0000 1.172
+++ if_bge.c 13 Jan 2007 10:05:33 -0000
@@ -590,6 +590,7 @@
/* Reading with autopolling on may trigger PCI errors */
autopoll = CSR_READ_4(sc, BGE_MI_MODE);
if (autopoll & BGE_MIMODE_AUTOPOLL) {
+ BGE_STS_CLRBIT(sc, BGE_STS_AUTOPOLL);
BGE_CLRBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL);
DELAY(40);
}
@@ -613,6 +614,7 @@
done:
if (autopoll & BGE_MIMODE_AUTOPOLL) {
+ BGE_STS_SETBIT(sc, BGE_STS_AUTOPOLL);
BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL);
DELAY(40);
}
@@ -635,6 +637,7 @@
/* Reading with autopolling on may trigger PCI errors */
autopoll = CSR_READ_4(sc, BGE_MI_MODE);
if (autopoll & BGE_MIMODE_AUTOPOLL) {
+ BGE_STS_CLRBIT(sc, BGE_STS_AUTOPOLL);
BGE_CLRBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL);
DELAY(40);
}
@@ -648,6 +651,7 @@
}
if (autopoll & BGE_MIMODE_AUTOPOLL) {
+ BGE_STS_SETBIT(sc, BGE_STS_AUTOPOLL);
BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL);
DELAY(40);
}
@@ -1588,6 +1592,7 @@
if (sc->bge_flags & BGE_FLAG_TBI) {
CSR_WRITE_4(sc, BGE_MI_STS, BGE_MISTS_LINK);
} else {
+ BGE_STS_SETBIT(sc, BGE_STS_AUTOPOLL);
BGE_SETBIT(sc, BGE_MI_MODE, BGE_MIMODE_AUTOPOLL|10<<16);
if (sc->bge_asicrev == BGE_ASICREV_BCM5700 &&
sc->bge_chipid != BGE_CHIPID_BCM5700_B2)
@@ -2992,12 +2997,13 @@
/* Note link event. It will be processed by POLL_AND_CHECK_STATUS cmd */
if (statusword & BGE_STATFLAG_LINKSTATE_CHANGED)
- sc->bge_link_evt++;
+ BGE_STS_SETBIT(sc, BGE_STS_LINK_EVT);
if (cmd == POLL_AND_CHECK_STATUS)
if ((sc->bge_asicrev == BGE_ASICREV_BCM5700 &&
sc->bge_chipid != BGE_CHIPID_BCM5700_B2) ||
- sc->bge_link_evt || (sc->bge_flags & BGE_FLAG_TBI))
+ BGE_STS_BIT(sc, BGE_STS_LINK_EVT) ||
+ (sc->bge_flags & BGE_FLAG_TBI))
bge_link_upd(sc);
sc->rxcycles = count;
@@ -3065,7 +3071,7 @@
if ((sc->bge_asicrev == BGE_ASICREV_BCM5700 &&
sc->bge_chipid != BGE_CHIPID_BCM5700_B2) ||
- statusword || sc->bge_link_evt)
+ statusword || BGE_STS_BIT(sc, BGE_STS_LINK_EVT))
bge_link_upd(sc);
if (ifp->if_drv_flags & IFF_DRV_RUNNING) {
@@ -3121,10 +3127,15 @@
bge_stats_update(sc);
if ((sc->bge_flags & BGE_FLAG_TBI) == 0) {
- mii = device_get_softc(sc->bge_miibus);
- /* Don't mess with the PHY in IPMI/ASF mode */
- if (!((sc->bge_asf_mode & ASF_STACKUP) && (sc->bge_link)))
+ /*
+ * Do not touch PHY if we have link up. This could break
+ * IPMI/ASF mode or produce extra input errors.
+ * (extra input errors was reported for bcm5701 & bcm5704).
+ */
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK)) {
+ mii = device_get_softc(sc->bge_miibus);
mii_tick(mii);
+ }
} else {
/*
* Since in TBI mode auto-polling can't be used we should poll
@@ -3136,7 +3147,7 @@
if (!(sc->bge_ifp->if_capenable & IFCAP_POLLING))
#endif
{
- sc->bge_link_evt++;
+ BGE_STS_SETBIT(sc, BGE_STS_LINK_EVT);
BGE_SETBIT(sc, BGE_MISC_LOCAL_CTL, BGE_MLC_INTR_SET);
}
}
@@ -3352,7 +3363,7 @@
sc = ifp->if_softc;
- if (!sc->bge_link || IFQ_DRV_IS_EMPTY(&ifp->if_snd))
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK) || IFQ_DRV_IS_EMPTY(&ifp->if_snd))
return;
prodidx = sc->bge_tx_prodidx;
@@ -3633,7 +3644,7 @@
return (0);
}
- sc->bge_link_evt++;
+ BGE_STS_SETBIT(sc, BGE_STS_LINK_EVT);
mii = device_get_softc(sc->bge_miibus);
if (mii->mii_instance) {
struct mii_softc *miisc;
@@ -3935,9 +3946,9 @@
sc->bge_tx_saved_considx = BGE_TXCONS_UNSET;
/* Clear MAC's link state (PHY may still have link UP). */
- if (bootverbose && sc->bge_link)
+ if (bootverbose && BGE_STS_BIT(sc, BGE_STS_LINK))
if_printf(sc->bge_ifp, "link DOWN\n");
- sc->bge_link = 0;
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK);
ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
}
@@ -3995,12 +4006,12 @@
bge_link_upd(struct bge_softc *sc)
{
struct mii_data *mii;
- uint32_t link, status;
+ uint32_t status;
BGE_LOCK_ASSERT(sc);
/* Clear 'pending link event' flag. */
- sc->bge_link_evt = 0;
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK_EVT);
/*
* Process link state changes.
@@ -4023,16 +4034,16 @@
if (status & BGE_MACSTAT_MI_INTERRUPT) {
mii = device_get_softc(sc->bge_miibus);
mii_pollstat(mii);
- if (!sc->bge_link &&
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK) &&
mii->mii_media_status & IFM_ACTIVE &&
IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE) {
- sc->bge_link++;
+ BGE_STS_SETBIT(sc, BGE_STS_LINK);
if (bootverbose)
if_printf(sc->bge_ifp, "link UP\n");
- } else if (sc->bge_link &&
+ } else if (BGE_STS_BIT(sc, BGE_STS_LINK) &&
(!(mii->mii_media_status & IFM_ACTIVE) ||
IFM_SUBTYPE(mii->mii_media_active) == IFM_NONE)) {
- sc->bge_link = 0;
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK);
if (bootverbose)
if_printf(sc->bge_ifp, "link DOWN\n");
}
@@ -4050,8 +4061,8 @@
if (sc->bge_flags & BGE_FLAG_TBI) {
status = CSR_READ_4(sc, BGE_MAC_STS);
if (status & BGE_MACSTAT_TBI_PCS_SYNCHED) {
- if (!sc->bge_link) {
- sc->bge_link++;
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK)) {
+ BGE_STS_SETBIT(sc, BGE_STS_LINK);
if (sc->bge_asicrev == BGE_ASICREV_BCM5704)
BGE_CLRBIT(sc, BGE_MAC_MODE,
BGE_MACMODE_TBI_SEND_CFGS);
@@ -4061,40 +4072,38 @@
if_link_state_change(sc->bge_ifp,
LINK_STATE_UP);
}
- } else if (sc->bge_link) {
- sc->bge_link = 0;
+ } else if (BGE_STS_BIT(sc, BGE_STS_LINK)) {
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK);
if (bootverbose)
if_printf(sc->bge_ifp, "link DOWN\n");
if_link_state_change(sc->bge_ifp, LINK_STATE_DOWN);
}
- /* Discard link events for MII/GMII cards if MI auto-polling disabled */
- } else if (CSR_READ_4(sc, BGE_MI_MODE) & BGE_MIMODE_AUTOPOLL) {
- /*
- * Some broken BCM chips have BGE_STATFLAG_LINKSTATE_CHANGED bit
- * in status word always set. Workaround this bug by reading
- * PHY link status directly.
- */
- link = (CSR_READ_4(sc, BGE_MI_STS) & BGE_MISTS_LINK) ? 1 : 0;
+ /*
+ * Discard link events for MII/GMII cards if MI auto-polling disabled.
+ * This should not happen since mii callouts are locked now, but
+ * we keep this check for debug.
+ */
+ } else if (BGE_STS_BIT(sc, BGE_STS_AUTOPOLL)) {
+ mii = device_get_softc(sc->bge_miibus);
+ mii_pollstat(mii);
- if (link != sc->bge_link ||
- sc->bge_asicrev == BGE_ASICREV_BCM5700) {
- mii = device_get_softc(sc->bge_miibus);
- mii_pollstat(mii);
- if (!sc->bge_link &&
- mii->mii_media_status & IFM_ACTIVE &&
- IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE) {
- sc->bge_link++;
- if (bootverbose)
- if_printf(sc->bge_ifp, "link UP\n");
- } else if (sc->bge_link &&
- (!(mii->mii_media_status & IFM_ACTIVE) ||
- IFM_SUBTYPE(mii->mii_media_active) == IFM_NONE)) {
- sc->bge_link = 0;
- if (bootverbose)
- if_printf(sc->bge_ifp, "link DOWN\n");
- }
+ if (!BGE_STS_BIT(sc, BGE_STS_LINK) &&
+ mii->mii_media_status & IFM_ACTIVE &&
+ IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE) {
+ BGE_STS_SETBIT(sc, BGE_STS_LINK);
+ if (bootverbose)
+ if_printf(sc->bge_ifp, "link UP\n");
+ } else if (BGE_STS_BIT(sc, BGE_STS_LINK) &&
+ (!(mii->mii_media_status & IFM_ACTIVE) ||
+ IFM_SUBTYPE(mii->mii_media_active) == IFM_NONE)) {
+ BGE_STS_CLRBIT(sc, BGE_STS_LINK);
+ if (bootverbose)
+ if_printf(sc->bge_ifp, "link DOWN\n");
}
- }
+ } else
+ /* Should not happen, see above. */
+ if_printf(sc->bge_ifp,
+ "link event discarded: PHY auto-polling is off.\n");
/* Clear the attention. */
CSR_WRITE_4(sc, BGE_MAC_STS, BGE_MACSTAT_SYNC_CHANGED|
More information about the freebsd-net
mailing list