Messages in this thread | | | Date | Fri, 18 Mar 2016 10:21:21 -0400 (EDT) | From | Alan Stern <> | Subject | Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout |
| |
On Fri, 18 Mar 2016, Rajesh Bhagat wrote:
> --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2897,10 +2897,14 @@ done: > /* The xHC may think the device is already reset, > * so ignore the status. > */ > - if (hcd->driver->reset_device) > - hcd->driver->reset_device(hcd, udev); > - > - usb_set_device_state(udev, USB_STATE_DEFAULT); > + if (hcd->driver->reset_device) { > + status = hcd->driver->reset_device(hcd, udev); > + if (status == 0) > + usb_set_device_state(udev, USB_STATE_DEFAULT); > + else > + usb_set_device_state(udev, USB_STATE_NOTATTACHED); > + } else > + usb_set_device_state(udev, USB_STATE_DEFAULT);
This is a really bad patch:
You left in the comment about ignoring the status, but then you changed the code so that it doesn't ignore the status!
You also called usb_set_device_state() more times than necessary. You could have done it like this:
if (hcd->driver->reset_device) status = hcd->driver->reset_device(hcd, udev); if (status == 0) usb_set_device_state(udev, USB_STATE_DEFAULT); else usb_set_device_state(udev, USB_STATE_NOTATTACHED);
(Even that could be simplified further, by combining it with the code that follows.)
Finally, you violated the 80-column limit.
Alan Stern
| |