[PATCH net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status

[PATCH net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status

[Heiner Kallweit]

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(-)

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;
-- 
2.21.0

Re: [PATCH net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status

[Andrew Lunn]

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

Re: [PATCH net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status

[Heiner Kallweit]

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

Re: [PATCH net-next] net: phy: avoid unneeded MDIO reads in genphy_read_status

[David Miller]

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. :-)