[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240109232838.GA3626@francesco-nb>
Date: Wed, 10 Jan 2024 00:28:38 +0100
From: Francesco Dolcini <francesco@...cini.it>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>
Cc: linux-renesas-soc@...r.kernel.org, netdev@...r.kernel.org,
Francesco Dolcini <francesco.dolcini@...adex.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: Re: [RFC PATCH] net: phy: micrel: reset KSZ9x31 when resuming
- Philippe, email address is no longer valid.
On Tue, Jan 09, 2024 at 09:52:22PM +0100, Wolfram Sang wrote:
> 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?
I have access, however
- Philippe is long gone from Toradex and he was the one looking into
this topic
- we did solve the issue in a different way, e.g. we no longer
power-off the phy in suspend
Therefore is not straightforward to provide valuable feedback to you
now.
>
> 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);
Is something like that fine?
Don't we need to reconfigure the ethernet phy completely on resume
if we do reset it? kszphy_config_reset() is taking care of something,
but I think that the phy being reset on resume is not handled
correctly.
Francesco
Powered by blists - more mailing lists