net: phy: at803x: remove at803x_aneg_done()
authorMichael Walle <michael@walle.cc>
Thu, 18 Mar 2021 19:44:31 +0000 (20:44 +0100)
committerDavid S. Miller <davem@davemloft.net>
Fri, 19 Mar 2021 18:36:42 +0000 (11:36 -0700)
Here is what Vladimir says about it:

  at803x_aneg_done() keeps the aneg reporting as "not done" even when
  the copper-side link was reported as up, but the in-band autoneg has
  not finished.

  That was the _intended_ behavior when that code was introduced, and
  Heiner have said about it [1]:

  | That's not nice from the PHY:
  | It signals "link up", and if the system asks the PHY for link details,
  | then it sheepishly says "well, link is *almost* up".

  If the specification of phy_aneg_done behavior does not include
  in-band autoneg (and it doesn't), then this piece of code does not
  belong here.

  The fact that we can no longer trigger this code from phylib is yet
  another reason why it fails at its intended (and wrong) purpose and
  should be removed.

Removing the SGMII link check, would just keep the call to
genphy_aneg_done(), which is also the fallback. Thus we can just remove
at803x_aneg_done() altogether.

[1] https://lore.kernel.org/netdev/fdf0074a-2572-5914-6f3e-77202cbf96de@gmail.com/

Suggested-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/phy/at803x.c

index c2aa4c9..d7799be 100644 (file)
@@ -751,36 +751,6 @@ static void at803x_link_change_notify(struct phy_device *phydev)
        }
 }
 
-static int at803x_aneg_done(struct phy_device *phydev)
-{
-       int ccr;
-
-       int aneg_done = genphy_aneg_done(phydev);
-       if (aneg_done != BMSR_ANEGCOMPLETE)
-               return aneg_done;
-
-       /*
-        * in SGMII mode, if copper side autoneg is successful,
-        * also check SGMII side autoneg result
-        */
-       ccr = phy_read(phydev, AT803X_REG_CHIP_CONFIG);
-       if ((ccr & AT803X_MODE_CFG_MASK) != AT803X_MODE_CFG_SGMII)
-               return aneg_done;
-
-       /* switch to SGMII/fiber page */
-       phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr & ~AT803X_BT_BX_REG_SEL);
-
-       /* check if the SGMII link is OK. */
-       if (!(phy_read(phydev, AT803X_PSSR) & AT803X_PSSR_MR_AN_COMPLETE)) {
-               phydev_warn(phydev, "803x_aneg_done: SGMII link is not ok\n");
-               aneg_done = 0;
-       }
-       /* switch back to copper page */
-       phy_write(phydev, AT803X_REG_CHIP_CONFIG, ccr | AT803X_BT_BX_REG_SEL);
-
-       return aneg_done;
-}
-
 static int at803x_read_status(struct phy_device *phydev)
 {
        int ss, err, old_link = phydev->link;
@@ -1198,7 +1168,6 @@ static struct phy_driver at803x_driver[] = {
        .resume                 = at803x_resume,
        /* PHY_GBIT_FEATURES */
        .read_status            = at803x_read_status,
-       .aneg_done              = at803x_aneg_done,
        .config_intr            = &at803x_config_intr,
        .handle_interrupt       = at803x_handle_interrupt,
        .get_tunable            = at803x_get_tunable,