From 0fe51aa5eee51db7c7ecd201d42a977ad79c58b6 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 1 Nov 2012 11:20:44 -0700 Subject: [PATCH 1/7] USB: Don't use EHCI port sempahore for USB 3.0 hubs. The EHCI host controller needs to prevent EHCI initialization when the UHCI or OHCI companion controller is in the middle of a port reset. It uses ehci_cf_port_reset_rwsem to do this. USB 3.0 hubs can't be under an EHCI host controller, so it makes no sense to down the semaphore for USB 3.0 hubs. It also makes the warm port reset code more complex. Don't down ehci_cf_port_reset_rwsem for USB 3.0 hubs. Signed-off-by: Sarah Sharp Acked-by: Alan Stern --- drivers/usb/core/hub.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 9641e9c1dec5..855bb0d6313d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2661,17 +2661,16 @@ static int hub_port_reset(struct usb_hub *hub, int port1, { int i, status; - if (!warm) { - /* Block EHCI CF initialization during the port reset. - * Some companion controllers don't like it when they mix. - */ - down_read(&ehci_cf_port_reset_rwsem); - } else { - if (!hub_is_superspeed(hub->hdev)) { + if (!hub_is_superspeed(hub->hdev)) { + if (warm) { dev_err(hub->intfdev, "only USB3 hub support " "warm reset\n"); return -EINVAL; } + /* Block EHCI CF initialization during the port reset. + * Some companion controllers don't like it when they mix. + */ + down_read(&ehci_cf_port_reset_rwsem); } /* Reset the port */ @@ -2709,7 +2708,7 @@ static int hub_port_reset(struct usb_hub *hub, int port1, port1); done: - if (!warm) + if (!hub_is_superspeed(hub->hdev)) up_read(&ehci_cf_port_reset_rwsem); return status; From 2d4fa940f99663c82ba55b2244638833b388e4e2 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Tue, 4 Dec 2012 15:27:50 -0800 Subject: [PATCH 2/7] USB: Prepare for refactoring by adding extra udev checks. The next patch will refactor the hub port code to rip out the recursive call to hub_port_reset on a failed hot reset. In preparation for that, make sure all code paths can deal with being called with a NULL udev. The usb_device will not be valid if warm reset was issued because a port transitioned to the Inactive or Compliance Mode on a device connect. This patch should have no effect on current behavior. Signed-off-by: Sarah Sharp Acked-by: Alan Stern --- drivers/usb/core/hub.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 855bb0d6313d..c3368f978c44 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2584,6 +2584,9 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, 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)) @@ -2627,13 +2630,15 @@ static void hub_port_finish_reset(struct usb_hub *hub, int port1, struct usb_hcd *hcd; /* TRSTRCY = 10 ms; plus some extra */ msleep(10 + 40); - 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); + 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); + } } /* FALL THROUGH */ case -ENOTCONN: @@ -2647,7 +2652,7 @@ static void hub_port_finish_reset(struct usb_hub *hub, int port1, clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_PORT_LINK_STATE); } - if (!warm) + if (!warm && udev) usb_set_device_state(udev, *status ? USB_STATE_NOTATTACHED : USB_STATE_DEFAULT); From a24a6078754f28528bc91e7e7b3e6ae86bd936d8 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 1 Nov 2012 11:20:44 -0700 Subject: [PATCH 3/7] USB: Rip out recursive call on warm port reset. 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 Acked-by: Alan Stern --- drivers/usb/core/hub.c | 150 +++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 82 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index c3368f978c44..d1fceb219d97 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 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, } 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, From d3b9d7a9051d7024a93c76a84b2f84b3b66ad6d5 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Thu, 1 Nov 2012 11:20:44 -0700 Subject: [PATCH 4/7] USB: Fix connected device switch to Inactive state. A USB 3.0 device can transition to the Inactive state if a U1 or U2 exit transition fails. The current code in hub_events simply issues a warm reset, but does not call any pre-reset or post-reset driver methods (or unbind/rebind drivers without them). Therefore the drivers won't know their device has just been reset. hub_events should instead call usb_reset_device. This means hub_port_reset now needs to figure out whether it should issue a warm reset or a hot reset. Remove the FIXME note about needing disconnect() for a NOTATTACHED device. This patch fixes that. Signed-off-by: Sarah Sharp Acked-by: Alan Stern --- drivers/usb/core/hub.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d1fceb219d97..b00f10956b00 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2605,7 +2605,6 @@ static void hub_port_finish_reset(struct usb_hub *hub, int port1, case -ENODEV: clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_RESET); - /* FIXME need disconnect() for NOTATTACHED device */ if (hub_is_superspeed(hub->hdev)) { clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_C_BH_PORT_RESET); @@ -2639,6 +2638,18 @@ static int hub_port_reset(struct usb_hub *hub, int port1, * Some companion controllers don't like it when they mix. */ down_read(&ehci_cf_port_reset_rwsem); + } else if (!warm) { + /* + * If the caller hasn't explicitly requested a warm reset, + * double check and see if one is needed. + */ + status = hub_port_status(hub, port1, + &portstatus, &portchange); + if (status < 0) + goto done; + + if (hub_port_warm_reset_required(hub, portstatus)) + warm = true; } /* Reset the port */ @@ -4690,12 +4701,21 @@ static void hub_events(void) */ if (hub_port_warm_reset_required(hub, portstatus)) { int status; + struct usb_device *udev = + hub->ports[i - 1]->child; dev_dbg(hub_dev, "warm reset port %d\n", i); - status = hub_port_reset(hub, i, NULL, - HUB_BH_RESET_TIME, true); - if (status < 0) - hub_port_disable(hub, i, 1); + if (!udev) { + status = hub_port_reset(hub, i, + NULL, HUB_BH_RESET_TIME, + true); + if (status < 0) + hub_port_disable(hub, i, 1); + } else { + usb_lock_device(udev); + status = usb_reset_device(udev); + usb_unlock_device(udev); + } connect_change = 0; } From c2f60db740f2935a5e8a348298b590f2462e952f Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Mon, 10 Dec 2012 15:43:08 -0800 Subject: [PATCH 5/7] USB: Use helper function hub_set_port_link_state Change the code that manually issues a Set Port Feature(Link State) to use the new helper function hub_set_port_link_state(). Signed-off-by: Sarah Sharp Acked-by: Alan Stern --- drivers/usb/core/hub.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index b00f10956b00..be976ee84b8e 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2946,9 +2946,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg) /* see 7.1.7.6 */ if (hub_is_superspeed(hub->hdev)) - status = set_port_feature(hub->hdev, - port1 | (USB_SS_PORT_LS_U3 << 3), - USB_PORT_FEAT_LINK_STATE); + status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U3); else status = set_port_feature(hub->hdev, port1, USB_PORT_FEAT_SUSPEND); @@ -3118,9 +3116,7 @@ int usb_port_resume(struct usb_device *udev, pm_message_t msg) /* see 7.1.7.7; affects power usage, but not budgeting */ if (hub_is_superspeed(hub->hdev)) - status = set_port_feature(hub->hdev, - port1 | (USB_SS_PORT_LS_U0 << 3), - USB_PORT_FEAT_LINK_STATE); + status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U0); else status = clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_SUSPEND); From 470f0be8aa78034030d3e05580e85ba7d0e26da7 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Tue, 11 Dec 2012 10:14:03 -0800 Subject: [PATCH 6/7] USB: Refactor hub_port_wait_reset. Refactor hub_port_wait_reset into a small loop to wait for the port reset to be complete, and then a larger block to deal with the final port status. This patch should not change any current behavior. Signed-off-by: Sarah Sharp Acked-by: Alan Stern --- drivers/usb/core/hub.c | 73 +++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 36 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index be976ee84b8e..ae10862fb041 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2535,42 +2535,9 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, return ret; /* The port state is unknown until the reset completes. */ - if ((portstatus & USB_PORT_STAT_RESET)) - goto delay; - - 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 (!hub_is_superspeed(hub->hdev) && - (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; - return 0; - } + if (!(portstatus & USB_PORT_STAT_RESET)) + break; -delay: /* switch to the long delay after two short delay failures */ if (delay_time >= 2 * HUB_SHORT_RESET_TIME) delay = HUB_LONG_RESET_TIME; @@ -2580,7 +2547,41 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, port1, warm ? "warm " : "", delay); } - return -EBUSY; + if ((portstatus & USB_PORT_STAT_RESET)) + return -EBUSY; + + 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 (!hub_is_superspeed(hub->hdev) && + (portchange & USB_PORT_STAT_C_CONNECTION)) + return -ENOTCONN; + + if (!(portstatus & USB_PORT_STAT_ENABLE)) + return -EBUSY; + + 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; + return 0; } static void hub_port_finish_reset(struct usb_hub *hub, int port1, From 026630d09ba7b6cd44a43b92ad256c28a9e3ddc2 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Sun, 16 Dec 2012 04:21:31 +0100 Subject: [PATCH 7/7] usb: host: xhci: remove unused trb var in xhci_irq() The union xhci_trb *trb variable is defined and assigned inside the xHCI IRQ handler function but is never used. Signed-off-by: Javier Martinez Canillas Signed-off-by: Sarah Sharp --- drivers/usb/host/xhci-ring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 59fb5c677dbe..f69720983fc7 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2706,13 +2706,11 @@ irqreturn_t xhci_irq(struct usb_hcd *hcd) { struct xhci_hcd *xhci = hcd_to_xhci(hcd); u32 status; - union xhci_trb *trb; u64 temp_64; union xhci_trb *event_ring_deq; dma_addr_t deq; spin_lock(&xhci->lock); - trb = xhci->event_ring->dequeue; /* Check if the xHC generated the interrupt, or the irq is shared */ status = xhci_readl(xhci, &xhci->op_regs->status); if (status == 0xffffffff)