* [PATCH] net: usb: asix: avoid to call phylink_stop() a second time
@ 2025-08-06 8:30 Xu Yang
2025-08-06 15:58 ` Andrew Lunn
0 siblings, 1 reply; 6+ messages in thread
From: Xu Yang @ 2025-08-06 8:30 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze,
khalasa, o.rempel
Cc: linux-usb, netdev, linux-kernel, imx, jun.li
The kernel will have below dump when system resume if the USB net device
was already disconnected during system suspend.
[ 46.392207] ------------[ cut here ]------------
[ 46.392216] called from state HALTED
[ 46.392255] WARNING: CPU: 0 PID: 56 at drivers/net/phy/phy.c:1630 phy_stop+0x12c/0x194
[ 46.392272] Modules linked in:
[ 46.392281] CPU: 0 UID: 0 PID: 56 Comm: kworker/0:3 Not tainted 6.15.0-rc7-next-20250523-06664-ga6888feb9f45-dirty #311 PREEMPT
[ 46.392287] Hardware name: NXP i.MX93 11X11 EVK board (DT)
[ 46.392291] Workqueue: usb_hub_wq hub_event
[ 46.392301] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 46.392306] pc : phy_stop+0x12c/0x194
[ 46.392311] lr : phy_stop+0x12c/0x194
[ 46.392315] sp : ffff8000828fb720
[ 46.392317] x29: ffff8000828fb720 x28: ffff000005558b50 x27: ffff00000555b400
[ 46.392324] x26: ffff000004e4f000 x25: ffff00000557f800 x24: 0000000000000000
[ 46.392331] x23: 0000000000000000 x22: ffff8000817eea10 x21: ffff000004fc5000
[ 46.392338] x20: ffff000004fc5a00 x19: ffff0000056eb000 x18: fffffffffffeb3c0
[ 46.392345] x17: ffff7ffffdc3c000 x16: ffff800080000000 x15: 0000000000000000
[ 46.392352] x14: 0000000000000000 x13: 206574617473206d x12: ffff800082057068
[ 46.392359] x11: 0000000000000058 x10: 0000000000000018 x9 : ffff800082057068
[ 46.392366] x8 : 0000000000000264 x7 : ffff8000820af068 x6 : ffff8000820af068
[ 46.392373] x5 : ffff00007fb80308 x4 : 0000000000000000 x3 : ffff7ffffdc3c000
[ 46.392379] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff000004ed4600
[ 46.392386] Call trace:
[ 46.392390] phy_stop+0x12c/0x194 (P)
[ 46.392396] phylink_stop+0x28/0x114
[ 46.392404] ax88772_stop+0x18/0x28
[ 46.392411] usbnet_stop+0x80/0x230
[ 46.392418] __dev_close_many+0xb4/0x1e0
[ 46.392427] dev_close_many+0x88/0x140
[ 46.392434] unregister_netdevice_many_notify+0x1b8/0xa10
[ 46.392440] unregister_netdevice_queue+0xe0/0xe8
[ 46.392445] unregister_netdev+0x24/0x50
[ 46.392450] usbnet_disconnect+0x50/0x124
[ 46.392457] usb_unbind_interface+0x78/0x2b4
[ 46.392463] device_remove+0x70/0x80
[ 46.392470] device_release_driver_internal+0x1cc/0x224
[ 46.392475] device_release_driver+0x18/0x30
[ 46.392480] bus_remove_device+0xc8/0x108
[ 46.392488] device_del+0x14c/0x420
[ 46.392495] usb_disable_device+0xe4/0x1c0
[ 46.392502] usb_disconnect+0xd8/0x2ac
[ 46.392508] hub_event+0x91c/0x1580
[ 46.392514] process_one_work+0x148/0x290
[ 46.392523] worker_thread+0x2c8/0x3e4
[ 46.392530] kthread+0x12c/0x204
[ 46.392536] ret_from_fork+0x10/0x20
[ 46.392545] ---[ end trace 0000000000000000 ]---
It's because usb_resume_interface() will be skipped if the USB core found
the USB device was already disconnected. In this case, asix_resume() will
not be called anymore. So asix_suspend/resume() can't be balanced. When
ax88772_stop() is called, the phy device was already stopped. To avoid
calling phylink_stop() a second time, check whether usb net device is
already in suspend state.
Fixes: e0bffe3e6894 ("net: asix: ax88772: migrate to phylink")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
drivers/net/usb/asix_devices.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 9b0318fb50b5..ac28f5fe7ac2 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -932,7 +932,8 @@ static int ax88772_stop(struct usbnet *dev)
{
struct asix_common_private *priv = dev->driver_priv;
- phylink_stop(priv->phylink);
+ if (!dev->suspend_count)
+ phylink_stop(priv->phylink);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: usb: asix: avoid to call phylink_stop() a second time 2025-08-06 8:30 [PATCH] net: usb: asix: avoid to call phylink_stop() a second time Xu Yang @ 2025-08-06 15:58 ` Andrew Lunn 2025-08-07 4:03 ` Xu Yang 0 siblings, 1 reply; 6+ messages in thread From: Andrew Lunn @ 2025-08-06 15:58 UTC (permalink / raw) To: Xu Yang Cc: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze, khalasa, o.rempel, linux-usb, netdev, linux-kernel, imx, jun.li On Wed, Aug 06, 2025 at 04:30:17PM +0800, Xu Yang wrote: > The kernel will have below dump when system resume if the USB net device > was already disconnected during system suspend. By disconnected, you mean pulled out? > It's because usb_resume_interface() will be skipped if the USB core found > the USB device was already disconnected. In this case, asix_resume() will > not be called anymore. So asix_suspend/resume() can't be balanced. When > ax88772_stop() is called, the phy device was already stopped. To avoid > calling phylink_stop() a second time, check whether usb net device is > already in suspend state. > > Fixes: e0bffe3e6894 ("net: asix: ax88772: migrate to phylink") > Cc: stable@vger.kernel.org > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > --- > drivers/net/usb/asix_devices.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > index 9b0318fb50b5..ac28f5fe7ac2 100644 > --- a/drivers/net/usb/asix_devices.c > +++ b/drivers/net/usb/asix_devices.c > @@ -932,7 +932,8 @@ static int ax88772_stop(struct usbnet *dev) > { > struct asix_common_private *priv = dev->driver_priv; > > - phylink_stop(priv->phylink); > + if (!dev->suspend_count) > + phylink_stop(priv->phylink); Looking at ax88172a.c, lan78xx.c and smsc95xx.c, they don't have anything like this. Is asix special, or are all the others broken as well? Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: asix: avoid to call phylink_stop() a second time 2025-08-06 15:58 ` Andrew Lunn @ 2025-08-07 4:03 ` Xu Yang 2025-08-15 8:15 ` Xu Yang 0 siblings, 1 reply; 6+ messages in thread From: Xu Yang @ 2025-08-07 4:03 UTC (permalink / raw) To: Andrew Lunn Cc: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze, khalasa, o.rempel, linux-usb, netdev, linux-kernel, imx, jun.li Hi Andrew, Thanks for your comments! On Wed, Aug 06, 2025 at 05:58:18PM +0200, Andrew Lunn wrote: > On Wed, Aug 06, 2025 at 04:30:17PM +0800, Xu Yang wrote: > > The kernel will have below dump when system resume if the USB net device > > was already disconnected during system suspend. > > By disconnected, you mean pulled out? Yes. > > > It's because usb_resume_interface() will be skipped if the USB core found > > the USB device was already disconnected. In this case, asix_resume() will > > not be called anymore. So asix_suspend/resume() can't be balanced. When > > ax88772_stop() is called, the phy device was already stopped. To avoid > > calling phylink_stop() a second time, check whether usb net device is > > already in suspend state. > > > > Fixes: e0bffe3e6894 ("net: asix: ax88772: migrate to phylink") > > Cc: stable@vger.kernel.org > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > > drivers/net/usb/asix_devices.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > > index 9b0318fb50b5..ac28f5fe7ac2 100644 > > --- a/drivers/net/usb/asix_devices.c > > +++ b/drivers/net/usb/asix_devices.c > > @@ -932,7 +932,8 @@ static int ax88772_stop(struct usbnet *dev) > > { > > struct asix_common_private *priv = dev->driver_priv; > > > > - phylink_stop(priv->phylink); > > + if (!dev->suspend_count) > > + phylink_stop(priv->phylink); > > Looking at ax88172a.c, lan78xx.c and smsc95xx.c, they don't have > anything like this. Is asix special, or are all the others broken as > well? I have limited USB net devices. So I can't test others now. But based on the error path, only below driver call phy_stop() or phylink_stop() in their stop() callback: drivers/net/usb/asix_devices.c ax88772_stop() phylink_stop() drivers/net/usb/ax88172a.c ax88172a_stop() phy_stop() drivers/net/usb/lan78xx.c lan78xx_stop() phylink_stop() drivers/net/usb/smsc95xx.c smsc95xx_stop() phy_stop() However, only asix_devices.c and lan78xx.c call phylink_suspend() in suspend() callback. So I think lan78xx.c has this issue too. Should I change usbnet common code like below? diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index c39dfa17813a..44a8d325dfb1 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -839,7 +839,7 @@ int usbnet_stop (struct net_device *net) pm = usb_autopm_get_interface(dev->intf); /* allow minidriver to stop correctly (wireless devices to turn off * radio etc) */ - if (info->stop) { + if (info->stop && !dev->suspend_count) { retval = info->stop(dev); if (retval < 0) netif_info(dev, ifdown, dev->net, Thanks, Xu Yang > > Andrew ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: asix: avoid to call phylink_stop() a second time 2025-08-07 4:03 ` Xu Yang @ 2025-08-15 8:15 ` Xu Yang 2025-08-15 12:59 ` Andrew Lunn 0 siblings, 1 reply; 6+ messages in thread From: Xu Yang @ 2025-08-15 8:15 UTC (permalink / raw) To: Andrew Lunn Cc: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze, khalasa, o.rempel, linux-usb, netdev, linux-kernel, imx, jun.li On Thu, Aug 07, 2025 at 12:03:39PM +0800, Xu Yang wrote: > Hi Andrew, > > Thanks for your comments! > > On Wed, Aug 06, 2025 at 05:58:18PM +0200, Andrew Lunn wrote: > > On Wed, Aug 06, 2025 at 04:30:17PM +0800, Xu Yang wrote: > > > The kernel will have below dump when system resume if the USB net device > > > was already disconnected during system suspend. > > > > By disconnected, you mean pulled out? > > Yes. > > > > > > It's because usb_resume_interface() will be skipped if the USB core found > > > the USB device was already disconnected. In this case, asix_resume() will > > > not be called anymore. So asix_suspend/resume() can't be balanced. When > > > ax88772_stop() is called, the phy device was already stopped. To avoid > > > calling phylink_stop() a second time, check whether usb net device is > > > already in suspend state. > > > > > > Fixes: e0bffe3e6894 ("net: asix: ax88772: migrate to phylink") > > > Cc: stable@vger.kernel.org > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > > --- > > > drivers/net/usb/asix_devices.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c > > > index 9b0318fb50b5..ac28f5fe7ac2 100644 > > > --- a/drivers/net/usb/asix_devices.c > > > +++ b/drivers/net/usb/asix_devices.c > > > @@ -932,7 +932,8 @@ static int ax88772_stop(struct usbnet *dev) > > > { > > > struct asix_common_private *priv = dev->driver_priv; > > > > > > - phylink_stop(priv->phylink); > > > + if (!dev->suspend_count) > > > + phylink_stop(priv->phylink); > > > > Looking at ax88172a.c, lan78xx.c and smsc95xx.c, they don't have > > anything like this. Is asix special, or are all the others broken as > > well? > > I have limited USB net devices. So I can't test others now. > > But based on the error path, only below driver call phy_stop() or phylink_stop() > in their stop() callback: > > drivers/net/usb/asix_devices.c > ax88772_stop() > phylink_stop() > > drivers/net/usb/ax88172a.c > ax88172a_stop() > phy_stop() > > drivers/net/usb/lan78xx.c > lan78xx_stop() > phylink_stop() > > drivers/net/usb/smsc95xx.c > smsc95xx_stop() > phy_stop() > > However, only asix_devices.c and lan78xx.c call phylink_suspend() in suspend() > callback. So I think lan78xx.c has this issue too. > > Should I change usbnet common code like below? > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > index c39dfa17813a..44a8d325dfb1 100644 > --- a/drivers/net/usb/usbnet.c > +++ b/drivers/net/usb/usbnet.c > @@ -839,7 +839,7 @@ int usbnet_stop (struct net_device *net) > pm = usb_autopm_get_interface(dev->intf); > /* allow minidriver to stop correctly (wireless devices to turn off > * radio etc) */ > - if (info->stop) { > + if (info->stop && !dev->suspend_count) { > retval = info->stop(dev); > if (retval < 0) > netif_info(dev, ifdown, dev->net, Do you mind sharing some suggestions on this? Thanks in advance! Thanks, Xu Yang > > Thanks, > Xu Yang > > > > > Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: asix: avoid to call phylink_stop() a second time 2025-08-15 8:15 ` Xu Yang @ 2025-08-15 12:59 ` Andrew Lunn 2025-08-18 6:44 ` Xu Yang 0 siblings, 1 reply; 6+ messages in thread From: Andrew Lunn @ 2025-08-15 12:59 UTC (permalink / raw) To: Xu Yang Cc: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze, khalasa, o.rempel, linux-usb, netdev, linux-kernel, imx, jun.li > > > Looking at ax88172a.c, lan78xx.c and smsc95xx.c, they don't have > > > anything like this. Is asix special, or are all the others broken as > > > well? > > > > I have limited USB net devices. So I can't test others now. > > > > But based on the error path, only below driver call phy_stop() or phylink_stop() > > in their stop() callback: > > > > drivers/net/usb/asix_devices.c > > ax88772_stop() > > phylink_stop() > > > > drivers/net/usb/ax88172a.c > > ax88172a_stop() > > phy_stop() > > > > drivers/net/usb/lan78xx.c > > lan78xx_stop() > > phylink_stop() > > > > drivers/net/usb/smsc95xx.c > > smsc95xx_stop() > > phy_stop() > > > > However, only asix_devices.c and lan78xx.c call phylink_suspend() in suspend() > > callback. So I think lan78xx.c has this issue too. > > > > Should I change usbnet common code like below? > > > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > > index c39dfa17813a..44a8d325dfb1 100644 > > --- a/drivers/net/usb/usbnet.c > > +++ b/drivers/net/usb/usbnet.c > > @@ -839,7 +839,7 @@ int usbnet_stop (struct net_device *net) > > pm = usb_autopm_get_interface(dev->intf); > > /* allow minidriver to stop correctly (wireless devices to turn off > > * radio etc) */ > > - if (info->stop) { > > + if (info->stop && !dev->suspend_count) { > > retval = info->stop(dev); > > if (retval < 0) > > netif_info(dev, ifdown, dev->net, > > Do you mind sharing some suggestions on this? Thanks in advance! It does look to be a common problem, so solving it in usbnet would be best. Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: usb: asix: avoid to call phylink_stop() a second time 2025-08-15 12:59 ` Andrew Lunn @ 2025-08-18 6:44 ` Xu Yang 0 siblings, 0 replies; 6+ messages in thread From: Xu Yang @ 2025-08-18 6:44 UTC (permalink / raw) To: Andrew Lunn Cc: andrew+netdev, davem, edumazet, kuba, pabeni, max.schulze, khalasa, o.rempel, linux-usb, netdev, linux-kernel, imx, jun.li On Fri, Aug 15, 2025 at 02:59:12PM +0200, Andrew Lunn wrote: > > > > Looking at ax88172a.c, lan78xx.c and smsc95xx.c, they don't have > > > > anything like this. Is asix special, or are all the others broken as > > > > well? > > > > > > I have limited USB net devices. So I can't test others now. > > > [...] > > > > > > Should I change usbnet common code like below? > > > > > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > > > index c39dfa17813a..44a8d325dfb1 100644 > > > --- a/drivers/net/usb/usbnet.c > > > +++ b/drivers/net/usb/usbnet.c > > > @@ -839,7 +839,7 @@ int usbnet_stop (struct net_device *net) > > > pm = usb_autopm_get_interface(dev->intf); > > > /* allow minidriver to stop correctly (wireless devices to turn off > > > * radio etc) */ > > > - if (info->stop) { > > > + if (info->stop && !dev->suspend_count) { > > > retval = info->stop(dev); > > > if (retval < 0) > > > netif_info(dev, ifdown, dev->net, > > > > Do you mind sharing some suggestions on this? Thanks in advance! > > It does look to be a common problem, so solving it in usbnet would be > best. Okay. Thank you! Thanks, Xu Yang > > Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-18 6:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-06 8:30 [PATCH] net: usb: asix: avoid to call phylink_stop() a second time Xu Yang 2025-08-06 15:58 ` Andrew Lunn 2025-08-07 4:03 ` Xu Yang 2025-08-15 8:15 ` Xu Yang 2025-08-15 12:59 ` Andrew Lunn 2025-08-18 6:44 ` Xu Yang
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).