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 -- [EMAIL PROTECTED] ===
================================================================

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|
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to