Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/pci/ixgbe Fix a bug that Denverton which uses firmwa...



details:   https://anonhg.NetBSD.org/src/rev/de96123319e9
branches:  trunk
changeset: 357377:de96123319e9
user:      msaitoh <msaitoh%NetBSD.org@localhost>
date:      Wed Nov 08 08:41:13 2017 +0000

description:
Fix a bug that Denverton which uses firmware don't linkup if the media is
forced to 100baseTX-FDX or 10baseT-FDX. As I wrote in ixgbe_phy.c rev. 1.13,
popular switches and OSes don't use auto-negotiation if the media is forced to
100BASE-TX or 10BASE-T. Do the same thing. But, if we don't set
FW_PHY_ACT_SETUP_LINK_AN in ixgbe_setup_fw_link(), the firmware wrongly set
BMCR register. Two problems are observed:

        a) FDX may not be set.
        b) BMCR_SPEED1 (bit 6) is always cleard.

        + -------+------+-----------+-----+
        |request | BMCR | BMCR spd | BMCR |
        |        | (HEX)| (in bits)|  FDX |
        +--------+------+----------+------+
        |   10M  | 0000 |  10M(00) |    0 |
        |   10M  | 2100 | 100M(01) |    1 |
        |  100M  | 0000 |  10M(00) |    0 |
        |  100M  | 0100 |  10M(00) |    1 |
        +--------------------------+------+

 To avoid this problem, after sending request to firmware, check BMCR register
and fix the setting if it's required.

 Before this change:

        +------------------+---------------------------------------------+
        |                  |                    denverton                |
        |                  +---------+--------+---------+----------------+
        |                  |   auto  | 1G FDX | 100 FDX |         10 FDX |
        +---------+--------+---------+--------+---------+----------------+
        |         |   auto |  1G FDX | 1G FDX | 100 FDX | 10FDX/down(NG) |
        |         +--------+---------+--------+---------+----------------+
        |         | 1G FDX |  1G FDX | 1G FDX |    down |           down |
        | link    +--------+---------+--------+---------+----------------+
        | partner |100 FDX | down(*1)|   down | down(NG)|           down |
        |         +--------+---------+--------+---------+----------------+
        |         | 10 FDX | down(*1)|   down |    down |       down(NG) |
        +---------+--------+---------+--------+---------+----------------+
        (Observed on: NVM Image Version 0.05 ID 0x8, NVM Map version 1.16,
        OEM NVM Image version 0.06, ETrackID 8000087c)

 After this change:

        +------------------+---------------------------------------------+
        |                  |                    denverton                |
        |                  +---------+--------+---------+----------------+
        |                  |   auto  | 1G FDX | 100 FDX |         10 FDX |
        +---------+--------+---------+--------+---------+----------------+
        |         |   auto |  1G FDX | 1G FDX | 100 FDX |         10 FDX |
        |         +--------+---------+--------+---------+----------------+
        |         | 1G FDX |  1G FDX | 1G FDX |    down |           down |
        | link    +--------+---------+--------+---------+----------------+
        | partner |100 FDX | down(*1)|   down | 100 FDX |           down |
        |         +--------+---------+--------+---------+----------------+
        |         | 10 FDX | down(*1)|   down |    down |         10 FDX |
        +---------+--------+---------+--------+---------+----------------+
        *1): may be correct because ixg doesn't support half duplex.

diffstat:

 sys/dev/pci/ixgbe/ixgbe_x550.c |  65 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diffs (99 lines):

diff -r 0f1bfb376138 -r de96123319e9 sys/dev/pci/ixgbe/ixgbe_x550.c
--- a/sys/dev/pci/ixgbe/ixgbe_x550.c    Wed Nov 08 00:51:47 2017 +0000
+++ b/sys/dev/pci/ixgbe/ixgbe_x550.c    Wed Nov 08 08:41:13 2017 +0000
@@ -38,6 +38,7 @@
 #include "ixgbe_api.h"
 #include "ixgbe_common.h"
 #include "ixgbe_phy.h"
+#include <dev/mii/mii.h>
 
 static s32 ixgbe_setup_ixfi_x550em(struct ixgbe_hw *hw, ixgbe_link_speed *speed);
 static s32 ixgbe_acquire_swfw_sync_X550a(struct ixgbe_hw *, u32 mask);
