* [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode
@ 2019-02-14 21:12 Heiner Kallweit
2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Heiner Kallweit @ 2019-02-14 21:12 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev
Improve phy_resolve_aneg_linkmode and use it in genphy_read_status.
Heiner Kallweit (2):
net: phy: improve phy_resolve_aneg_linkmode
net: phy: use phy_resolve_aneg_linkmode in genphy_read_status
drivers/net/phy/phy-core.c | 43 ++++++------------------------------
drivers/net/phy/phy_device.c | 24 +-------------------
2 files changed, 8 insertions(+), 59 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode 2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit @ 2019-02-14 21:15 ` Heiner Kallweit 2019-02-15 13:57 ` Andrew Lunn 2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit 2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller 2 siblings, 1 reply; 6+ messages in thread From: Heiner Kallweit @ 2019-02-14 21:15 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev 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; + } if (phydev->duplex == DUPLEX_FULL) { phydev->pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT, -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode 2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit @ 2019-02-15 13:57 ` Andrew Lunn 2019-02-15 18:57 ` Heiner Kallweit 0 siblings, 1 reply; 6+ messages in thread From: Andrew Lunn @ 2019-02-15 13:57 UTC (permalink / raw) To: Heiner Kallweit; +Cc: Florian Fainelli, David Miller, netdev 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode 2019-02-15 13:57 ` Andrew Lunn @ 2019-02-15 18:57 ` Heiner Kallweit 0 siblings, 0 replies; 6+ messages in thread From: Heiner Kallweit @ 2019-02-15 18:57 UTC (permalink / raw) To: Andrew Lunn; +Cc: Florian Fainelli, David Miller, netdev 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status 2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit 2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit @ 2019-02-14 21:16 ` Heiner Kallweit 2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller 2 siblings, 0 replies; 6+ messages in thread From: Heiner Kallweit @ 2019-02-14 21:16 UTC (permalink / raw) To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev Now that we have phy_resolve_aneg_linkmode() we can make genphy_read_status() much simpler. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy_device.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 2c61282a2..d7ccbd94f 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1747,8 +1747,6 @@ int genphy_read_status(struct phy_device *phydev) int err; int lpa; int lpagb = 0; - int common_adv; - int common_adv_gb = 0; /* Update the link, but return if there was an error */ err = genphy_update_link(phydev); @@ -1780,7 +1778,6 @@ int genphy_read_status(struct phy_device *phydev) mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, lpagb); - common_adv_gb = lpagb & adv << 2; } lpa = phy_read(phydev, MII_LPA); @@ -1793,31 +1790,12 @@ int genphy_read_status(struct phy_device *phydev) if (adv < 0) return adv; - common_adv = lpa & adv; - phydev->speed = SPEED_10; phydev->duplex = DUPLEX_HALF; phydev->pause = 0; phydev->asym_pause = 0; - if (common_adv_gb & (LPA_1000FULL | LPA_1000HALF)) { - phydev->speed = SPEED_1000; - - if (common_adv_gb & LPA_1000FULL) - phydev->duplex = DUPLEX_FULL; - } else if (common_adv & (LPA_100FULL | LPA_100HALF)) { - phydev->speed = SPEED_100; - - if (common_adv & LPA_100FULL) - phydev->duplex = DUPLEX_FULL; - } else - if (common_adv & LPA_10FULL) - phydev->duplex = DUPLEX_FULL; - - if (phydev->duplex == DUPLEX_FULL) { - phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0; - phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0; - } + phy_resolve_aneg_linkmode(phydev); } else { int bmcr = phy_read(phydev, MII_BMCR); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode 2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit 2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit 2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit @ 2019-02-17 23:22 ` David Miller 2 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2019-02-17 23:22 UTC (permalink / raw) To: hkallweit1; +Cc: andrew, f.fainelli, netdev From: Heiner Kallweit <hkallweit1@gmail.com> Date: Thu, 14 Feb 2019 22:12:59 +0100 > Improve phy_resolve_aneg_linkmode and use it in genphy_read_status. Series applied, thanks Heiner. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-17 23:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-14 21:12 [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode Heiner Kallweit 2019-02-14 21:15 ` [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode Heiner Kallweit 2019-02-15 13:57 ` Andrew Lunn 2019-02-15 18:57 ` Heiner Kallweit 2019-02-14 21:16 ` [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status Heiner Kallweit 2019-02-17 23:22 ` [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).