[PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[YuanZhe Liu]

lan78xx_disconnect() unconditionally calls phylink_stop(), but
phylink_start() is only called from lan78xx_open(). When the device is
unplugged before the interface has been brought up, the attached PHY is
still in PHY_READY and phy_stop() reports:

  called from state READY
  WARNING: drivers/net/phy/phy.c:1670 at phy_stop+0xad/0x380

The same warning is emitted after an ifup/ifdown cycle, where the PHY is
already in PHY_HALTED. Call phylink_stop() only when the network device is
running, which is when phylink_start() has been called.

Fixes: e110bc825897 ("net: usb: lan78xx: Convert to PHYLINK for improved PHY and MAC management")
Cc: stable@vger.kernel.org
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
---
 drivers/net/usb/lan78xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 5655941f1478..68e61004b3d7 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -4581,7 +4581,8 @@ static void lan78xx_disconnect(struct usb_interface *intf)
 	net = dev->net;
 
 	rtnl_lock();
-	phylink_stop(dev->phylink);
+	if (netif_running(net))
+		phylink_stop(dev->phylink);
 	phylink_disconnect_phy(dev->phylink);
 	rtnl_unlock();
 
-- 
2.34.1

Re: [PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[YuanZhe Liu]

Hi Andrew,

Could you please review this patch?

  [PATCH net] net: usb: lan78xx: only stop phylink if the interface is
  running

It calls phylink_stop() during lan78xx disconnect only when the network
interface is running.

The patch is unchanged. I am copying this address for visibility.

Original submission:
https://lore.kernel.org/r/20261029160204.2567-1-25031212351@stu.xidian.edu.cn

Thanks,
YuanZhe

Re: [PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[Andrew Lunn]

On Sat, Oct 31, 2026 at 01:38:41AM +0800, YuanZhe Liu wrote:
> Hi Andrew,
> 
> Could you please review this patch?

You should respond to what the sashiko-bot@kernel.org has said.

    Andrew

Re: [PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[Andrew Lunn]

On Fri, Oct 30, 2026 at 12:02:04AM +0800, YuanZhe Liu wrote:
> lan78xx_disconnect() unconditionally calls phylink_stop(), but
> phylink_start() is only called from lan78xx_open(). When the device is
> unplugged before the interface has been brought up, the attached PHY is
> still in PHY_READY and phy_stop() reports:
> 
>   called from state READY
>   WARNING: drivers/net/phy/phy.c:1670 at phy_stop+0xad/0x380
> 
> The same warning is emitted after an ifup/ifdown cycle, where the PHY is
> already in PHY_HALTED. Call phylink_stop() only when the network device is
> running, which is when phylink_start() has been called.
> 
> Fixes: e110bc825897 ("net: usb: lan78xx: Convert to PHYLINK for improved PHY and MAC management")

Why did you pick this commit?

    Andrew

Re: [PATCH net] net: usb: lan78xx: only stop phylink if the interface is running

[YuanZhe Liu]

On Fri, Oct 30, 2026 at 04:02:36PM +0000, sashiko-bot@kernel.org 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