Received-SPF: pass (google.com: domain of linux-kernel+bounces-21402-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: linux-renesas-soc@vger.kernel.org
Cc: netdev@vger.kernel.org,
	Philippe Schenker <philippe.schenker@toradex.com>,
	Francesco Dolcini <francesco.dolcini@toradex.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [RFC PATCH] net: phy: micrel: reset KSZ9x31 when resuming
Date: Tue,  9 Jan 2024 21:52:22 +0100
Message-Id: <20240109205223.40219-1-wsa+renesas@sang-engineering.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

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@sang-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