Re: Fix for kern/20700

[buhrow%lothlorien.nfbcal.org@localhost (Brian Buhrow)]

        Hello.  After working on this problem for a while, I propose
committing the diff below as a fix for this bug.  The problem of
negotiating link at a speed other than the 1000-base-T remains, but see
below for my reasoning on this topic.

        the current state of affairs is that Nway autonegotiation works for
twisted pair copper connections.  Nailed up speed requests work with 10 and
100mbit connections, but will never work with 1000-base-T connections,
unless the chip has initialized, and hence negotiated, a connection before
ifconfig is called in the boot sequence.

        What this patch does is turn on Nway autonegotiation when the user
requests 1000baset with ifconfig and turn off advertisements for speeds
other than 1000baset in the Nway autonegotiation process.  This makes 
1000baset nailed links work, which, in my view, is the behavior of least
astonishment.

        Because of the race between Nway negotiation and parallel detection,
which Thor alluded to earlier in this thread, I've found it very difficult
to prevent the chip from establishing link with its partners at speeds
other than the requested speed without totally disabling the ability for
the link to come up again on its own once the proper speeds and duplexes
were configured on both ends of the link.  I experimented with disallowing
the interface from passing traffic if the link speed didn't match the
requested speed, but that seemed even less intuitive, since ifconfig would
show the interface as being active, but wouldn't actually work.  Finally,
I figured out that I could probably implement the requested behavior to fix
this bug completely, but to do so would require that I touch every phy
driver in the system, creating a huge patch and a larger testing problem.

        In summary, in my view, this patch improves the state of the world,
permitting more configuration cases to work than do today, as well as
causing the system to behave in less astonishing ways than it does today.
Right now, if you get a link at 1000baset when the system boots, and you
don't have autonegotiation configured, if you pull the plug or otherwise
bounce the port, you have to reboot or turn autonegotiation on to
re-establish link.  this patch insures that you'll get a restoration of
service, regardless of how you set ifconfig, without user intervention.

        If folks don't strongly object, I'll commit this patch in the next
week or so and request a pullup for the 5.x branch.  If someone has an idea
for fully implementing the requested behavior without having to touch all
the phy drivers, I'm very interested and will be happy to implement and
test.

-thanks
-Brian

P.S.  This patch applies cleanly to 4.x, 5.x and -current source trees.


--- mii_physubr.c.40    2007-11-15 20:14:47.000000000 -0800
+++ mii_physubr.c       2011-12-07 10:01:39.000000000 -0800
@@ -170,15 +170,21 @@
                bmcr |= BMCR_LOOP;
 
        PHY_WRITE(sc, MII_ANAR, anar);
-       PHY_WRITE(sc, MII_BMCR, bmcr);
        if (sc->mii_flags & MIIF_HAVE_GTCR)
                PHY_WRITE(sc, MII_100T2CR, gtcr);
+       if (IFM_SUBTYPE(ife->ifm_media) == IFM_1000_T) {
+               mii_phy_auto(sc, 0);
+       } else {
+               PHY_WRITE(sc, MII_BMCR, bmcr);
+       }
 }
 
 int
 mii_phy_auto(struct mii_softc *sc, int waitfor)
 {
        int i;
+       struct mii_data *mii = sc->mii_pdata;
+       struct ifmedia_entry *ife = mii->mii_media.ifm_cur;
 
        if ((sc->mii_flags & MIIF_DOINGAUTO) == 0) {
                /*
@@ -212,6 +218,16 @@
                                     (EXTSR_1000THDX|EXTSR_1000TFDX)))
                                        anar |= ANAR_X_PAUSE_ASYM;
                        }
+
+                       /*
+                        *for 1000-base-T, autonegotiation mus be enabled, but 
+                        *if we're not set to auto, only advertise
+                        *1000-base-T with the link partner.
+                        */
+                       if (IFM_SUBTYPE(ife->ifm_media) == IFM_1000_T) {
+                               anar &= 
~(ANAR_T4|ANAR_TX_FD|ANAR_TX|ANAR_10_FD|ANAR_10);
+                       }
+                               
                        PHY_WRITE(sc, MII_ANAR, anar);
                        if (sc->mii_flags & MIIF_HAVE_GTCR) {
                                uint16_t gtcr = 0;
@@ -294,7 +310,8 @@
         * status so we can generate an announcement if the status
         * changes.
         */
-       if (IFM_SUBTYPE(ife->ifm_media) != IFM_AUTO)
+       if ((IFM_SUBTYPE(ife->ifm_media) != IFM_AUTO) &&
+       (IFM_SUBTYPE(ife->ifm_media) != IFM_1000_T))
                return (0);
 
        /* Read the status register twice; BMSR_LINK is latch-low. */