Re: [PATCH net] net: usb: lan78xx: only stop phylink if the interface is running
From: YuanZhe Liu
Date: Sat Oct 31 2026 - 12:53:52 EST
On Fri, Oct 30, 2026 at 04:02:36PM +0000, sashiko-bot@xxxxxxxxxx wrote:
> Does calling phylink_stop() and phylink_disconnect_phy() before
> unregister_netdev(net) in drivers/net/usb/lan78xx.c:lan78xx_disconnect()
> cause out-of-order teardown, a duplicate phylink_stop() call, and a race
> window when rtnl_lock() is dropped?
Yes, the ordering is wrong. v2 drops the explicit phylink_stop() from the
disconnect path and unregisters the net device first:
unregister_netdev(net);
rtnl_lock();
phylink_disconnect_phy(dev->phylink);
rtnl_unlock();
unregister_netdev() closes a running device, so phylink_stop() is done by
lan78xx_stop() with dev->dev_mutex held and after the TX/RX URBs have been
terminated, which is the same sequence as a normal ifdown. When the
interface was never started no phylink_stop() happens at all, and that is
exactly the case phy_stop() complains about with "called from state READY".
This also removes the second phylink_stop() invocation and the window in
which the net device was still registered with no PHY attached.
On the remaining points:
- The duplicate phylink_stop() was harmless in practice, because after
phylink_disconnect_phy() pl->phydev is NULL and PHYLINK_DISABLE_STOPPED
is already set. It is gone now anyway.
- The PHY interrupt is not released by phylink_disconnect_phy(). The virq
created by irq_create_mapping() is disposed in lan78xx_remove_irq_domain(),
called from lan78xx_unbind(), which also clears dev->domain_data.phyirq;
lan78xx_status() tests that field before generic_handle_irq_safe(). The
intr URB is killed later in lan78xx_disconnect(), and that ordering is
unchanged by this patch.
- Andrew also asked why the Fixes tag points at e110bc825897: that commit
added both the phylink instance and the unconditional phylink_stop() call
in lan78xx_disconnect(), and
git log -S'phylink_stop(dev->phylink)' -- drivers/net/usb/lan78xx.c
shows that the line has not been touched since.
v2 is sent as a separate thread.
Thanks,
YuanZhe