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@xxxxxxxxxxxxxxx
> > > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > > ---
> > > 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
