Re: [PATCH net v2] net: phylink: allow stopping a suspended instance

[James Hilliard]

On Wed, Oct 14, 2026 at 10:02 AM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Tue, Oct 13, 2026 at 01:46:24PM -0600, James Hilliard wrote:
> > On Mon, Oct 12, 2026 at 7:22 PM Andrew Lunn <andrew@xxxxxxx> wrote:
> > >
> > > On Mon, Oct 05, 2026 at 01:09:05PM -0600, James Hilliard wrote:
> > > > After a failed MAC resume, a driver must be able to stop phylink without
> > > > resuming it and reconfiguring the failed MAC. This is needed when recovery
> > > > leaves the interface administratively up but detached until a subsequent
> > > > down/up: ndo_stop() must finish the suspended phylink lifetime even though
> > > > the MAC could not be restored. The missing transition was identified by
> > > > code inspection of the stmmac recovery paths.
> > >
> > > I don't particularly like this. It feels like a workaround for a
> > > broken MAC. It would be better to fix the MAC so that resume does not
> > > fail.
> >
> > Fixing an identified cause of resume failure belongs in the MAC
> > driver. The intended purpose here is narrower: allow orderly shutdown
> > after the driver has returned an error, rather than recover the MAC
> > through phylink.
> >
> > stmmac_resume() already returns before phylink_resume() when hardware
> > setup fails.
>
> But why did the hardware setup fail?

The exercised failure was injected, rather than a diagnosed spontaneous
MAC fault. This change comes from auditing the existing error paths for
the deferred stmmac recovery fix.

> This is what i mean about it being a workaround.

The deferred fix addresses a separate software failure: suspend has
already disabled NAPI, but an unsuccessful resume leaves the interface
administratively up. Closing it can then disable NAPI again and hang.
The recovery changes preserve resource ownership so administrative down
can finish and a later open can retry initialization.

That shutdown also has to finish the suspended phylink lifetime.
Without MAC WoL, suspend has already stopped phylink, although
prepare_resume() may have powered the PHY back up. With MAC WoL,
link-down was deferred and the MAC_WOL disable bit remains set.
Suspend-owned advertisement changes also need restoring.

Those are phylink-owned states. Avoiding the second NAPI disable in
stmmac does not resolve them, and calling phylink_resume() just to
permit shutdown would reconfigure the MAC after its setup failed.

The required operation is therefore to terminate the suspended
instance without bringing it back into service. Cleanup must not
depend on first succeeding at the initialization that failed.

The original resume error remains an error, and an identified
MAC/PHY sequencing defect still needs its own fix. This transition
addresses the subsequent shutdown failure; it does not hide or
work around the cause of the unsuccessful resume.

>     Andrew
>