From 83ea067fe2eae9a67c172aede6e11b9f194b1d52 Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 31 Mar 2017 10:37:07 +0100 Subject: [PATCH 1/3] net: phy: avoid setting unsupported EEE advertisments We currently allow userspace to set any EEE advertisments it desires, whether or not the PHY supports them. For example: # ethtool --set-eee eth1 advertise 0xffffffff # ethtool --show-eee eth1 EEE Settings for eth1: EEE status: disabled Tx LPI: disabled Supported EEE link modes: 100baseT/Full 1000baseT/Full 10000baseT/Full Advertised EEE link modes: 100baseT/Full 1000baseT/Full 1000baseKX/Full 10000baseT/Full 10000baseKX4/Full 10000baseKR/Full Clearly, this is not sane, we should only allow link modes that are supported to be advertised (as we do elsewhere.) Ensure that we mask the MDIO_AN_EEE_ADV value with the capabilities retrieved from the MDIO_PCS_EEE_ABLE register. Reviewed-by: Florian Fainelli Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index ba4676ee9018..7b1c93b0233a 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1332,17 +1332,22 @@ EXPORT_SYMBOL(phy_ethtool_get_eee); */ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data) { - int val = ethtool_adv_to_mmd_eee_adv_t(data->advertised); + int cap, adv; if (!phydev->drv) return -EIO; + /* Get Supported EEE */ + cap = phy_read_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE); + if (cap < 0) + return cap; + + adv = ethtool_adv_to_mmd_eee_adv_t(data->advertised) & cap; + /* Mask prohibited EEE modes */ - val &= ~phydev->eee_broken_modes; + adv &= ~phydev->eee_broken_modes; - phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, val); - - return 0; + return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, adv); } EXPORT_SYMBOL(phy_ethtool_set_eee); From f75abeb8338e2d5bcdfc393dff3950a7039eab5a Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 31 Mar 2017 10:37:12 +0100 Subject: [PATCH 2/3] net: phy: restart phy autonegotiation after EEE advertisment change When the EEE advertisment is changed, we should restart autonegotiation to update the link partner with the new EEE settings. Add this trigger but only if the advertisment has changed. Reviewed-by: Florian Fainelli Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 7b1c93b0233a..345251f21699 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1332,7 +1332,7 @@ EXPORT_SYMBOL(phy_ethtool_get_eee); */ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data) { - int cap, adv; + int cap, old_adv, adv, ret; if (!phydev->drv) return -EIO; @@ -1342,12 +1342,29 @@ int phy_ethtool_set_eee(struct phy_device *phydev, struct ethtool_eee *data) if (cap < 0) return cap; + old_adv = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV); + if (old_adv < 0) + return old_adv; + adv = ethtool_adv_to_mmd_eee_adv_t(data->advertised) & cap; /* Mask prohibited EEE modes */ adv &= ~phydev->eee_broken_modes; - return phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, adv); + if (old_adv != adv) { + ret = phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, adv); + if (ret < 0) + return ret; + + /* Restart autonegotiation so the new modes get sent to the + * link partner. + */ + ret = genphy_restart_aneg(phydev); + if (ret < 0) + return ret; + } + + return 0; } EXPORT_SYMBOL(phy_ethtool_set_eee); From 32d751412b8baf977deb3b2dce72025a98cbdd5e Mon Sep 17 00:00:00 2001 From: Russell King Date: Fri, 31 Mar 2017 10:37:18 +0100 Subject: [PATCH 3/3] net: phy: allow EEE with any interface mode EEE is able to work in any PHY interface mode, there is nothing which fundamentally restricts it to only a few modes. For example, EEE works in SGMII mode with the Marvell 88E1512. Rather than just adding SGMII mode to the list, Florian suggests removing the list of interface modes entirely: It actually sounds like we should just kill the check entirely, it does not appear that any of the interface mode would not fundamentally be able to support EEE, because the "lowest" mode we support is MII, and even there it's quite possible to support EEE. Reviewed-by: Florian Fainelli Signed-off-by: Russell King Signed-off-by: David S. Miller --- drivers/net/phy/phy.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 345251f21699..867c42154087 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1208,15 +1208,8 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable) return -EIO; /* According to 802.3az,the EEE is supported only in full duplex-mode. - * Also EEE feature is active when core is operating with MII, GMII - * or RGMII (all kinds). Internal PHYs are also allowed to proceed and - * should return an error if they do not support EEE. */ - if ((phydev->duplex == DUPLEX_FULL) && - ((phydev->interface == PHY_INTERFACE_MODE_MII) || - (phydev->interface == PHY_INTERFACE_MODE_GMII) || - phy_interface_is_rgmii(phydev) || - phy_is_internal(phydev))) { + if (phydev->duplex == DUPLEX_FULL) { int eee_lp, eee_cap, eee_adv; u32 lp, cap, adv; int status;