Make the USB core check the bandwidth when switching from one
interface alternate setting to another. Also check the bandwidth
when resetting a configuration (so that alt setting 0 is used). If
this check fails, the device's state is unchanged. If the device
refuses the new alt setting, re-instate the old alt setting in the
host controller hardware.
I'm not entirely sure how to handle this race condition:
Device A has one interface with two alt settings:
alt setting 0 has an interrupt endpoint with max packet size 32
alt setting 1 has an interrupt endpoint with max packet size 64
Device A currently uses alt setting 1, but the driver wants to switch
back to alt setting 0.
1. USB core sets up alt setting 0 for device A, which succeeds on
the xHCI hardware side.
2. A new USB device gets plugged in, and consumes the rest of the bus
bandwidth. Or another driver asks for a different alt setting that
consumes more bandwidth.
3. USB core sends the USB_REQ_SET_INTERFACE control transfer, which
fails due to electrical noise.
If the USB core tries to re-instate alt setting 1, the xHCI hardware
will reject it because it takes up too much bandwidth. So it seems
like there needs to be some sort of bus lock whenever the USB core
starts to install a new configuration or alt setting.
Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
---
drivers/usb/core/hcd.c | 23 +++++++++++++++++--
drivers/usb/core/hcd.h | 3 +-
drivers/usb/core/message.c | 51 +++++++++++++++++++++++++++++++++++++++----
3 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 95ccfa0..808a5ff 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1575,12 +1575,13 @@ rescan:
/* Check whether a new configuration or alt setting for an interface
* will exceed the bandwidth for the bus (or the host controller resources).
* Only pass in a non-NULL config or interface, not both!
- * Passing NULL for both new_config and new_intf means the device will be
+ * Passing NULL for both new_config and old_alt means the device will be
* de-configured by issuing a set configuration 0 command.
*/
int usb_hcd_check_bandwidth(struct usb_device *udev,
struct usb_host_config *new_config,
- struct usb_interface *new_intf)
+ struct usb_host_interface *old_alt,
+ struct usb_host_interface *new_alt)
{
int num_intfs, i, j;
struct usb_interface_cache *intf_cache;
@@ -1594,7 +1595,7 @@ int usb_hcd_check_bandwidth(struct usb_device *udev,
return 0;
/* Configuration is being removed - set configuration 0 */
- if (!new_config && !new_intf) {
+ if (!new_config && !old_alt) {
for (i = 1; i < 16; ++i) {
ep = udev->ep_out[i];
if (ep)
@@ -1651,6 +1652,22 @@ int usb_hcd_check_bandwidth(struct usb_device *udev,
}
}
}
+ if (old_alt && new_alt) {
+ /* Drop all the endpoints in the old alt setting */
+ for (i = 0; i < old_alt->desc.bNumEndpoints; i++) {
+ ret = hcd->driver->drop_endpoint(hcd, udev,
+ &old_alt->endpoint[i]);
+ if (ret < 0)
+ goto reset;
+ }
+ /* Add all the endpoints in the new alt setting */
+ for (i = 0; i < new_alt->desc.bNumEndpoints; i++) {
+ ret = hcd->driver->add_endpoint(hcd, udev,
+ &new_alt->endpoint[i]);
+ if (ret < 0)
+ goto reset;
+ }
+ }
ret = hcd->driver->check_bandwidth(hcd, udev);
reset:
if (ret < 0)
diff --git a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
index 79782a1..27b3ec7 100644
--- a/drivers/usb/core/hcd.h
+++ b/drivers/usb/core/hcd.h
@@ -292,7 +292,8 @@ extern void usb_hcd_reset_endpoint(struct usb_device *udev,
extern void usb_hcd_synchronize_unlinks(struct usb_device *udev);
extern int usb_hcd_check_bandwidth(struct usb_device *udev,
struct usb_host_config *new_config,
- struct usb_interface *new_intf);
+ struct usb_host_interface *old_alt,
+ struct usb_host_interface *new_alt);
extern int usb_hcd_get_frame_number(struct usb_device *udev);
extern struct usb_hcd *usb_create_hcd(const struct hc_driver *driver,
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 9720e69..90ff67a 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1337,6 +1337,13 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
return -EINVAL;
}
+ /* Make sure we have enough bandwidth for this alternate interface.
+ * Remove the current alt setting and add the new alt setting.
+ */
+ ret = usb_hcd_check_bandwidth(dev, NULL, iface->cur_altsetting, alt);
+ if (ret < 0)
+ return ret;
+
if (dev->quirks & USB_QUIRK_NO_SET_INTF)
ret = -EPIPE;
else
@@ -1352,8 +1359,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
"manual set_interface for iface %d, alt %d\n",
interface, alternate);
manual = 1;
- } else if (ret < 0)
+ } else if (ret < 0) {
+ /* Re-instate the old alt setting */
+ usb_hcd_check_bandwidth(dev, NULL, alt, iface->cur_altsetting);
return ret;
+ }
/* FIXME drivers shouldn't need to replicate/bugfix the logic here
* when they implement async or easily-killable versions of this or
@@ -1450,12 +1460,43 @@ int usb_reset_configuration(struct usb_device *dev)
}
config = dev->actconfig;
+ retval = 0;
+ /* Make sure we have enough bandwidth for each alternate setting 0 */
+ for (i = 0; i < config->desc.bNumInterfaces; i++) {
+ struct usb_interface *intf = config->interface[i];
+ struct usb_host_interface *alt;
+
+ alt = usb_altnum_to_altsetting(intf, 0);
+ if (!alt)
+ alt = &intf->altsetting[0];
+ if (alt != intf->cur_altsetting)
+ retval = usb_hcd_check_bandwidth(dev, NULL,
+ intf->cur_altsetting, alt);
+ if (retval < 0)
+ break;
+ }
+ /* If not, reinstate the old alternate settings */
+ if (retval < 0) {
+reset_old_alts:
+ for (; i >= 0; i--) {
+ struct usb_interface *intf = config->interface[i];
+ struct usb_host_interface *alt;
+
+ alt = usb_altnum_to_altsetting(intf, 0);
+ if (!alt)
+ alt = &intf->altsetting[0];
+ if (alt != intf->cur_altsetting)
+ usb_hcd_check_bandwidth(dev, NULL,
+ alt, intf->cur_altsetting);
+ }
+ return retval;
+ }
retval = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
USB_REQ_SET_CONFIGURATION, 0,
config->desc.bConfigurationValue, 0,
NULL, 0, USB_CTRL_SET_TIMEOUT);
if (retval < 0)
- return retval;
+ goto reset_old_alts;
/* re-init hc/hcd interface/endpoint state */
for (i = 0; i < config->desc.bNumInterfaces; i++) {
@@ -1734,9 +1775,9 @@ free_interfaces:
* this call fails, the device state is unchanged.
*/
if (cp)
- ret = usb_hcd_check_bandwidth(dev, cp, NULL);
+ ret = usb_hcd_check_bandwidth(dev, cp, NULL, NULL);
else
- ret = usb_hcd_check_bandwidth(dev, NULL, NULL);
+ ret = usb_hcd_check_bandwidth(dev, NULL, NULL, NULL);
if (ret < 0) {
usb_autosuspend_device(dev);
goto free_interfaces;
@@ -1764,7 +1805,7 @@ free_interfaces:
dev->actconfig = cp;
if (!cp) {
usb_set_device_state(dev, USB_STATE_ADDRESS);
- usb_hcd_check_bandwidth(dev, NULL, NULL);
+ usb_hcd_check_bandwidth(dev, NULL, NULL, NULL);
usb_autosuspend_device(dev);
goto free_interfaces;
}
--
1.6.0.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
