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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to