On Mon, Aug 18, 2025 at 11:04:55AM +0200, Oliver Neukum wrote:
> On 8/18/25 09:57, Xu Yang wrote:
>
> > --- 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);
>
> This needs to fail ...
It returns 0, so "pm = 0" here which means it succeed.
>
> > /* allow minidriver to stop correctly (wireless devices to turn off
> > * radio etc) */
> > - if (info->stop) {
> > + if (info->stop && !dev->suspend_count) {
>
> ... for !dev->suspend_count to be false
The suspend_count won't go to 0 because there is no chance to call
usbnet_resume() if the USB device is physically disconnected from the
USB port during system suspend.
>
> > retval = info->stop(dev);
> > if (retval < 0)
> > netif_info(dev, ifdown, dev->net,
>
> In other words, this means that the driver has insufficient
> error handling in this method. This needs to be fixed and it
> needs to be fixed explicitly. We do not hide error handling.
Do you mean info->stop() should be called and return error code if something
is wrong?
>
> Please use a literal "if (pm < 0)" to skip the parts we need to skip
> if the resumption fails.
pm = 0 here.
Thanks,
Xu Yang
>
> Regards
> Oliver
>
> NACKED-BY: Oliver Neukum <oneukum@xxxxxxxx>
