On Wed, 21 Nov 2012, Sarah Sharp wrote:
> A device may need to be reset several times during enumeration, and this
> causes a disconnect between the USB core's device state and the xHC's
> device state.
>
> For example, usb_reset_and_verify_device calls into hub_port_init in a
> loop. Say that on the first call into hub_port_init, the device is
> successfully reset, but doesn't respond to several set address control
> transfers. Then the port will be disabled, but the udev will remain in
> tact. usb_reset_and_verify_device will call into hub_port_init again.
>
> The problem is that the xHC was sent a Reset Device command after the
> first port reset. At that point, it thinks the device is in the Default
> state. The failed Set Address commands do not move the device from the
> Default state. On the second port reset, the Reset Device command is
> issued again. Since the xHC thinks device is already in the Default
> state, it will reject the second Reset Device command.
That's kind of strange. Why shouldn't the computer be allowed to reset
a device twice in a row? The hardware's "xHC knows best" attitude is
a little annoying...
> This will cause
> the port reset to fail until the device is logically disconnected.
>
> Fix this by ignoring the return value of the HCD reset_device callback.
Does this really fix anything? Since the device can't be reset after
the first attempt, the end result is bound to be a failure anyway.
Would it be simpler to just forget about the loop (set the number of
tries to 1) in usb_reset_and_verify_device for SuperSpeed devices?
> This commit should be backported to kernels as old as 3.2, that contain
> the commit 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc "usbcore: refine
> warm reset logic".
>
> Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/usb/core/hub.c | 13 +++++--------
> 1 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 7f8f10e..3bc50fc 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2565,14 +2565,11 @@ static void hub_port_finish_reset(struct usb_hub *hub, int port1,
> msleep(10 + 40);
> update_devnum(udev, 0);
> hcd = bus_to_hcd(udev->bus);
> - if (hcd->driver->reset_device) {
> - *status = hcd->driver->reset_device(hcd, udev);
> - if (*status < 0) {
> - dev_err(&udev->dev, "Cannot reset "
> - "HCD device state\n");
> - break;
> - }
> - }
> + /* The xHC may think the device is already reset,
> + * so ignore the status.
> + */
When adding new comments, stick to the recommended format:
/*
* Comment
* more comment
*/
Don't worry if this clashes with comments that are already present.
Also, it would be best not to mention xHC here. In theory, other
controller types might implement a reset_device method. "The HC ..."
would be better.
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