@@ -788,6 +789,8 @@
        return ret_val;
 }
 
+#define IXGBE_DENVERTON_WA 1
+
 /**
  * ixgbe_setup_fw_link - Setup firmware-controlled PHYs
  * @hw: pointer to hardware structure
@@ -796,6 +799,10 @@
 {
        u32 setup[FW_PHY_ACT_DATA_COUNT] = { 0 };
        s32 rc;
+#ifdef IXGBE_DENVERTON_WA
+       s32 ret_val;
+       u16 phydata;
+#endif
        u16 i;
 
        if (hw->phy.reset_disable || ixgbe_check_reset_blocked(hw))
@@ -833,9 +840,67 @@
        if (hw->phy.eee_speeds_advertised)
                setup[0] |= FW_PHY_ACT_SETUP_LINK_EEE;
 
+#ifdef IXGBE_DENVERTON_WA
+       /* Don't use auto-nego for 10/100Mbps */
+       if ((hw->phy.autoneg_advertised == IXGBE_LINK_SPEED_100_FULL)
+           || (hw->phy.autoneg_advertised == IXGBE_LINK_SPEED_10_FULL)) {
+               setup[0] &= ~FW_PHY_ACT_SETUP_LINK_AN;
+               setup[0] &= ~FW_PHY_ACT_SETUP_LINK_EEE;
+               setup[0] &= ~(FW_PHY_ACT_SETUP_LINK_PAUSE_RXTX
+                   << FW_PHY_ACT_SETUP_LINK_PAUSE_SHIFT);
+       }
+#endif
+
        rc = ixgbe_fw_phy_activity(hw, FW_PHY_ACT_SETUP_LINK, &setup);
        if (rc)
                return rc;
+
+#ifdef IXGBE_DENVERTON_WA
+       ret_val = ixgbe_read_phy_reg_x550a(hw, MII_BMCR, 0, &phydata);
+       if (ret_val != 0)
+               goto out;
+
+       /*
+        *  Broken firmware sets BMCR register incorrectly if
+        * FW_PHY_ACT_SETUP_LINK_AN isn't set.
+        * a) FDX may not be set.
+        * b) BMCR_SPEED1 (bit 6) is always cleard.
+        * + -------+------+-----------+-----+--------------------------+
+        * |request | BMCR | BMCR spd | BMCR |                          |
+        * |        | (HEX)| (in bits)|  FDX |                          |
+        * +--------+------+----------+------+--------------------------+
+        * |   10M  | 0000 |  10M(00) |    0 |                          |
+        * |   10M  | 2000 | 100M(01) |    0 |(I've never observed this)|
+        * |   10M  | 2100 | 100M(01) |    1 |                          |
+        * |  100M  | 0000 |  10M(00) |    0 |                          |
+        * |  100M  | 0100 |  10M(00) |    1 |                          |
+        * +--------------------------+------+--------------------------+
+        */
+       if (((hw->phy.autoneg_advertised == IXGBE_LINK_SPEED_100_FULL)
+               && (((phydata & BMCR_FDX) == 0) || (BMCR_SPEED(phydata) == 0)))
+           || ((hw->phy.autoneg_advertised == IXGBE_LINK_SPEED_10_FULL)
+               && (((phydata & BMCR_FDX) == 0)
+                   || (BMCR_SPEED(phydata) != BMCR_S10)))) {
+               phydata = BMCR_FDX;
+               switch (hw->phy.autoneg_advertised) {
+               case IXGBE_LINK_SPEED_10_FULL:
+                       phydata |= BMCR_S10;
+                       break;
+               case IXGBE_LINK_SPEED_100_FULL:
+                       phydata |= BMCR_S100;
+                       break;
+               case IXGBE_LINK_SPEED_1GB_FULL:
+                       panic("%s: 1GB_FULL is set", __func__);
+                       break;
+               default:
+                       break;
+               }
+               ret_val = ixgbe_write_phy_reg_x550a(hw, MII_BMCR, 0, phydata);
+               if (ret_val != 0)
+                       return ret_val;
+       }
+out:
+#endif
        if (setup[0] == FW_PHY_ACT_SETUP_LINK_RSP_DOWN)
                return IXGBE_ERR_OVERTEMP;
        return IXGBE_SUCCESS;



Home | Main Index | Thread Index | Old Index