| Message ID | 1571d9d1-b3ad-8c96-a476-4ae18d20abfe@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: improve and use phy_resolve_aneg_linkmode | expand |
On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote: > We have the settings array of modes which is sorted based on aneg > priority. Instead of checking each mode manually let's simply iterate > over the sorted settings. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/phy-core.c | 43 +++++++------------------------------- > 1 file changed, 7 insertions(+), 36 deletions(-) > > diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c > index cdea028d1..5d43106fe 100644 > --- a/drivers/net/phy/phy-core.c > +++ b/drivers/net/phy/phy-core.c > @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size, > void phy_resolve_aneg_linkmode(struct phy_device *phydev) > { > __ETHTOOL_DECLARE_LINK_MODE_MASK(common); > + int i; > > linkmode_and(common, phydev->lp_advertising, phydev->advertising); > > - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) { > - phydev->speed = SPEED_10000; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, > - common)) { > - phydev->speed = SPEED_5000; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > - common)) { > - phydev->speed = SPEED_2500; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, > - common)) { > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, > - common)) { > - phydev->speed = SPEED_1000; > - phydev->duplex = DUPLEX_HALF; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, > - common)) { > - phydev->speed = SPEED_100; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, > - common)) { > - phydev->speed = SPEED_100; > - phydev->duplex = DUPLEX_HALF; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, > - common)) { > - phydev->speed = SPEED_10; > - phydev->duplex = DUPLEX_FULL; > - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, > - common)) { > - phydev->speed = SPEED_10; > - phydev->duplex = DUPLEX_HALF; > - } > + for (i = 0; i < ARRAY_SIZE(settings); i++) > + if (test_bit(settings[i].bit, common)) { > + phydev->speed = settings[i].speed; > + phydev->duplex = settings[i].duplex; > + break; > + } Hi Heiner Nice simplification. I just have one thought about this. The original code was limited to baseT. The new code could in theory return a non BaseT speed. For that to happen, it would require that phydev->lp_advertising and phydev->advertising contain a non BaseT link mode? Is that possible? I don't think it is. Andrew
On 15.02.2019 14:57, Andrew Lunn wrote: > On Thu, Feb 14, 2019 at 10:15:31PM +0100, Heiner Kallweit wrote: >> We have the settings array of modes which is sorted based on aneg >> priority. Instead of checking each mode manually let's simply iterate >> over the sorted settings. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/net/phy/phy-core.c | 43 +++++++------------------------------- >> 1 file changed, 7 insertions(+), 36 deletions(-) >> >> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c >> index cdea028d1..5d43106fe 100644 >> --- a/drivers/net/phy/phy-core.c >> +++ b/drivers/net/phy/phy-core.c >> @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size, >> void phy_resolve_aneg_linkmode(struct phy_device *phydev) >> { >> __ETHTOOL_DECLARE_LINK_MODE_MASK(common); >> + int i; >> >> linkmode_and(common, phydev->lp_advertising, phydev->advertising); >> >> - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) { >> - phydev->speed = SPEED_10000; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, >> - common)) { >> - phydev->speed = SPEED_5000; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, >> - common)) { >> - phydev->speed = SPEED_2500; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, >> - common)) { >> - phydev->speed = SPEED_1000; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, >> - common)) { >> - phydev->speed = SPEED_1000; >> - phydev->duplex = DUPLEX_HALF; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, >> - common)) { >> - phydev->speed = SPEED_100; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, >> - common)) { >> - phydev->speed = SPEED_100; >> - phydev->duplex = DUPLEX_HALF; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, >> - common)) { >> - phydev->speed = SPEED_10; >> - phydev->duplex = DUPLEX_FULL; >> - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, >> - common)) { >> - phydev->speed = SPEED_10; >> - phydev->duplex = DUPLEX_HALF; >> - } >> + for (i = 0; i < ARRAY_SIZE(settings); i++) >> + if (test_bit(settings[i].bit, common)) { >> + phydev->speed = settings[i].speed; >> + phydev->duplex = settings[i].duplex; >> + break; >> + } > > Hi Heiner > > Nice simplification. > > I just have one thought about this. The original code was limited to > baseT. The new code could in theory return a non BaseT speed. For that > to happen, it would require that phydev->lp_advertising and > phydev->advertising contain a non BaseT link mode? Is that possible? > I don't think it is. > Currently we set only BaseT modes because that's what the clause 45 standard registers offer. However drivers may come that use vendor registers for e.g. backplane auto-negotiation (IIRC clause 73). Now it's even better because this function shouldn't be (and doesn't have to be) limited to a specific physical link type. > Andrew > Heiner
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index cdea028d1..5d43106fe 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size, void phy_resolve_aneg_linkmode(struct phy_device *phydev) { __ETHTOOL_DECLARE_LINK_MODE_MASK(common); + int i; linkmode_and(common, phydev->lp_advertising, phydev->advertising); - if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) { - phydev->speed = SPEED_10000; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, - common)) { - phydev->speed = SPEED_5000; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, - common)) { - phydev->speed = SPEED_2500; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, - common)) { - phydev->speed = SPEED_1000; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT, - common)) { - phydev->speed = SPEED_1000; - phydev->duplex = DUPLEX_HALF; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, - common)) { - phydev->speed = SPEED_100; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, - common)) { - phydev->speed = SPEED_100; - phydev->duplex = DUPLEX_HALF; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, - common)) { - phydev->speed = SPEED_10; - phydev->duplex = DUPLEX_FULL; - } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT, - common)) { - phydev->speed = SPEED_10; - phydev->duplex = DUPLEX_HALF; - } + for (i = 0; i < ARRAY_SIZE(settings); i++) + if (test_bit(settings[i].bit, common)) { + phydev->speed = settings[i].speed; + phydev->duplex = settings[i].duplex; + break; + } if (phydev->duplex == DUPLEX_FULL) { phydev->pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
We have the settings array of modes which is sorted based on aneg priority. Instead of checking each mode manually let's simply iterate over the sorted settings. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy-core.c | 43 +++++++------------------------------- 1 file changed, 7 insertions(+), 36 deletions(-)