net: dsa: mv88e6xxx: Link in pcs_get_state() if AN is bypassed

Function mv88e6xxx_serdes_pcs_get_state() currently does not report link
up if AN is enabled, Link bit is set, but Speed and Duplex Resolved bit
is not set, which testing shows is the case for when auto-negotiation
was bypassed (we have AN enabled but link partner does not).

An example of such link partner is Marvell 88X3310 PHY, when put into
the mode where host interface changes between 10gbase-r, 5gbase-r,
2500base-x and sgmii according to copper speed. The 88X3310 does not
enable AN in 2500base-x, and so SerDes on mv88e6xxx currently does not
link with it.

Fix this.

Fixes: 860d703ec5b2 ("net: dsa: mv88e6xxx: extend phylink to Serdes PHYs")
Signed-off-by: Marek Behún <kabel@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 6f60376b932c7957139330d3e9a7ff050c83f06a..55273013bfb55c5b59a72b6b7fdaff0c1c508915 100644 (file)
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -50,11 +50,22 @@ static int mv88e6390_serdes_write(struct mv88e6xxx_chip *chip,
 }
 
 static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
-                                         u16 status, u16 lpa,
+                                         u16 ctrl, u16 status, u16 lpa,
                                          struct phylink_link_state *state)
 {
+       state->link = !!(status & MV88E6390_SGMII_PHY_STATUS_LINK);
+
        if (status & MV88E6390_SGMII_PHY_STATUS_SPD_DPL_VALID) {
-               state->link = !!(status & MV88E6390_SGMII_PHY_STATUS_LINK);
+               /* The Spped and Duplex Resolved register is 1 if AN is enabled
+                * and complete, or if AN is disabled. So with disabled AN we
+                * still get here on link up. But we want to set an_complete
+                * only if AN was enabled, thus we look at BMCR_ANENABLE.
+                * (According to 802.3-2008 section 22.2.4.2.10, we should be
+                *  able to get this same value from BMSR_ANEGCAPABLE, but tests
+                *  show that these Marvell PHYs don't conform to this part of
+                *  the specificaion - BMSR_ANEGCAPABLE is simply always 1.)
+                */
+               state->an_complete = !!(ctrl & BMCR_ANENABLE);
                state->duplex = status &
                                MV88E6390_SGMII_PHY_STATUS_DUPLEX_FULL ?
                                                 DUPLEX_FULL : DUPLEX_HALF;
@@ -81,6 +92,18 @@ static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
                        dev_err(chip->dev, "invalid PHY speed\n");
                        return -EINVAL;
                }
+       } else if (state->link &&
+                  state->interface != PHY_INTERFACE_MODE_SGMII) {
+               /* If Speed and Duplex Resolved register is 0 and link is up, it
+                * means that AN was enabled, but link partner had it disabled
+                * and the PHY invoked the Auto-Negotiation Bypass feature and
+                * linked anyway.
+                */
+               state->duplex = DUPLEX_FULL;
+               if (state->interface == PHY_INTERFACE_MODE_2500BASEX)
+                       state->speed = SPEED_2500;
+               else
+                       state->speed = SPEED_1000;
        } else {
                state->link = false;
        }
@@ -168,9 +191,15 @@ int mv88e6352_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
 int mv88e6352_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
                                   int lane, struct phylink_link_state *state)
 {
-       u16 lpa, status;
+       u16 lpa, status, ctrl;
        int err;
 
+       err = mv88e6352_serdes_read(chip, MII_BMCR, &ctrl);
+       if (err) {
+               dev_err(chip->dev, "can't read Serdes PHY control: %d\n", err);
+               return err;
+       }
+
        err = mv88e6352_serdes_read(chip, 0x11, &status);
        if (err) {
                dev_err(chip->dev, "can't read Serdes PHY status: %d\n", err);
@@ -183,7 +212,7 @@ int mv88e6352_serdes_pcs_get_state(struct mv88e6xxx_chip *chip, int port,
                return err;
        }
 
-       return mv88e6xxx_serdes_pcs_get_state(chip, status, lpa, state);
+       return mv88e6xxx_serdes_pcs_get_state(chip, ctrl, status, lpa, state);
 }
 
 int mv88e6352_serdes_pcs_an_restart(struct mv88e6xxx_chip *chip, int port,
@@ -883,9 +912,16 @@ int mv88e6390_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
 static int mv88e6390_serdes_pcs_get_state_sgmii(struct mv88e6xxx_chip *chip,
        int port, int lane, struct phylink_link_state *state)
 {
-       u16 lpa, status;
+       u16 lpa, status, ctrl;
        int err;
 
+       err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
+                                   MV88E6390_SGMII_BMCR, &ctrl);
+       if (err) {
+               dev_err(chip->dev, "can't read Serdes PHY control: %d\n", err);
+               return err;
+       }
+
        err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS,
                                    MV88E6390_SGMII_PHY_STATUS, &status);
        if (err) {
@@ -900,7 +936,7 @@ static int mv88e6390_serdes_pcs_get_state_sgmii(struct mv88e6xxx_chip *chip,
                return err;
        }
 
-       return mv88e6xxx_serdes_pcs_get_state(chip, status, lpa, state);
+       return mv88e6xxx_serdes_pcs_get_state(chip, ctrl, status, lpa, state);
 }
 
 static int mv88e6390_serdes_pcs_get_state_10g(struct mv88e6xxx_chip *chip,