- 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@xxxxxxxxxxx/ > > Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> > --- > > 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@xxxxxxxxxxxxxxxxxxxx/ > > 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
