[PATCH net v3] net: phy: smsc: fix printing too many logs

[PATCH net v3] net: phy: smsc: fix printing too many logs

[Dejin Zheng]

Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
to simplify the code") will print a lot of logs as follows when Ethernet
cable is not connected:

[    4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status failed: -110

When wait 640 ms for check ENERGYON bit, the timeout should not be
regarded as an actual error and an error message also should not be
printed. due to a hardware bug in LAN87XX device, it leads to unstable
detection of plugging in Ethernet cable when LAN87xx is in Energy Detect
Power-Down mode. the workaround for it involves, when the link is down,
and at each read_status() call:

- disable EDPD mode, forcing the PHY out of low-power mode
- waiting 640ms to see if we have any energy detected from the media
- re-enable entry to EDPD mode

This is presumably enough to allow the PHY to notice that a cable is
connected, and resume normal operations to negotiate with the partner.
The problem is that when no media is detected, the 640ms wait times
out and this commit was modified to prints an error message. it is an
inappropriate conversion by used phy_read_poll_timeout() to introduce
this bug. so fix this issue by use read_poll_timeout() to replace
phy_read_poll_timeout().

Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify the code")
Reported-by: Kevin Groeneveld <kgroeneveld@gmail.com>
Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
v2 -> v3:
	- modify commit message to why need this patch. And many thanks
	  to Andrew and Russell for their comments.
v1 -> v2:
	- add more commit message spell out what the change does.

 drivers/net/phy/smsc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 93da7d3d0954..74568ae16125 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -122,10 +122,13 @@ static int lan87xx_read_status(struct phy_device *phydev)
 		if (rc < 0)
 			return rc;
 
-		/* Wait max 640 ms to detect energy */
-		phy_read_poll_timeout(phydev, MII_LAN83C185_CTRL_STATUS, rc,
-				      rc & MII_LAN83C185_ENERGYON, 10000,
-				      640000, true);
+		/* Wait max 640 ms to detect energy and the timeout is not
+		 * an actual error.
+		 */
+		read_poll_timeout(phy_read, rc,
+				  rc & MII_LAN83C185_ENERGYON || rc < 0,
+				  10000, 640000, true, phydev,
+				  MII_LAN83C185_CTRL_STATUS);
 		if (rc < 0)
 			return rc;
 
-- 
2.25.0

Re: [PATCH net v3] net: phy: smsc: fix printing too many logs

[Andrew Lunn]

On Sat, Jun 20, 2020 at 10:55:34PM +0800, Dejin Zheng wrote:
> Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
> to simplify the code") will print a lot of logs as follows when Ethernet
> cable is not connected:
> 
> [    4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status failed: -110
> 
> When wait 640 ms for check ENERGYON bit, the timeout should not be
> regarded as an actual error and an error message also should not be
> printed. due to a hardware bug in LAN87XX device, it leads to unstable
> detection of plugging in Ethernet cable when LAN87xx is in Energy Detect
> Power-Down mode. the workaround for it involves, when the link is down,
> and at each read_status() call:
> 
> - disable EDPD mode, forcing the PHY out of low-power mode
> - waiting 640ms to see if we have any energy detected from the media
> - re-enable entry to EDPD mode
> 
> This is presumably enough to allow the PHY to notice that a cable is
> connected, and resume normal operations to negotiate with the partner.
> The problem is that when no media is detected, the 640ms wait times
> out and this commit was modified to prints an error message. it is an
> inappropriate conversion by used phy_read_poll_timeout() to introduce
> this bug. so fix this issue by use read_poll_timeout() to replace
> phy_read_poll_timeout().
> 
> Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify the code")
> Reported-by: Kevin Groeneveld <kgroeneveld@gmail.com>
> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v3] net: phy: smsc: fix printing too many logs

[David Miller]

From: Dejin Zheng <zhengdejin5@gmail.com>
Date: Sat, 20 Jun 2020 22:55:34 +0800

> Commit 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout()
> to simplify the code") will print a lot of logs as follows when Ethernet
> cable is not connected:
> 
> [    4.473105] SMSC LAN8710/LAN8720 2188000.ethernet-1:00: lan87xx_read_status failed: -110
> 
> When wait 640 ms for check ENERGYON bit, the timeout should not be
> regarded as an actual error and an error message also should not be
> printed. due to a hardware bug in LAN87XX device, it leads to unstable
> detection of plugging in Ethernet cable when LAN87xx is in Energy Detect
> Power-Down mode. the workaround for it involves, when the link is down,
> and at each read_status() call:
> 
> - disable EDPD mode, forcing the PHY out of low-power mode
> - waiting 640ms to see if we have any energy detected from the media
> - re-enable entry to EDPD mode
> 
> This is presumably enough to allow the PHY to notice that a cable is
> connected, and resume normal operations to negotiate with the partner.
> The problem is that when no media is detected, the 640ms wait times
> out and this commit was modified to prints an error message. it is an
> inappropriate conversion by used phy_read_poll_timeout() to introduce
> this bug. so fix this issue by use read_poll_timeout() to replace
> phy_read_poll_timeout().
> 
> Fixes: 7ae7ad2f11ef47 ("net: phy: smsc: use phy_read_poll_timeout() to simplify the code")
> Reported-by: Kevin Groeneveld <kgroeneveld@gmail.com>
> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>

Applied and queued up for -stable, thanks.