From 22f6a0f0e3549b73b3319e26e1689f72b1f1284b Mon Sep 17 00:00:00 2001 From: Shaibal Dutta Date: Sat, 1 Feb 2014 19:16:46 -0800 Subject: usb: move hub init and LED blink work to power efficient workqueue Allow the scheduler to select the best CPU to handle hub initalization and LED blinking work. This extends idle residency times on idle CPUs and conserves power. This functionality is enabled when CONFIG_WQ_POWER_EFFICIENT is selected. [zoran.markovic@linaro.org: Rebased to latest kernel. Added commit message. Changed reference from system to power efficient workqueue for LEDs in check_highspeed() and hub_port_connect_change().] Acked-by: Alan Stern Cc: Sarah Sharp Cc: Xenia Ragiadakou Cc: Julius Werner Cc: Krzysztof Mazur Cc: Matthias Beyer Cc: Dan Williams Cc: Mathias Nyman Cc: Thomas Pugliese Signed-off-by: Shaibal Dutta Signed-off-by: Zoran Markovic Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 64ea21971be2..519f2c3594b2 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -499,7 +499,8 @@ static void led_work (struct work_struct *work) changed++; } if (changed) - schedule_delayed_work(&hub->leds, LED_CYCLE_PERIOD); + queue_delayed_work(system_power_efficient_wq, + &hub->leds, LED_CYCLE_PERIOD); } /* use a short timeout for hub/port status fetches */ @@ -1041,7 +1042,8 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) if (type == HUB_INIT) { delay = hub_power_on(hub, false); PREPARE_DELAYED_WORK(&hub->init_work, hub_init_func2); - schedule_delayed_work(&hub->init_work, + queue_delayed_work(system_power_efficient_wq, + &hub->init_work, msecs_to_jiffies(delay)); /* Suppress autosuspend until init is done */ @@ -1195,7 +1197,8 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) /* Don't do a long sleep inside a workqueue routine */ if (type == HUB_INIT2) { PREPARE_DELAYED_WORK(&hub->init_work, hub_init_func3); - schedule_delayed_work(&hub->init_work, + queue_delayed_work(system_power_efficient_wq, + &hub->init_work, msecs_to_jiffies(delay)); return; /* Continues at init3: below */ } else { @@ -1209,7 +1212,8 @@ static void hub_activate(struct usb_hub *hub, enum hub_activation_type type) if (status < 0) dev_err(hub->intfdev, "activate --> %d\n", status); if (hub->has_indicators && blinkenlights) - schedule_delayed_work(&hub->leds, LED_CYCLE_PERIOD); + queue_delayed_work(system_power_efficient_wq, + &hub->leds, LED_CYCLE_PERIOD); /* Scan all ports that need attention */ kick_khubd(hub); @@ -4311,7 +4315,8 @@ check_highspeed (struct usb_hub *hub, struct usb_device *udev, int port1) /* hub LEDs are probably harder to miss than syslog */ if (hub->has_indicators) { hub->indicator[port1-1] = INDICATOR_GREEN_BLINK; - schedule_delayed_work (&hub->leds, 0); + queue_delayed_work(system_power_efficient_wq, + &hub->leds, 0); } } kfree(qual); @@ -4540,7 +4545,9 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, if (hub->has_indicators) { hub->indicator[port1-1] = INDICATOR_AMBER_BLINK; - schedule_delayed_work (&hub->leds, 0); + queue_delayed_work( + system_power_efficient_wq, + &hub->leds, 0); } status = -ENOTCONN; /* Don't retry */ goto loop_disable; -- cgit v1.2.2 From 938569eb75b329fee9f2634900ad7e12caf13fd2 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Thu, 27 Feb 2014 10:57:10 +0100 Subject: hub: debug message for failing to enable device This error case isn't reported during enumeration. Signed-off-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 519f2c3594b2..69687de9de67 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4111,8 +4111,12 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1, did_new_scheme = true; retval = hub_enable_device(udev); - if (retval < 0) + if (retval < 0) { + dev_err(&udev->dev, + "hub failed to enable device, error %d\n", + retval); goto fail; + } #define GET_DESCRIPTOR_BUFSIZE 64 buf = kmalloc(GET_DESCRIPTOR_BUFSIZE, GFP_NOIO); -- cgit v1.2.2 From 23c058201fe36adf1a225281739b3ec31ec4e858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= Date: Thu, 27 Feb 2014 14:23:55 +0100 Subject: usb: hub: usb_ext_cap_descriptor.bmAttributes is le32 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Better check the correct bit on big endian systems too. Shuts up the following sparse __CHECK_ENDIAN__ warning: .../hub.c:3965:32: warning: restricted __le32 degrades to integer Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 69687de9de67..f6c6b6f7cc9c 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3962,7 +3962,7 @@ static void hub_set_initial_usb2_lpm_policy(struct usb_device *udev) connect_type = usb_get_hub_port_connect_type(udev->parent, udev->portnum); - if ((udev->bos->ext_cap->bmAttributes & USB_BESL_SUPPORT) || + if ((udev->bos->ext_cap->bmAttributes & cpu_to_le32(USB_BESL_SUPPORT)) || connect_type == USB_PORT_CONNECT_TYPE_HARD_WIRED) { udev->usb2_hw_lpm_allowed = 1; usb_set_usb2_hardware_lpm(udev, 1); -- cgit v1.2.2 From 25cd2882e2fc8bd8ed7acaee0ec979f11feda6d7 Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Fri, 17 Jan 2014 14:15:44 -0800 Subject: usb/xhci: Change how we indicate a host supports Link PM. The xHCI driver currently uses a USB core internal field, udev->lpm_capable, to indicate the xHCI driver knows how to calculate the LPM timeout values. If this value is set for the host controller udev, it means Link PM can be enabled for child devices under that host. Change the code so the xHCI driver isn't mucking with USB core internal fields. Instead, indicate the xHCI driver doesn't support Link PM on this host by clearing the U1 and U2 exit latencies in the roothub SuperSpeed Extended Capabilities BOS descriptor. The code to check for the roothub setting U1 and U2 exit latencies to zero will also disable LPM for external devices that do that same. This was already effectively done with commit ae8963adb4ad8c5f2a89ca1d99fb7bb721e7599f "usb: Don't enable LPM if the exit latency is zero." Leave that code in place, so that if a device sets one exit latency value to zero, but the other is set to a valid value, LPM is only enabled for the U1 or U2 state that had the valid value. This is the same behavior the code had before. Also, change messages about missing Link PM information from warning level to info level. Only print a warning about the first device that doesn't support LPM, to avoid log spam. Further, cleanup some unnecessary line breaks to help people to grep for the error messages. Signed-off-by: Sarah Sharp Cc: Alan Stern --- drivers/usb/core/hub.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index f6c6b6f7cc9c..763c3134dd78 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -141,19 +141,27 @@ static int usb_device_supports_lpm(struct usb_device *udev) return 0; } - /* All USB 3.0 must support LPM, but we need their max exit latency - * information from the SuperSpeed Extended Capabilities BOS descriptor. + /* + * According to the USB 3.0 spec, all USB 3.0 devices must support LPM. + * However, there are some that don't, and they set the U1/U2 exit + * latencies to zero. */ if (!udev->bos->ss_cap) { - dev_warn(&udev->dev, "No LPM exit latency info found. " - "Power management will be impacted.\n"); + dev_info(&udev->dev, "No LPM exit latency info found, disabling LPM.\n"); return 0; } - if (udev->parent->lpm_capable) - return 1; - dev_warn(&udev->dev, "Parent hub missing LPM exit latency info. " - "Power management will be impacted.\n"); + if (udev->bos->ss_cap->bU1devExitLat == 0 && + udev->bos->ss_cap->bU2DevExitLat == 0) { + if (udev->parent) + dev_info(&udev->dev, "LPM exit latency is zeroed, disabling LPM.\n"); + else + dev_info(&udev->dev, "We don't know the algorithms for LPM for this host, disabling LPM.\n"); + return 0; + } + + if (!udev->parent || udev->parent->lpm_capable) + return 1; return 0; } -- cgit v1.2.2 From 7a7b562d08ad6db98d6c8ec634620a11aaf8921a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 8 Nov 2013 16:37:26 +0100 Subject: usb: Clear host_endpoint->streams when implicitly freeing streams If streams are still allocated on device-reset or set-interface then the hcd code implictly frees the streams. Clear host_endpoint->streams in this case so that if a driver later tries to re-allocate them it won't run afoul of the device already having streams check in usb_alloc_streams(). Note normally streams still being allocated at reset / set-intf would be a driver bug, but this can happen without it being a driver bug on reset-resume. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hub.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 763c3134dd78..4f7629d1ba6a 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5131,7 +5131,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) struct usb_hcd *hcd = bus_to_hcd(udev->bus); struct usb_device_descriptor descriptor = udev->descriptor; struct usb_host_bos *bos; - int i, ret = 0; + int i, j, ret = 0; int port1 = udev->portnum; if (udev->state == USB_STATE_NOTATTACHED || @@ -5257,6 +5257,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev) ret); goto re_enumerate; } + /* Resetting also frees any allocated streams */ + for (j = 0; j < intf->cur_altsetting->desc.bNumEndpoints; j++) + intf->cur_altsetting->endpoint[j].streams = 0; } done: -- cgit v1.2.2 From a82b76f7fa6154e8ab2d8071842a3e38b9c0d0ff Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 8 Nov 2013 13:41:05 +0100 Subject: usb: Reset USB-3 devices on USB-3 link bounce On disconnect USB3 protocol ports transit from U0 to SS.Inactive to Rx.Detect, on a recoverable error, the port stays in SS.Inactive and we recover from it by doing a warm-reset (through usb_device_reset if we have a udev for the port). If this really is a disconnect we may end up trying the warm-reset anyways, since khubd may run before the SS.Inactive to Rx.Detect transition, or it may get skipped if the transition to Rx.Detect happens before khubd gets run. With a loose connector, or in the case which actually led me to debugging this bad ACPI firmware toggling Vbus off and on in quick succession, the port may transition from Rx.Detect to U0 again before khubd gets run. In this case the device state is unknown really, but khubd happily goes into the resuscitate an existing device path, and the device driver never gets notified about the device state being messed up. If the above scenario happens with a streams using device, as soon as an urb is submitted to an endpoint with streams, the following appears in dmesg: ERROR Transfer event for disabled endpoint or incorrect stream ring @0000000036807420 00000000 00000000 04000000 04078000 Notice how the TRB address is all zeros. I've seen this both on Intel Pantherpoint and Nec xhci hosts. Luckily we can detect the U0 to SS.Inactive to Rx.Detect to U0 all having happened before khubd runs case since the C_LINK_STATE bit gets set in the portchange bits on the U0 -> SS.Inactive change. This bit will also be set on suspend / resume, but then it gets cleared by port_hub_init before khubd runs. So if the C_LINK_STATE bit is set and a warm-reset is not needed, iow the port is not still in SS.Inactive, and the port still has a connection, then the device needs to be reset to put it back in a known state. I've verified that doing the device reset also fixes the transfer event with all zeros address issue. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hub.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 4f7629d1ba6a..5da5394127e9 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4758,6 +4758,8 @@ static void hub_events(void) /* deal with port status changes */ for (i = 1; i <= hdev->maxchild; i++) { + struct usb_device *udev = hub->ports[i - 1]->child; + if (test_bit(i, hub->busy_bits)) continue; connect_change = test_bit(i, hub->change_bits); @@ -4856,8 +4858,6 @@ 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); if (!udev || @@ -4874,6 +4874,24 @@ static void hub_events(void) usb_unlock_device(udev); connect_change = 0; } + /* + * On disconnect USB3 protocol ports transit from U0 to + * SS.Inactive to Rx.Detect. If this happens a warm- + * reset is not needed, but a (re)connect may happen + * before khubd runs and sees the disconnect, and the + * device may be an unknown state. + * + * If the port went through SS.Inactive without khubd + * seeing it the C_LINK_STATE change flag will be set, + * and we reset the dev to put it in a known state. + */ + } else if (udev && hub_is_superspeed(hub->hdev) && + (portchange & USB_PORT_STAT_C_LINK_STATE) && + (portstatus & USB_PORT_STAT_CONNECTION)) { + usb_lock_device(udev); + usb_reset_device(udev); + usb_unlock_device(udev); + connect_change = 0; } if (connect_change) -- cgit v1.2.2 From 6aec044cc2f5670cf3b143c151c8be846499bd15 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 12 Mar 2014 11:30:38 -0400 Subject: USB: unbind all interfaces before rebinding any When a driver doesn't have pre_reset, post_reset, or reset_resume methods, the USB core unbinds that driver when its device undergoes a reset or a reset-resume, and then rebinds it afterward. The existing straightforward implementation can lead to problems, because each interface gets unbound and rebound before the next interface is handled. If a driver claims additional interfaces, the claim may fail because the old binding instance may still own the additional interface when the new instance tries to claim it. This patch fixes the problem by first unbinding all the interfaces that are marked (i.e., their needs_binding flag is set) and then rebinding all of them. The patch also makes the helper functions in driver.c a little more uniform and adjusts some out-of-date comments. Signed-off-by: Alan Stern Reported-and-tested-by: "Poulain, Loic" Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 5da5394127e9..2d74dfb9c989 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -5380,10 +5380,11 @@ int usb_reset_device(struct usb_device *udev) else if (cintf->condition == USB_INTERFACE_BOUND) rebind = 1; + if (rebind) + cintf->needs_binding = 1; } - if (ret == 0 && rebind) - usb_rebind_intf(cintf); } + usb_unbind_and_rebind_marked_interfaces(udev); } usb_autosuspend_device(udev); -- cgit v1.2.2 From 1d10255c1c496557a5674e651c4ebbe0f61279f2 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Tue, 18 Mar 2014 10:39:05 -0400 Subject: USB: disable reset-resume when USB_QUIRK_RESET is set The USB_QUIRK_RESET flag indicates that a USB device changes its identity in some way when it is reset. It may lose its firmware, its descriptors may change, or it may switch back to a default mode of operation. If a device does this, the kernel needs to avoid resetting it. Resets are likely to fail, or worse, succeed while changing the device's state in a way the system can't detect. This means we should disable the reset-resume mechanism whenever this quirk flag is present. An attempted reset-resume will fail, the device will be logically disconnected, and later on the hub driver will rediscover and re-enumerate the device. This will cause the appropriate udev events to be generated, so that userspace will have a chance to switch the device into its normal operating mode, if necessary. Signed-off-by: Alan Stern CC: Oliver Neukum Reviewed-by: Hans de Goede Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 2d74dfb9c989..d670aaf13a3d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3105,9 +3105,19 @@ static int finish_port_resume(struct usb_device *udev) * operation is carried out here, after the port has been * resumed. */ - if (udev->reset_resume) + if (udev->reset_resume) { + /* + * If the device morphs or switches modes when it is reset, + * we don't want to perform a reset-resume. We'll fail the + * resume, which will cause a logical disconnect, and then + * the device will be rediscovered. + */ retry_reset_resume: - status = usb_reset_and_verify_device(udev); + if (udev->quirks & USB_QUIRK_RESET) + status = -ENODEV; + else + status = usb_reset_and_verify_device(udev); + } /* 10.5.4.5 says be sure devices in the tree are still there. * For now let's assume the device didn't go crazy on resume, -- cgit v1.2.2