| Message ID | 92a0d681-33ea-feb7-bdf5-ff6fd9911ce1@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status | expand |
On Wed, Apr 24, 2019 at 09:49:30PM +0200, Heiner Kallweit wrote: > Considering that in polling mode each link drop will be latched, > settings can't have changed if link was up and is up. Hi Heiner What about the case of the PHY performing a downshift? Could it be up, then finds a pair fails, so performs a downshift. Does it always report link down and then up to the new speed? Or could it just shift down? And so we want to read the new speed? I suppose it could also perform an upshift? A broken pair comes back to life, so it returns to the higher speed? Andrew
On 25.04.2019 00:41, Andrew Lunn wrote: > On Wed, Apr 24, 2019 at 09:49:30PM +0200, Heiner Kallweit wrote: >> Considering that in polling mode each link drop will be latched, >> settings can't have changed if link was up and is up. > > Hi Heiner > Hi Andrew, interesting questions. > What about the case of the PHY performing a downshift? > > Could it be up, then finds a pair fails, so performs a downshift. Does > it always report link down and then up to the new speed? Or could it > just shift down? And so we want to read the new speed? > My experience with downshift is that it's part of an extended autoneg process. The link stays down all the time until both link partners have agreed on a downshifted speed. > I suppose it could also perform an upshift? A broken pair comes back > to life, so it returns to the higher speed? > So far I didn't come across an upshift feature. But also then I'd expect some autoneg process with the link being down. More likely seems to be that upshifting requires an explicit autoneg restart. > Andrew > Heiner
From: Heiner Kallweit <hkallweit1@gmail.com> Date: Wed, 24 Apr 2019 21:49:30 +0200 > Considering that in polling mode each link drop will be latched, > settings can't have changed if link was up and is up. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Applied, let's see what happens. :-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index f794ff3a1..2a2aaa5f3 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1739,13 +1739,17 @@ EXPORT_SYMBOL(genphy_update_link); */ int genphy_read_status(struct phy_device *phydev) { - int adv, lpa, lpagb, err; + int adv, lpa, lpagb, err, old_link = phydev->link; /* Update the link, but return if there was an error */ err = genphy_update_link(phydev); if (err) return err; + /* why bother the PHY if nothing can have changed */ + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link) + return 0; + phydev->speed = SPEED_UNKNOWN; phydev->duplex = DUPLEX_UNKNOWN; phydev->pause = 0;
Considering that in polling mode each link drop will be latched, settings can't have changed if link was up and is up. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy_device.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)