When a hot reset fails on a USB 3.0 port, the current port reset code recursively calls hub_port_reset inside hub_port_wait_reset. This isn't ideal, since we should avoid recursive calls in the kernel, and it also doesn't allow us to issue multiple warm resets on reset failures. Rip out the recursive call. Instead, add code to hub_port_reset to issue a warm reset if the hot reset fails, and try multiple warm resets before giving up on the port. In hub_port_wait_reset, remove the recursive call and re-indent. The code is basically the same, except: 1. It bails out early if the port has transitioned to Inactive or Compliance Mode after the reset completed. 2. It doesn't consider a connect status change to be a failed reset. If multiple warm resets needed to be issued, the connect status may have changed, so we need to ignore that and look at the port link state instead. hub_port_reset will now do that. 3. It unconditionally sets udev->speed on all types of successful resets. The old recursive code would set the port speed when the second hub_port_reset returned. The old code did not handle connected devices needing a warm reset well. There were only two situations that the old code handled correctly: an empty port needing a warm reset, and a hot reset that migrated to a warm reset. When an empty port needed a warm reset, hub_port_reset was called with the warm variable set. The code in hub_port_finish_reset would skip telling the USB core and the xHC host that the device was reset, because otherwise that would result in a NULL pointer dereference. When a USB 3.0 device reset migrated to a warm reset, the recursive call made the call stack look like this: hub_port_reset(warm = false) hub_wait_port_reset(warm = false) hub_port_reset(warm = true) hub_wait_port_reset(warm = true) hub_port_finish_reset(warm = true) (return up the call stack to the first wait) hub_port_finish_reset(warm = false) The old code didn't want to notify the USB core or the xHC host of device reset twice, so it only did it in the second call to hub_port_finish_reset, when warm was set to false. This was necessary because before patch two ("USB: Ignore xHCI Reset Device status."), the USB core would pay attention to the xHC Reset Device command error status, and the second call would always fail. Now that we no longer have the recursive call, and warm can change from false to true in hub_port_reset, we need to have hub_port_finish_reset unconditionally notify the USB core and the xHC of the device reset. In hub_port_finish_reset, unconditionally clear the connect status change (CSC) bit for USB 3.0 hubs when the port reset is done. If we had to issue multiple warm resets for a device, that bit may have been set if the device went into SS.Inactive and then was successfully warm reset. Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> --- drivers/usb/core/hub.c | 150 ++++++++++++++++++++++-------------------------- 1 files changed, 68 insertions(+), 82 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c3368f9..d1fceb2 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2538,73 +2538,35 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, if ((portstatus & USB_PORT_STAT_RESET)) goto delay; - /* - * Some buggy devices require a warm reset to be issued even - * when the port appears not to be connected. + if (hub_port_warm_reset_required(hub, portstatus)) + return -ENOTCONN; + + /* Device went away? */ + if (!(portstatus & USB_PORT_STAT_CONNECTION)) + return -ENOTCONN; + + /* bomb out completely if the connection bounced. A USB 3.0 + * connection may bounce if multiple warm resets were issued, + * but the device may have successfully re-connected. Ignore it. */ - if (!warm) { - /* - * Some buggy devices can cause an NEC host controller - * to transition to the "Error" state after a hot port - * reset. This will show up as the port state in - * "Inactive", and the port may also report a - * disconnect. Forcing a warm port reset seems to make - * the device work. - * - * See https://bugzilla.kernel.org/show_bug.cgi?id=41752 - */ - if (hub_port_warm_reset_required(hub, portstatus)) { - int ret; - - if ((portchange & USB_PORT_STAT_C_CONNECTION)) - clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_CONNECTION); - if (portchange & USB_PORT_STAT_C_LINK_STATE) - clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_PORT_LINK_STATE); - if (portchange & USB_PORT_STAT_C_RESET) - clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_RESET); - dev_dbg(hub->intfdev, "hot reset failed, warm reset port %d\n", - port1); - ret = hub_port_reset(hub, port1, - udev, HUB_BH_RESET_TIME, - true); - if ((portchange & USB_PORT_STAT_C_CONNECTION)) - clear_port_feature(hub->hdev, port1, - USB_PORT_FEAT_C_CONNECTION); - return ret; - } - /* Device went away? */ - if (!(portstatus & USB_PORT_STAT_CONNECTION)) - return -ENOTCONN; - - /* bomb out completely if the connection bounced */ - if ((portchange & USB_PORT_STAT_C_CONNECTION)) - return -ENOTCONN; - - if ((portstatus & USB_PORT_STAT_ENABLE)) { - if (!udev) - return 0; - - if (hub_is_wusb(hub)) - udev->speed = USB_SPEED_WIRELESS; - else if (hub_is_superspeed(hub->hdev)) - udev->speed = USB_SPEED_SUPER; - else if (portstatus & USB_PORT_STAT_HIGH_SPEED) - udev->speed = USB_SPEED_HIGH; - else if (portstatus & USB_PORT_STAT_LOW_SPEED) - udev->speed = USB_SPEED_LOW; - else - udev->speed = USB_SPEED_FULL; + if (!hub_is_superspeed(hub->hdev) && + (portchange & USB_PORT_STAT_C_CONNECTION)) + return -ENOTCONN; + + if ((portstatus & USB_PORT_STAT_ENABLE)) { + if (!udev) return 0; - } - } else { - if (!(portstatus & USB_PORT_STAT_CONNECTION) || - hub_port_warm_reset_required(hub, - portstatus)) - return -ENOTCONN; + if (hub_is_wusb(hub)) + udev->speed = USB_SPEED_WIRELESS; + else if (hub_is_superspeed(hub->hdev)) + udev->speed = USB_SPEED_SUPER; + else if (portstatus & USB_PORT_STAT_HIGH_SPEED) + udev->speed = USB_SPEED_HIGH; + else if (portstatus & USB_PORT_STAT_LOW_SPEED) + udev->speed = USB_SPEED_LOW; + else + udev->speed = USB_SPEED_FULL; return 0; } @@ -2622,23 +2584,21 @@ delay: } static void hub_port_finish_reset(struct usb_hub *hub, int port1, - struct usb_device *udev, int *status, bool warm) + struct usb_device *udev, int *status) { switch (*status) { case 0: - if (!warm) { - struct usb_hcd *hcd; - /* TRSTRCY = 10 ms; plus some extra */ - msleep(10 + 40); - if (udev) { - update_devnum(udev, 0); - hcd = bus_to_hcd(udev->bus); - /* The xHC may think the device is already - * reset, so ignore the status. - */ - if (hcd->driver->reset_device) - hcd->driver->reset_device(hcd, udev); - } + /* TRSTRCY = 10 ms; plus some extra */ + msleep(10 + 40); + if (udev) { + struct usb_hcd *hcd = bus_to_hcd(udev->bus); + + update_devnum(udev, 0); + /* The xHC may think the device is already reset, + * so ignore the status. + */ + if (hcd->driver->reset_device) + hcd->driver->reset_device(hcd, udev); } /* FALL THROUGH */ case -ENOTCONN: @@ -2651,8 +2611,10 @@ static void hub_port_finish_reset(struct usb_hub *hub, int port1, USB_PORT_FEAT_C_BH_PORT_RESET); clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_PORT_LINK_STATE); + clear_port_feature(hub->hdev, port1, + USB_PORT_FEAT_C_CONNECTION); } - if (!warm && udev) + if (udev) usb_set_device_state(udev, *status ? USB_STATE_NOTATTACHED : USB_STATE_DEFAULT); @@ -2665,6 +2627,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1, struct usb_device *udev, unsigned int delay, bool warm) { int i, status; + u16 portchange, portstatus; if (!hub_is_superspeed(hub->hdev)) { if (warm) { @@ -2696,10 +2659,33 @@ static int hub_port_reset(struct usb_hub *hub, int port1, status); } - /* return on disconnect or reset */ + /* Check for disconnect or reset */ if (status == 0 || status == -ENOTCONN || status == -ENODEV) { - hub_port_finish_reset(hub, port1, udev, &status, warm); - goto done; + hub_port_finish_reset(hub, port1, udev, &status); + + if (!hub_is_superspeed(hub->hdev)) + goto done; + + /* + * If a USB 3.0 device migrates from reset to an error + * state, re-issue the warm reset. + */ + if (hub_port_status(hub, port1, + &portstatus, &portchange) < 0) + goto done; + + if (!hub_port_warm_reset_required(hub, portstatus)) + goto done; + + /* + * If the port is in SS.Inactive or Compliance Mode, the + * hot or warm reset failed. Try another warm reset. + */ + if (!warm) { + dev_dbg(hub->intfdev, "hot reset failed, warm reset port %d\n", + port1); + warm = true; + } } dev_dbg (hub->intfdev, -- 1.7.9 -- 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