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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
