From 31c6bf70955dda6ef92ab40624f289576cff97d3 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Sat, 11 Jan 2014 02:04:00 +0100 Subject: usb: core: let dynamic ids override static ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This modifies the probing order so that any matching dynamic entry always will be used, even if the driver has a matching static entry. It is sometimes useful to dynamically update existing device entries. With the new ability to set the dynamic entry driver_info field, this can be used to test new additions to class driver exception lists or proposed changes to existing static per-device driver_info entries. Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 5d01558cef66..9cd218135087 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -302,9 +302,9 @@ static int usb_probe_interface(struct device *dev) return error; } - id = usb_match_id(intf, driver->id_table); + id = usb_match_dynamic_id(intf, driver); if (!id) - id = usb_match_dynamic_id(intf, driver); + id = usb_match_id(intf, driver->id_table); if (!id) return error; -- cgit v1.2.3-58-ga151 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') 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.3-58-ga151 From f080a51bef2caa9b0f647dc430bc608d5723ac29 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 20 Feb 2014 10:49:30 -0500 Subject: USB: complain if userspace resets an active endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is an error for a driver to call usb_clear_halt() or usb_reset_endpoint() while there are URBs queued for the endpoint, because the end result is not well defined. At the time the endpoint gets reset, it may or may not be actively running. As far as I know, no kernel drivers do this. But some userspace drivers do, and it seems like a good idea to bring this error to their attention. This patch adds a warning to the kernel log whenever a program invokes the USBDEVFS_CLEAR_HALT or USBDEVFS_RESETEP ioctls at an inappropriate time, and includes the name of the program. This will make it clear that any subsequent errors are not due to the misbehavior of a kernel driver. Signed-off-by: Alan Stern Suggested-by: Bjørn Mork CC: Stanislaw Gruszka Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 90e18f6fa2bb..f3ba2e076ee3 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1043,6 +1043,20 @@ static int proc_bulk(struct dev_state *ps, void __user *arg) return ret; } +static void check_reset_of_active_ep(struct usb_device *udev, + unsigned int epnum, char *ioctl_name) +{ + struct usb_host_endpoint **eps; + struct usb_host_endpoint *ep; + + eps = (epnum & USB_DIR_IN) ? udev->ep_in : udev->ep_out; + ep = eps[epnum & 0x0f]; + if (ep && !list_empty(&ep->urb_list)) + dev_warn(&udev->dev, "Process %d (%s) called USBDEVFS_%s for active endpoint 0x%02x\n", + task_pid_nr(current), current->comm, + ioctl_name, epnum); +} + static int proc_resetep(struct dev_state *ps, void __user *arg) { unsigned int ep; @@ -1056,6 +1070,7 @@ static int proc_resetep(struct dev_state *ps, void __user *arg) ret = checkintf(ps, ret); if (ret) return ret; + check_reset_of_active_ep(ps->dev, ep, "RESETEP"); usb_reset_endpoint(ps->dev, ep); return 0; } @@ -1074,6 +1089,7 @@ static int proc_clearhalt(struct dev_state *ps, void __user *arg) ret = checkintf(ps, ret); if (ret) return ret; + check_reset_of_active_ep(ps->dev, ep, "CLEAR_HALT"); if (ep & USB_DIR_IN) pipe = usb_rcvbulkpipe(ps->dev, ep & 0x7f); else -- cgit v1.2.3-58-ga151 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') 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.3-58-ga151 From 23c058201fe36adf1a225281739b3ec31ec4e858 Mon Sep 17 00:00:00 2001 From: Bjørn Mork 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') 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.3-58-ga151 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 ++++++++++++++++-------- drivers/usb/host/xhci-hub.c | 8 +++++--- drivers/usb/host/xhci-pci.c | 6 ------ 3 files changed, 21 insertions(+), 17 deletions(-) (limited to 'drivers/usb/core') 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; } diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 9992fbfec85f..1ad6bc1951c7 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -732,9 +732,11 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, /* Set the U1 and U2 exit latencies. */ memcpy(buf, &usb_bos_descriptor, USB_DT_BOS_SIZE + USB_DT_USB_SS_CAP_SIZE); - temp = readl(&xhci->cap_regs->hcs_params3); - buf[12] = HCS_U1_LATENCY(temp); - put_unaligned_le16(HCS_U2_LATENCY(temp), &buf[13]); + if ((xhci->quirks & XHCI_LPM_SUPPORT)) { + temp = readl(&xhci->cap_regs->hcs_params3); + buf[12] = HCS_U1_LATENCY(temp); + put_unaligned_le16(HCS_U2_LATENCY(temp), &buf[13]); + } /* Indicate whether the host has LTM support. */ temp = readl(&xhci->cap_regs->hcc_params); diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 04f986d9234f..6c03584ac15f 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -222,12 +222,6 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) goto put_usb3_hcd; /* Roothub already marked as USB 3.0 speed */ - /* We know the LPM timeout algorithms for this host, let the USB core - * enable and disable LPM for devices under the USB 3.0 roothub. - */ - if (xhci->quirks & XHCI_LPM_SUPPORT) - hcd_to_bus(xhci->shared_hcd)->root_hub->lpm_capable = 1; - return 0; put_usb3_hcd: -- cgit v1.2.3-58-ga151 From 12d4bbcea727710bbd04de3e1de05957a0675e60 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:23 +0200 Subject: usb-core: Fix usb_free_streams return value documentation Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hcd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 2518c3250750..9b445f43f825 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2078,8 +2078,7 @@ EXPORT_SYMBOL_GPL(usb_alloc_streams); * Reverts a group of bulk endpoints back to not using stream IDs. * Can fail if we are given bad arguments, or HCD is broken. * - * Return: On success, the number of allocated streams. On failure, a negative - * error code. + * Return: 0 on success. On failure, a negative error code. */ int usb_free_streams(struct usb_interface *interface, struct usb_host_endpoint **eps, unsigned int num_eps, -- cgit v1.2.3-58-ga151 From 8f5d35441ff26b31e3812556ce468c76f1eb216b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:24 +0200 Subject: usb-core: Move USB_MAXENDPOINTS definitions to usb.h So that it can be used in other places too. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/config.c | 1 - include/linux/usb.h | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c index 8d72f0c65937..14ba398d6def 100644 --- a/drivers/usb/core/config.c +++ b/drivers/usb/core/config.c @@ -10,7 +10,6 @@ #define USB_MAXALTSETTING 128 /* Hard limit */ -#define USB_MAXENDPOINTS 30 /* Hard limit */ #define USB_MAXCONFIG 8 /* Arbitrary limit */ diff --git a/include/linux/usb.h b/include/linux/usb.h index 7f6eb859873e..9b73dd782c09 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -202,6 +202,8 @@ static inline void usb_set_intfdata(struct usb_interface *intf, void *data) struct usb_interface *usb_get_intf(struct usb_interface *intf); void usb_put_intf(struct usb_interface *intf); +/* Hard limit */ +#define USB_MAXENDPOINTS 30 /* this maximum is arbitrary */ #define USB_MAXINTERFACES 32 #define USB_MAXIADS (USB_MAXINTERFACES/2) -- cgit v1.2.3-58-ga151 From 8d4f70b2fa52ca80f74faebc2471f74ee374cf61 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:25 +0200 Subject: usb-core: Track if an endpoint has streams This is a preparation patch for adding support for bulk streams to usbfs. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/hcd.c | 34 ++++++++++++++++++++++++++-------- include/linux/usb.h | 2 ++ 2 files changed, 28 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 9b445f43f825..9c4e2922b04d 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2049,7 +2049,7 @@ int usb_alloc_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); @@ -2058,13 +2058,24 @@ int usb_alloc_streams(struct usb_interface *interface, if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ - for (i = 0; i < num_eps; i++) + for (i = 0; i < num_eps; i++) { + /* Streams only apply to bulk endpoints. */ if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) return -EINVAL; + /* Re-alloc is not allowed */ + if (eps[i]->streams) + return -EINVAL; + } - return hcd->driver->alloc_streams(hcd, dev, eps, num_eps, + ret = hcd->driver->alloc_streams(hcd, dev, eps, num_eps, num_streams, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = ret; + + return ret; } EXPORT_SYMBOL_GPL(usb_alloc_streams); @@ -2086,19 +2097,26 @@ int usb_free_streams(struct usb_interface *interface, { struct usb_hcd *hcd; struct usb_device *dev; - int i; + int i, ret; dev = interface_to_usbdev(interface); hcd = bus_to_hcd(dev->bus); if (dev->speed != USB_SPEED_SUPER) return -EINVAL; - /* Streams only apply to bulk endpoints. */ + /* Double-free is not allowed */ for (i = 0; i < num_eps; i++) - if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc)) + if (!eps[i] || !eps[i]->streams) return -EINVAL; - return hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + ret = hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); + if (ret < 0) + return ret; + + for (i = 0; i < num_eps; i++) + eps[i]->streams = 0; + + return ret; } EXPORT_SYMBOL_GPL(usb_free_streams); diff --git a/include/linux/usb.h b/include/linux/usb.h index 9b73dd782c09..f1015cee5944 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -57,6 +57,7 @@ struct ep_device; * @extra: descriptors following this endpoint in the configuration * @extralen: how many bytes of "extra" are valid * @enabled: URBs may be submitted to this endpoint + * @streams: number of USB-3 streams allocated on the endpoint * * USB requests are always queued to a given endpoint, identified by a * descriptor within an active interface in a given USB configuration. @@ -71,6 +72,7 @@ struct usb_host_endpoint { unsigned char *extra; /* Extra descriptors */ int extralen; int enabled; + int streams; }; /* host-side wrapper for one interface setting's parsed descriptors */ -- cgit v1.2.3-58-ga151 From 6343e8bf09de16ab4dcae2c6ca1a0e8dbd4dd770 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:26 +0200 Subject: usb-core: Free bulk streams on interface release Documentation/usb/bulk-streams.txt says: All stream IDs will be deallocated when the driver releases the interface, to ensure that drivers that don't support streams will be able to use the endpoint This commit actually implements this. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/driver.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 85e0450a2bc7..08283d40616c 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -400,8 +400,9 @@ static int usb_unbind_interface(struct device *dev) { struct usb_driver *driver = to_usb_driver(dev->driver); struct usb_interface *intf = to_usb_interface(dev); + struct usb_host_endpoint *ep, **eps = NULL; struct usb_device *udev; - int error, r, lpm_disable_error; + int i, j, error, r, lpm_disable_error; intf->condition = USB_INTERFACE_UNBINDING; @@ -425,6 +426,26 @@ static int usb_unbind_interface(struct device *dev) driver->disconnect(intf); usb_cancel_queued_reset(intf); + /* Free streams */ + for (i = 0, j = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) { + ep = &intf->cur_altsetting->endpoint[i]; + if (ep->streams == 0) + continue; + if (j == 0) { + eps = kmalloc(USB_MAXENDPOINTS * sizeof(void *), + GFP_KERNEL); + if (!eps) { + dev_warn(dev, "oom, leaking streams\n"); + break; + } + } + eps[j++] = ep; + } + if (j) { + usb_free_streams(intf, eps, j, GFP_KERNEL); + kfree(eps); + } + /* Reset other interface state. * We cannot do a Set-Interface if the device is suspended or * if it is prepared for a system sleep (since installing a new -- cgit v1.2.3-58-ga151 From 5ec9c1771ce83a1e2b7ec96ed9f29a9f1b25e71e Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:27 +0200 Subject: usbfs: Kill urbs on interface before doing a set_interface The usb_set_interface documentation says: * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed * (perhaps forced by unlinking). For in kernel drivers we trust the drivers to get this right, but we cannot trust userspace to get this right, so enforce it by killing any urbs still pending on the interface. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index f3ba2e076ee3..2a95e4e574bb 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1143,6 +1143,9 @@ static int proc_setintf(struct dev_state *ps, void __user *arg) return -EFAULT; if ((ret = checkintf(ps, setintf.interface))) return ret; + + destroy_async_on_interface(ps, setintf.interface); + return usb_set_interface(ps->dev, setintf.interface, setintf.altsetting); } -- cgit v1.2.3-58-ga151 From b2d03eb56e66620a9b27f1a0c2795722087effc9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:28 +0200 Subject: usbfs: proc_do_submiturb use a local variable for number_of_packets This is a preparation patch for adding support for bulk streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 2a95e4e574bb..c88d8bfaca8d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1208,6 +1208,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, struct usb_ctrlrequest *dr = NULL; unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; + int number_of_packets = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1261,7 +1262,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, le16_to_cpup(&dr->wIndex)); if (ret) goto error; - uurb->number_of_packets = 0; uurb->buffer_length = le16_to_cpup(&dr->wLength); uurb->buffer += 8; if ((dr->bRequestType & USB_DIR_IN) && uurb->buffer_length) { @@ -1291,7 +1291,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, uurb->type = USBDEVFS_URB_TYPE_INTERRUPT; goto interrupt_urb; } - uurb->number_of_packets = 0; num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; @@ -1301,7 +1300,6 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (!usb_endpoint_xfer_int(&ep->desc)) return -EINVAL; interrupt_urb: - uurb->number_of_packets = 0; break; case USBDEVFS_URB_TYPE_ISO: @@ -1311,15 +1309,16 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -EINVAL; if (!usb_endpoint_xfer_isoc(&ep->desc)) return -EINVAL; + number_of_packets = uurb->number_of_packets; isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) * - uurb->number_of_packets; + number_of_packets; if (!(isopkt = kmalloc(isofrmlen, GFP_KERNEL))) return -ENOMEM; if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) { ret = -EFAULT; goto error; } - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { /* * arbitrary limit need for USB 3.0 * bMaxBurst (0~15 allowed, 1~16 packets) @@ -1350,7 +1349,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, ret = -EFAULT; goto error; } - as = alloc_async(uurb->number_of_packets); + as = alloc_async(number_of_packets); if (!as) { ret = -ENOMEM; goto error; @@ -1444,7 +1443,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->setup_packet = (unsigned char *)dr; dr = NULL; as->urb->start_frame = uurb->start_frame; - as->urb->number_of_packets = uurb->number_of_packets; + as->urb->number_of_packets = number_of_packets; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); @@ -1452,7 +1451,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, as->urb->interval = ep->desc.bInterval; as->urb->context = as; as->urb->complete = async_completed; - for (totlen = u = 0; u < uurb->number_of_packets; u++) { + for (totlen = u = 0; u < number_of_packets; u++) { as->urb->iso_frame_desc[u].offset = totlen; as->urb->iso_frame_desc[u].length = isopkt[u].length; totlen += isopkt[u].length; -- cgit v1.2.3-58-ga151 From 948cd8c18c466fdcbe707bb2a42a148796bfccdd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:29 +0200 Subject: usbfs: Add support for bulk stream ids This patch makes it possible to specify a bulk stream id when submitting an urb using the async usbfs API. It overloads the number_of_packets usbdevfs_urb field for this. This is not pretty, but given other constraints it is the best we can do. The reasoning leading to this goes as follows: 1) We want to support bulk streams in the usbfs API 2) We do not want to extend the usbdevfs_urb struct with a new member, as that would mean defining new ioctl numbers for all async API ioctls + adding compat versions for the old ones (times 2 for 32 bit support) 3) 1 + 2 means we need to re-use an existing field 4) number_of_packets is only used for isoc urbs, and streams are bulk only so it is the best (and only) candidate for re-using Note that: 1) This patch only uses number_of_packets as stream_id if the app has actually allocated streams on the ep, so that old apps which may have garbage in there (as it was unused until now in the bulk case), will not break 2) This patch does not add support for allocating / freeing bulk-streams, that is done in a follow up patch Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 4 ++++ include/uapi/linux/usbdevice_fs.h | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index c88d8bfaca8d..d7571a63181d 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1209,6 +1209,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, unsigned int u, totlen, isofrmlen; int i, ret, is_in, num_sgs = 0, ifnum = -1; int number_of_packets = 0; + unsigned int stream_id = 0; void *buf; if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP | @@ -1294,6 +1295,8 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE); if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize) num_sgs = 0; + if (ep->streams) + stream_id = uurb->stream_id; break; case USBDEVFS_URB_TYPE_INTERRUPT: @@ -1444,6 +1447,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, dr = NULL; as->urb->start_frame = uurb->start_frame; as->urb->number_of_packets = number_of_packets; + as->urb->stream_id = stream_id; if (uurb->type == USBDEVFS_URB_TYPE_ISO || ps->dev->speed == USB_SPEED_HIGH) as->urb->interval = 1 << min(15, ep->desc.bInterval - 1); diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index 0c65e4b12617..cbf122db56bc 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -102,7 +102,10 @@ struct usbdevfs_urb { int buffer_length; int actual_length; int start_frame; - int number_of_packets; + union { + int number_of_packets; /* Only used for isoc urbs */ + unsigned int stream_id; /* Only used with bulk streams */ + }; int error_count; unsigned int signr; /* signal to be sent on completion, or 0 if none should be sent. */ -- cgit v1.2.3-58-ga151 From 2fec32b06e374642802f7fb4f5350317cd14732b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:30 +0200 Subject: usbfs: Add ep_to_host_endpoint helper function Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index d7571a63181d..502974b4deb5 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -769,6 +769,15 @@ static int check_ctrlrecip(struct dev_state *ps, unsigned int requesttype, return ret; } +static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, + unsigned char ep) +{ + if (ep & USB_ENDPOINT_DIR_MASK) + return dev->ep_in[ep & USB_ENDPOINT_NUMBER_MASK]; + else + return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -1230,15 +1239,10 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, if (ret) return ret; } - if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0) { - is_in = 1; - ep = ps->dev->ep_in[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } else { - is_in = 0; - ep = ps->dev->ep_out[uurb->endpoint & USB_ENDPOINT_NUMBER_MASK]; - } + ep = ep_to_host_endpoint(ps->dev, uurb->endpoint); if (!ep) return -ENOENT; + is_in = (uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0; u = 0; switch(uurb->type) { -- cgit v1.2.3-58-ga151 From bcf7f6e39335af4f03da8c26a98185fd49754fcc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 9 Oct 2013 17:19:31 +0200 Subject: usbfs: Add support for allocating / freeing streams This allows userspace to use bulk-streams, just like in kernel drivers, see Documentation/usb/bulk-streams.txt for details on the in kernel API. This is exported pretty much one on one to userspace. To use streams an app must first make a USBDEVFS_ALLOC_STREAMS ioctl, on success this will return the number of streams available (which may be less then requested). If there are n streams the app can then submit usbdevfs_urb-s with their stream_id member set to 1-n to use a specific stream. IE if USBDEVFS_ALLOC_STREAMS returns 4 then stream_id 1-4 can be used. When the app is done using streams it should call USBDEVFS_FREE_STREAMS Note applications are advised to use libusb rather then using the usbdevfs api directly. The latest version of libusb has support for streams. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/core/devio.c | 118 ++++++++++++++++++++++++++++++++++++++ include/uapi/linux/usbdevice_fs.h | 7 +++ 2 files changed, 125 insertions(+) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 502974b4deb5..12401ee4ba0e 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -778,6 +778,79 @@ static struct usb_host_endpoint *ep_to_host_endpoint(struct usb_device *dev, return dev->ep_out[ep & USB_ENDPOINT_NUMBER_MASK]; } +static int parse_usbdevfs_streams(struct dev_state *ps, + struct usbdevfs_streams __user *streams, + unsigned int *num_streams_ret, + unsigned int *num_eps_ret, + struct usb_host_endpoint ***eps_ret, + struct usb_interface **intf_ret) +{ + unsigned int i, num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf = NULL; + unsigned char ep; + int ifnum, ret; + + if (get_user(num_streams, &streams->num_streams) || + get_user(num_eps, &streams->num_eps)) + return -EFAULT; + + if (num_eps < 1 || num_eps > USB_MAXENDPOINTS) + return -EINVAL; + + /* The XHCI controller allows max 2 ^ 16 streams */ + if (num_streams_ret && (num_streams < 2 || num_streams > 65536)) + return -EINVAL; + + eps = kmalloc(num_eps * sizeof(*eps), GFP_KERNEL); + if (!eps) + return -ENOMEM; + + for (i = 0; i < num_eps; i++) { + if (get_user(ep, &streams->eps[i])) { + ret = -EFAULT; + goto error; + } + eps[i] = ep_to_host_endpoint(ps->dev, ep); + if (!eps[i]) { + ret = -EINVAL; + goto error; + } + + /* usb_alloc/free_streams operate on an usb_interface */ + ifnum = findintfep(ps->dev, ep); + if (ifnum < 0) { + ret = ifnum; + goto error; + } + + if (i == 0) { + ret = checkintf(ps, ifnum); + if (ret < 0) + goto error; + intf = usb_ifnum_to_if(ps->dev, ifnum); + } else { + /* Verify all eps belong to the same interface */ + if (ifnum != intf->altsetting->desc.bInterfaceNumber) { + ret = -EINVAL; + goto error; + } + } + } + + if (num_streams_ret) + *num_streams_ret = num_streams; + *num_eps_ret = num_eps; + *eps_ret = eps; + *intf_ret = intf; + + return 0; + +error: + kfree(eps); + return ret; +} + static int match_devt(struct device *dev, void *data) { return dev->devt == (dev_t) (unsigned long) data; @@ -2009,6 +2082,45 @@ static int proc_disconnect_claim(struct dev_state *ps, void __user *arg) return claimintf(ps, dc.interface); } +static int proc_alloc_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_streams, num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, &num_streams, &num_eps, + &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_alloc_streams(intf, eps, num_eps, num_streams, GFP_KERNEL); + kfree(eps); + return r; +} + +static int proc_free_streams(struct dev_state *ps, void __user *arg) +{ + unsigned num_eps; + struct usb_host_endpoint **eps; + struct usb_interface *intf; + int r; + + r = parse_usbdevfs_streams(ps, arg, NULL, &num_eps, &eps, &intf); + if (r) + return r; + + destroy_async_on_interface(ps, + intf->altsetting[0].desc.bInterfaceNumber); + + r = usb_free_streams(intf, eps, num_eps, GFP_KERNEL); + kfree(eps); + return r; +} + /* * NOTE: All requests here that have interface numbers as parameters * are assuming that somehow the configuration has been prevented from @@ -2185,6 +2297,12 @@ static long usbdev_do_ioctl(struct file *file, unsigned int cmd, case USBDEVFS_DISCONNECT_CLAIM: ret = proc_disconnect_claim(ps, p); break; + case USBDEVFS_ALLOC_STREAMS: + ret = proc_alloc_streams(ps, p); + break; + case USBDEVFS_FREE_STREAMS: + ret = proc_free_streams(ps, p); + break; } usb_unlock_device(dev); if (ret >= 0) diff --git a/include/uapi/linux/usbdevice_fs.h b/include/uapi/linux/usbdevice_fs.h index cbf122db56bc..abe5f4bd4d82 100644 --- a/include/uapi/linux/usbdevice_fs.h +++ b/include/uapi/linux/usbdevice_fs.h @@ -147,6 +147,11 @@ struct usbdevfs_disconnect_claim { char driver[USBDEVFS_MAXDRIVERNAME + 1]; }; +struct usbdevfs_streams { + unsigned int num_streams; /* Not used by USBDEVFS_FREE_STREAMS */ + unsigned int num_eps; + unsigned char eps[0]; +}; #define USBDEVFS_CONTROL _IOWR('U', 0, struct usbdevfs_ctrltransfer) #define USBDEVFS_CONTROL32 _IOWR('U', 0, struct usbdevfs_ctrltransfer32) @@ -179,5 +184,7 @@ struct usbdevfs_disconnect_claim { #define USBDEVFS_RELEASE_PORT _IOR('U', 25, unsigned int) #define USBDEVFS_GET_CAPABILITIES _IOR('U', 26, __u32) #define USBDEVFS_DISCONNECT_CLAIM _IOR('U', 27, struct usbdevfs_disconnect_claim) +#define USBDEVFS_ALLOC_STREAMS _IOR('U', 28, struct usbdevfs_streams) +#define USBDEVFS_FREE_STREAMS _IOR('U', 29, struct usbdevfs_streams) #endif /* _UAPI_LINUX_USBDEVICE_FS_H */ -- cgit v1.2.3-58-ga151 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 ++++- drivers/usb/core/message.c | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core') 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: diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index f829a1aad1c3..964695741031 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1293,8 +1293,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) struct usb_interface *iface; struct usb_host_interface *alt; struct usb_hcd *hcd = bus_to_hcd(dev->bus); - int ret; - int manual = 0; + int i, ret, manual = 0; unsigned int epaddr; unsigned int pipe; @@ -1329,6 +1328,10 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) mutex_unlock(hcd->bandwidth_mutex); return -ENOMEM; } + /* Changing alt-setting also frees any allocated streams */ + for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++) + iface->cur_altsetting->endpoint[i].streams = 0; + ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt); if (ret < 0) { dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n", -- cgit v1.2.3-58-ga151 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') 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.3-58-ga151 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/driver.c | 94 ++++++++++++++++++++++++++++------------------- drivers/usb/core/hub.c | 5 ++- drivers/usb/core/usb.h | 2 +- 3 files changed, 60 insertions(+), 41 deletions(-) (limited to 'drivers/usb/core') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 08283d40616c..888881e5f292 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1011,8 +1011,7 @@ EXPORT_SYMBOL_GPL(usb_deregister); * it doesn't support pre_reset/post_reset/reset_resume or * because it doesn't support suspend/resume. * - * The caller must hold @intf's device's lock, but not its pm_mutex - * and not @intf->dev.sem. + * The caller must hold @intf's device's lock, but not @intf's lock. */ void usb_forced_unbind_intf(struct usb_interface *intf) { @@ -1025,16 +1024,37 @@ void usb_forced_unbind_intf(struct usb_interface *intf) intf->needs_binding = 1; } +/* + * Unbind drivers for @udev's marked interfaces. These interfaces have + * the needs_binding flag set, for example by usb_resume_interface(). + * + * The caller must hold @udev's device lock. + */ +static void unbind_marked_interfaces(struct usb_device *udev) +{ + struct usb_host_config *config; + int i; + struct usb_interface *intf; + + config = udev->actconfig; + if (config) { + for (i = 0; i < config->desc.bNumInterfaces; ++i) { + intf = config->interface[i]; + if (intf->dev.driver && intf->needs_binding) + usb_forced_unbind_intf(intf); + } + } +} + /* Delayed forced unbinding of a USB interface driver and scan * for rebinding. * - * The caller must hold @intf's device's lock, but not its pm_mutex - * and not @intf->dev.sem. + * The caller must hold @intf's device's lock, but not @intf's lock. * * Note: Rebinds will be skipped if a system sleep transition is in * progress and the PM "complete" callback hasn't occurred yet. */ -void usb_rebind_intf(struct usb_interface *intf) +static void usb_rebind_intf(struct usb_interface *intf) { int rc; @@ -1051,68 +1071,66 @@ void usb_rebind_intf(struct usb_interface *intf) } } -#ifdef CONFIG_PM - -/* Unbind drivers for @udev's interfaces that don't support suspend/resume - * There is no check for reset_resume here because it can be determined - * only during resume whether reset_resume is needed. +/* + * Rebind drivers to @udev's marked interfaces. These interfaces have + * the needs_binding flag set. * * The caller must hold @udev's device lock. */ -static void unbind_no_pm_drivers_interfaces(struct usb_device *udev) +static void rebind_marked_interfaces(struct usb_device *udev) { struct usb_host_config *config; int i; struct usb_interface *intf; - struct usb_driver *drv; config = udev->actconfig; if (config) { for (i = 0; i < config->desc.bNumInterfaces; ++i) { intf = config->interface[i]; - - if (intf->dev.driver) { - drv = to_usb_driver(intf->dev.driver); - if (!drv->suspend || !drv->resume) - usb_forced_unbind_intf(intf); - } + if (intf->needs_binding) + usb_rebind_intf(intf); } } } -/* Unbind drivers for @udev's interfaces that failed to support reset-resume. - * These interfaces have the needs_binding flag set by usb_resume_interface(). +/* + * Unbind all of @udev's marked interfaces and then rebind all of them. + * This ordering is necessary because some drivers claim several interfaces + * when they are first probed. * * The caller must hold @udev's device lock. */ -static void unbind_no_reset_resume_drivers_interfaces(struct usb_device *udev) +void usb_unbind_and_rebind_marked_interfaces(struct usb_device *udev) { - struct usb_host_config *config; - int i; - struct usb_interface *intf; - - config = udev->actconfig; - if (config) { - for (i = 0; i < config->desc.bNumInterfaces; ++i) { - intf = config->interface[i]; - if (intf->dev.driver && intf->needs_binding) - usb_forced_unbind_intf(intf); - } - } + unbind_marked_interfaces(udev); + rebind_marked_interfaces(udev); } -static void do_rebind_interfaces(struct usb_device *udev) +#ifdef CONFIG_PM + +/* Unbind drivers for @udev's interfaces that don't support suspend/resume + * There is no check for reset_resume here because it can be determined + * only during resume whether reset_resume is needed. + * + * The caller must hold @udev's device lock. + */ +static void unbind_no_pm_drivers_interfaces(struct usb_device *udev) { struct usb_host_config *config; int i; struct usb_interface *intf; + struct usb_driver *drv; config = udev->actconfig; if (config) { for (i = 0; i < config->desc.bNumInterfaces; ++i) { intf = config->interface[i]; - if (intf->needs_binding) - usb_rebind_intf(intf); + + if (intf->dev.driver) { + drv = to_usb_driver(intf->dev.driver); + if (!drv->suspend || !drv->resume) + usb_forced_unbind_intf(intf); + } } } } @@ -1441,7 +1459,7 @@ int usb_resume_complete(struct device *dev) * whose needs_binding flag is set */ if (udev->state != USB_STATE_NOTATTACHED) - do_rebind_interfaces(udev); + rebind_marked_interfaces(udev); return 0; } @@ -1463,7 +1481,7 @@ int usb_resume(struct device *dev, pm_message_t msg) pm_runtime_disable(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); - unbind_no_reset_resume_drivers_interfaces(udev); + unbind_marked_interfaces(udev); } /* Avoid PM error messages for devices disconnected while suspended 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); diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 823857767a16..0923add72b59 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -55,7 +55,7 @@ extern int usb_match_one_id_intf(struct usb_device *dev, extern int usb_match_device(struct usb_device *dev, const struct usb_device_id *id); extern void usb_forced_unbind_intf(struct usb_interface *intf); -extern void usb_rebind_intf(struct usb_interface *intf); +extern void usb_unbind_and_rebind_marked_interfaces(struct usb_device *udev); extern int usb_hub_claim_port(struct usb_device *hdev, unsigned port, struct dev_state *owner); -- cgit v1.2.3-58-ga151 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') 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.3-58-ga151