Re: [PATCH] usb: xhci: Fix incomplete PM resume operation due to XHCI commmand timeout

[Alan Stern]

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