netdev - [RFC PATCH] net: phy: micrel: reset KSZ9x31 when resuming

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240109205223.40219-1-wsa+renesas@sang-engineering.com>
Date: Tue,  9 Jan 2024 21:52:22 +0100
From: Wolfram Sang <wsa+renesas@...g-engineering.com>
To: linux-renesas-soc@...r.kernel.org
Cc: netdev@...r.kernel.org,
	Philippe Schenker <philippe.schenker@...adex.com>,
	Francesco Dolcini <francesco.dolcini@...adex.com>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: [RFC PATCH] net: phy: micrel: reset KSZ9x31 when resuming

On a Renesas Ebisu board, the KSZ9031 PHY is stalled after resuming if
the interface has not been brought up before suspending. If it had been
brought up before, phylib ensures that reset is asserted before
suspending. But if it had never been brought up, there is no instance
which could ensure that reset is asserted. And upon resume, the PHY is
in an unknown state without reset being asserted. To bring it back to a
known state, simply reset it when it is about to be resumed.

This likely also helps another issue [1] where a KSZ9131 can be powered
using regulators. After switching power on again in resume, a reset is
also needed.

[1] https://patchwork.kernel.org/project/netdevbpf/patch/20211214121638.138784-4-philippe.schenker@toradex.com/

Signed-off-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
---

This is a different solution to a problem I already tried to solve
here[2]. Back then, I added code to the MAC, but I now believe it should
be solved on PHY level. We never saw the problem with other PHYs.
Looking at [1], it seems that KSZ9x31 is also sensitive to being
powered down without reset being asserted. I know it is not a perfect
proof, but I guess these assumptions are all we have.

Philippe, Francesco: do you still have access to machines with this
issue? Could you kindly test if so?

Patch is based on 6.7. Looking forward for comments if this is the
correct layer to tackle the problem. Thanks!

[2] https://lore.kernel.org/all/20230321103357.18940-1-wsa+renesas@sang-engineering.com/

 drivers/net/phy/micrel.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 08e3915001c3..c38d7982c06c 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1984,6 +1984,14 @@ static int kszphy_resume(struct phy_device *phydev)
 	return 0;
 }

+static int ksz9x31_resume(struct phy_device *phydev)
+{
+	phy_device_reset(phydev, 1);
+	phy_device_reset(phydev, 0);
+
+	return kszphy_resume(phydev);
+}
+
 static int kszphy_probe(struct phy_device *phydev)
 {
 	const struct kszphy_type *type = phydev->drv->driver_data;
@@ -4778,7 +4786,7 @@ static struct phy_driver ksphy_driver[] = {
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
 	.suspend	= kszphy_suspend,
-	.resume		= kszphy_resume,
+	.resume		= ksz9x31_resume,
 	.cable_test_start	= ksz9x31_cable_test_start,
 	.cable_test_get_status	= ksz9x31_cable_test_get_status,
 }, {
@@ -4851,7 +4859,7 @@ static struct phy_driver ksphy_driver[] = {
 	.get_strings	= kszphy_get_strings,
 	.get_stats	= kszphy_get_stats,
 	.suspend	= kszphy_suspend,
-	.resume		= kszphy_resume,
+	.resume		= ksz9x31_resume,
 	.cable_test_start	= ksz9x31_cable_test_start,
 	.cable_test_get_status	= ksz9x31_cable_test_get_status,
 	.get_features	= ksz9477_get_features,
-- 
2.39.2