From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Abid Ali <dev.nuvorolabs@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Heiner Kallweit <hkallweit1@gmail.com>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: phy: Fix premature resume by a PHY driver
Date: Sat, 19 Jul 2025 08:48:20 +0100 [thread overview]
Message-ID: <aHtNxLODmEHRVfdn@shell.armlinux.org.uk> (raw)
In-Reply-To: <20250719062550.652-1-dev.nuvorolabs@gmail.com>
On Sat, Jul 19, 2025 at 06:25:47AM +0000, Abid Ali wrote:
> On Fri, Jul 18, 2025 at 05:10:54 PM +0100, Russell King (Oracle) wrote:
> > Sorry but no. The PHY will be "resumed" from boot, even if it wasn't
> > "suspended". So the idea that resume should only be called if it was
> > previously suspended is incorrect.
>
> > E.g. .ndo_open -> ... -> phy_attach_direct() -> phy_resume() ->
> > phydrv->resume()
>
> I do point this path out and there is also a second call
> (2) .ndo_open -> phylink_start -> phy_start -> __phy_resume
> This would mean 2 calls to the PHY resume every time an interface is
> taken UP is expected behaviour?.
The whole point is this:
> > During this path, the PHY may or may not be suspended, depending on
> > the state of the hardware when control was passed to the kernel,
> > which includes kexec().
Thus, the resume function *must* cope with an already resumed PHY,
and thus adding extra complexity to try to ignore calling the resume
function if it wasn't previously suspended is likely to cause
regressions - phydrv->suspended will be clear for the initial call
to ->resume(). Thus, if the PHY was suspended at boot time, it won't
be resumed when one attempts to bring up the interface initially.
Just fix your driver's resume function.
> > PHY drivers must cope with an already functional PHY when their
> > resume() method is called.
>
> This is not what I expected, but if it is by design I do not see a
> need to fight it. Just to make it clear, if we need to reset a PHY
> after it returns from suspend(or any code thats dependant), the driver`s
> callback should provide this guarantee?.
Hardware or software reset?
How much a software reset disrupts the PHY is PHY dependent. E.g. there
are PHYs that need to be software reset for configuration and
advertisement changes, but all the software configuration is preserved
over such a reset.
So I can't answer your question. It depends how disruptive the reset
you are talking about is to your PHY implementation.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2025-07-19 7:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-18 15:42 [PATCH] net: phy: Fix premature resume by a PHY driver Abid Ali
2025-07-18 16:10 ` Russell King (Oracle)
2025-07-19 6:25 ` Abid Ali
2025-07-19 7:48 ` Russell King (Oracle) [this message]
2025-07-19 11:34 ` Abid Ali
2025-07-19 15:32 ` Russell King (Oracle)
2025-07-19 5:37 ` kernel test robot
2025-07-20 18:09 ` Dan Carpenter
2025-07-21 6:07 ` Abid Ali
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aHtNxLODmEHRVfdn@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=dev.nuvorolabs@gmail.com \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).