On Fri, Oct 26, 2012 at 03:01:32PM -0700, Sarah Sharp wrote:
> The USB core isn't dropping the endpoints before it calls
> xhci_check_bandwidth. I remember running into this bug a while back,
> and I even started on a fix, but then couldn't reproduce the problem.
> I found the branch with the old fix on it, but it still needs a bit of
> work. I'll send you a patch on Monday.
Matthias, can you try the attached patch? You should be able to echo 1
to the configuration file after this is applied.
Thanks,
Sarah Sharp
>From fda7b0f5cd061b5c6e680ac71fdebc4158867602 Mon Sep 17 00:00:00 2001
From: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Date: Tue, 7 Aug 2012 08:38:09 -0700
Subject: [PATCH] USB: Fix dropping endpoints when SetConfig fails.
When the Set Configuration control transfer fails, the USB core will set
the udev->config to NULL *before* calling usb_check_bandwidth. This
means that endpoints that were part of the previous installed
configuration will never be dropped from the xHCI host (until the device
is disconnected). This leads to extra host bandwidth resources being
held. Fix this by explicitly dropping the endpoints when the Set
Configuration fails.
This patch should be backported. FIXME: version.
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
drivers/usb/core/hcd.c | 80 +++++++++++++++++++++++++++++++++-----------
drivers/usb/core/message.c | 3 ++
include/linux/usb/hcd.h | 2 +
3 files changed, 65 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 1e741bc..8169e75 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1693,6 +1693,62 @@ rescan:
}
}
+static int usb_hcd_drop_or_add_alt0(struct usb_device *udev,
+ struct usb_host_config *config,
+ bool drop)
+{
+ int num_intfs, i, ret;
+ struct usb_hcd *hcd;
+
+ hcd = bus_to_hcd(udev->bus);
+ num_intfs = config->desc.bNumInterfaces;
+ for (i = 0; i < num_intfs; ++i) {
+ struct usb_host_interface *first_alt, *alt;
+ int iface_num, j;
+
+ first_alt = &config->intf_cache[i]->altsetting[0];
+ iface_num = first_alt->desc.bInterfaceNumber;
+ /* Set up endpoints for alternate interface setting 0 */
+ alt = usb_find_alt_setting(config, iface_num, 0);
+ if (!alt)
+ /* No alt setting 0? Pick the first setting. */
+ alt = first_alt;
+
+ for (j = 0; j < alt->desc.bNumEndpoints; j++) {
+ if (drop)
+ ret = hcd->driver->drop_endpoint(hcd, udev,
+ &alt->endpoint[j]);
+ else
+ ret = hcd->driver->add_endpoint(hcd, udev,
+ &alt->endpoint[j]);
+ if (ret < 0)
+ return ret;
+ }
+ }
+ return 0;
+}
+
+/**
+ * usb_hcd_revert_bandwidth - revert a configuration that failed to be installed.
+ */
+int usb_hcd_revert_bandwidth(struct usb_device *udev,
+ struct usb_host_config *failed_config)
+{
+ int ret;
+ struct usb_hcd *hcd;
+
+ hcd = bus_to_hcd(udev->bus);
+ if (!hcd->driver->check_bandwidth)
+ return 0;
+
+ ret = usb_hcd_drop_or_add_alt0(udev, failed_config, true);
+ if (!ret)
+ ret = hcd->driver->check_bandwidth(hcd, udev);
+ if (ret < 0)
+ hcd->driver->reset_bandwidth(hcd, udev);
+ return ret;
+}
+
/**
* usb_hcd_alloc_bandwidth - check whether a new bandwidth setting exceeds
* the bus bandwidth
@@ -1719,8 +1775,7 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
struct usb_host_interface *cur_alt,
struct usb_host_interface *new_alt)
{
- int num_intfs, i, j;
- struct usb_host_interface *alt = NULL;
+ int num_intfs, i;
int ret = 0;
struct usb_hcd *hcd;
struct usb_host_endpoint *ep;
@@ -1766,24 +1821,9 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
goto reset;
}
}
- for (i = 0; i < num_intfs; ++i) {
- struct usb_host_interface *first_alt;
- int iface_num;
-
- first_alt = &new_config->intf_cache[i]->altsetting[0];
- iface_num = first_alt->desc.bInterfaceNumber;
- /* Set up endpoints for alternate interface setting 0 */
- alt = usb_find_alt_setting(new_config, iface_num, 0);
- if (!alt)
- /* No alt setting 0? Pick the first setting. */
- alt = first_alt;
-
- for (j = 0; j < alt->desc.bNumEndpoints; j++) {
- ret = hcd->driver->add_endpoint(hcd, udev, &alt->endpoint[j]);
- if (ret < 0)
- goto reset;
- }
- }
+ ret = usb_hcd_drop_or_add_alt0(udev, new_config, false);
+ if (ret < 0)
+ goto reset;
}
if (cur_alt && new_alt) {
struct usb_interface *iface = usb_ifnum_to_if(udev,
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 1ed5afd..58b4bf1 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1812,7 +1812,10 @@ free_interfaces:
if (ret < 0) {
/* All the old state is gone, so what else can we do?
* The device is probably useless now anyway.
+ * We need to deallocate the bandwidth from the failed
+ * configuration, so we don't leak host resources.
*/
+ usb_hcd_revert_bandwidth(dev, cp);
cp = NULL;
}
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 608050b..0c22499 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -383,6 +383,8 @@ extern int usb_hcd_alloc_bandwidth(struct usb_device *udev,
struct usb_host_config *new_config,
struct usb_host_interface *old_alt,
struct usb_host_interface *new_alt);
+extern int usb_hcd_revert_bandwidth(struct usb_device *udev,
+ struct usb_host_config *failed_config);
extern int usb_hcd_get_frame_number(struct usb_device *udev);
extern struct usb_hcd *usb_create_hcd(const struct hc_driver *driver,
--
1.7.9
