* [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).