Re: [PATCH net v2] net: stmmac: resume PHY before hardware setup when opening the interface

Hi,

On 7/31/26 12:03, Stefan Agner wrote:

Hi Paolo, hi Maxime,

On 2026-07-21 10:12, Paolo Abeni wrote:

quoted

Hi,

On 7/8/26 11:16 AM, Maxime Chevallier wrote:

quoted

On 7/7/26 21:54, Stefan Agner wrote:

quoted

Since the referenced commit, changing the MTU on a running interface no
longer disconnects and reconnects the PHY; __stmmac_release() merely
stops phylink, which also suspends the PHY (BMCR power-down) when WoL
is not enabled. __stmmac_open() then performs the DMA software reset in
stmmac_hw_setup() before phylink_start() resumes the PHY again.

IEEE 802.3 22.2.4.1.5 allows a PHY to stop its receive clock while
powered down, and stmmac requires a running receive clock for the DMA
software reset to complete (the phylink config sets mac_requires_rxc).
On such setups, e.g. the RK3566-based Home Assistant Green with an
RTL8211F-VD PHY in RGMII mode, any runtime MTU change now times out and
leaves the interface dead:

  rk_gmac-dwmac fe010000.ethernet end0: Failed to reset the dma
  rk_gmac-dwmac fe010000.ethernet end0: stmmac_hw_setup: DMA engine initialization failed
  rk_gmac-dwmac fe010000.ethernet end0: __stmmac_open: Hw setup failed
  rk_gmac-dwmac fe010000.ethernet end0: failed reopening the interface after MTU change

In the field this is triggered by NetworkManager applying an MTU while
activating the connection, breaking networking entirely.

Resume the PHY in __stmmac_open() before the hardware setup, making it
the counterpart of the phylink_stop() in __stmmac_release(), like
stmmac_resume() already does for the same reason. phylink_start() also
resumes the PHY, but only after stmmac_hw_setup(), and it cannot be
moved before the hardware setup since it may bring the link up
immediately from a workqueue, racing with the initialization (see the
comment in stmmac_resume()). For the regular ndo_open path the PHY has
just been attached and is not suspended, in which case
phylink_prepare_resume() does nothing.

Fixes: db299a0c09e9 ("net: stmmac: move PHY handling out of __stmmac_open()/release()")
Link: https://github.com/home-assistant/operating-system/issues/4858
Signed-off-by: Stefan Agner <stefan@agner.ch>

I was able to reproduce the issue on imx8mp and socfpga. Adding this case to
my periodic test list...

Indeed the assymetry isn't very nice, OTOH there's not phylink counterpart
for phylink_prepare_resume(). This helper was added for the suspend/resume
case, and only for it as the doc states, it just happens to do exactly
what we need to fix the issue :

/**
 * phylink_prepare_resume() - prepare to resume a network device
 * @pl: a pointer to a &struct phylink returned from phylink_create()
 *
 * Optional, but if called must be called prior to phylink_resume().
 *
 * Prepare to resume a network device, preparing the PHY as necessary.
 */
void phylink_prepare_resume(struct phylink *pl)

I think this helper should be renamed and the doc updated, stmmac is the
sole user, and it's really about controlling that rxc and not about
suspend / resume. Resuming from suspend is just one of the cases where we
need that RXC early on.

So either something like 'phylink_prepare_start_or_resume' but it's long,
or maybe we can be more explicit about it and simply call it:

phylink_start_rxc(pl)

(without a corresponding stop)

I read the above as a possible follow-up more than actual changes
requested to this patch, am I correct?

Since this fixes a bug, I'd prefer to keep the patch minimal to make it
easier to backport. Therefor I would prefer to do make such structural
changes as a follow-up change rather than part of this patch.

Does that work for you, Maxime?

That works for me yeah, I agree we need that fixed.

Thanks Stefan,

Maxime