| Message ID | 2ad2dd35-45f5-9720-d2ae-0ee104a7d782@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: add and use genphy_c45_an_config_an | expand |
On 2/16/2019 11:52 AM, Heiner Kallweit wrote: > From: Andrew Lunn <andrew@lunn.ch> > Use new function genphy_c45_config_aneg() in mv3310_config_aneg(). > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > [hkallweit1@gmail.com: patch splitted] > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/marvell10g.c | 22 +--------------------- > 1 file changed, 1 insertion(+), 21 deletions(-) > > diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c > index 4a6ae63ab..03fa50087 100644 > --- a/drivers/net/phy/marvell10g.c > +++ b/drivers/net/phy/marvell10g.c > @@ -274,13 +274,7 @@ static int mv3310_config_aneg(struct phy_device *phydev) > if (phydev->autoneg == AUTONEG_DISABLE) > return genphy_c45_pma_setup_forced(phydev); > > - linkmode_and(phydev->advertising, phydev->advertising, > - phydev->supported); > - > - ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE, > - ADVERTISE_ALL | ADVERTISE_100BASE4 | > - ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM, > - linkmode_adv_to_mii_adv_t(phydev->advertising)); > + ret = genphy_c45_an_config_an(phydev); > if (ret < 0) > return ret; > if (ret > 0) > @@ -294,20 +288,6 @@ static int mv3310_config_aneg(struct phy_device *phydev) > if (ret > 0) > changed = true; > > - /* 10G control register */ > - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, > - phydev->advertising)) > - reg = MDIO_AN_10GBT_CTRL_ADV10G; > - else > - reg = 0; This is not strictly equivalent though because marvell10g only checks for the 10000baseT_Full bit set whereas genphy_c45_an_config_an() calls linkmode_adv_to_mii_10gbt_adv_t() which you recently updated to also check for 2.5G and 5G. This sounds about the right decision, but I wonder if Russell did this for a reason (like not able to test 2500baseT and 5000baseT?) > - > - ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL, > - MDIO_AN_10GBT_CTRL_ADV10G, reg); > - if (ret < 0) > - return ret; > - if (ret > 0) > - changed = true; > - > if (changed) > ret = genphy_c45_restart_aneg(phydev); > >
> This is not strictly equivalent though because marvell10g only checks > for the 10000baseT_Full bit set whereas genphy_c45_an_config_an() calls > linkmode_adv_to_mii_10gbt_adv_t() which you recently updated to also > check for 2.5G and 5G. This sounds about the right decision, but I > wonder if Russell did this for a reason (like not able to test 2500baseT > and 5000baseT?) Hi Florian Correct. But both Marvell MAC drivers recently gained the code needed for 2500BaseX and 5000BaseX. See Maxime and Russell comphy patches. Russell has tested 2500BaseX SFPs and Maxime has been testing 2500BaseT copper. Andrew
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c index 4a6ae63ab..03fa50087 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -274,13 +274,7 @@ static int mv3310_config_aneg(struct phy_device *phydev) if (phydev->autoneg == AUTONEG_DISABLE) return genphy_c45_pma_setup_forced(phydev); - linkmode_and(phydev->advertising, phydev->advertising, - phydev->supported); - - ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_ADVERTISE, - ADVERTISE_ALL | ADVERTISE_100BASE4 | - ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM, - linkmode_adv_to_mii_adv_t(phydev->advertising)); + ret = genphy_c45_an_config_an(phydev); if (ret < 0) return ret; if (ret > 0) @@ -294,20 +288,6 @@ static int mv3310_config_aneg(struct phy_device *phydev) if (ret > 0) changed = true; - /* 10G control register */ - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, - phydev->advertising)) - reg = MDIO_AN_10GBT_CTRL_ADV10G; - else - reg = 0; - - ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN, MDIO_AN_10GBT_CTRL, - MDIO_AN_10GBT_CTRL_ADV10G, reg); - if (ret < 0) - return ret; - if (ret > 0) - changed = true; - if (changed) ret = genphy_c45_restart_aneg(phydev);