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. Add a mutex per root hub to protect bandwidth operations: adding/reseting/changing configurations, and changing alternate interface settings. We want to ensure that the xHCI host controller and the USB device are set up for the same configurations and alternate settings. There are two (possibly three) steps to do this: 1. The host controller needs to check that bandwidth is available for a different setting, by issuing and waiting for a configure endpoint command. 2. Once that returns successfully, a control message is sent to the device. 3. If that fails, the host controller must be notified through another configure endpoint command. The mutex is used to make these three operations seem atomic, to prevent another driver from using more bandwidth for a different device while we're in the middle of these operations. Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> --- drivers/usb/core/hcd.c | 25 ++++++++++++++-- drivers/usb/core/hcd.h | 3 +- drivers/usb/core/hub.c | 11 +++++++ drivers/usb/core/message.c | 68 ++++++++++++++++++++++++++++++++++++++++--- include/linux/usb.h | 10 ++++++ 5 files changed, 108 insertions(+), 9 deletions(-) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 34de475..ff53854 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -38,6 +38,7 @@ #include <asm/unaligned.h> #include <linux/platform_device.h> #include <linux/workqueue.h> +#include <linux/mutex.h> #include <linux/usb.h> @@ -868,6 +869,7 @@ static void usb_bus_init (struct usb_bus *bus) bus->bandwidth_allocated = 0; bus->bandwidth_int_reqs = 0; bus->bandwidth_isoc_reqs = 0; + mutex_init(&bus->bandwidth_lock); INIT_LIST_HEAD (&bus->bus_list); } @@ -1592,12 +1594,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; @@ -1611,7 +1614,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) @@ -1668,6 +1671,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/hub.c b/drivers/usb/core/hub.c index 5ce8391..4ed8817 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3534,6 +3534,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev) { struct usb_device *parent_hdev = udev->parent; struct usb_hub *parent_hub; + struct usb_bus *bus = udev->bus; struct usb_device_descriptor descriptor = udev->descriptor; int i, ret = 0; int port1 = udev->portnum; @@ -3577,6 +3578,14 @@ static int usb_reset_and_verify_device(struct usb_device *udev) /* Restore the device's previous configuration */ if (!udev->actconfig) goto done; + + mutex_lock(&bus->bandwidth_lock); + ret = usb_hcd_check_bandwidth(udev, udev->actconfig, NULL, NULL); + if (ret < 0) { + dev_info(&udev->dev, "Not enough bandwidth for old config.\n"); + mutex_unlock(&bus->bandwidth_lock); + goto re_enumerate; + } ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0), USB_REQ_SET_CONFIGURATION, 0, udev->actconfig->desc.bConfigurationValue, 0, @@ -3585,8 +3594,10 @@ static int usb_reset_and_verify_device(struct usb_device *udev) dev_err(&udev->dev, "can't restore configuration #%d (error=%d)\n", udev->actconfig->desc.bConfigurationValue, ret); + mutex_unlock(&bus->bandwidth_lock); goto re_enumerate; } + mutex_unlock(&bus->bandwidth_lock); usb_set_device_state(udev, USB_STATE_CONFIGURED); /* Put interfaces back into the same altsettings as before. diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index da718e8..3a3ca09 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1303,6 +1303,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) { struct usb_interface *iface; struct usb_host_interface *alt; + struct usb_bus *bus = dev->bus; int ret; int manual = 0; unsigned int epaddr; @@ -1325,6 +1326,18 @@ 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. + */ + mutex_lock(&bus->bandwidth_lock); + ret = usb_hcd_check_bandwidth(dev, NULL, iface->cur_altsetting, alt); + if (ret < 0) { + dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n", + alternate); + mutex_unlock(&bus->bandwidth_lock); + return ret; + } + if (dev->quirks & USB_QUIRK_NO_SET_INTF) ret = -EPIPE; else @@ -1340,8 +1353,13 @@ 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); + mutex_unlock(&bus->bandwidth_lock); return ret; + } + mutex_unlock(&bus->bandwidth_lock); /* FIXME drivers shouldn't need to replicate/bugfix the logic here * when they implement async or easily-killable versions of this or @@ -1423,6 +1441,7 @@ int usb_reset_configuration(struct usb_device *dev) { int i, retval; struct usb_host_config *config; + struct usb_bus *bus = dev->bus; if (dev->state == USB_STATE_SUSPENDED) return -EHOSTUNREACH; @@ -1438,12 +1457,46 @@ int usb_reset_configuration(struct usb_device *dev) } config = dev->actconfig; + retval = 0; + mutex_lock(&bus->bandwidth_lock); + /* 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); + } + mutex_unlock(&bus->bandwidth_lock); + 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; + mutex_unlock(&bus->bandwidth_lock); /* re-init hc/hcd interface/endpoint state */ for (i = 0; i < config->desc.bNumInterfaces; i++) { @@ -1652,6 +1705,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration) int i, ret; struct usb_host_config *cp = NULL; struct usb_interface **new_interfaces = NULL; + struct usb_bus *bus = dev->bus; int n, nintf; if (dev->authorized == 0 || configuration == -1) @@ -1721,12 +1775,14 @@ free_interfaces: * host controller will not allow submissions to dropped endpoints. If * this call fails, the device state is unchanged. */ + mutex_lock(&bus->bandwidth_lock); 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); + mutex_unlock(&bus->bandwidth_lock); goto free_interfaces; } @@ -1752,10 +1808,12 @@ 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); + mutex_unlock(&bus->bandwidth_lock); goto free_interfaces; } + mutex_unlock(&bus->bandwidth_lock); usb_set_device_state(dev, USB_STATE_CONFIGURED); /* Initialize the new interface structures and the diff --git a/include/linux/usb.h b/include/linux/usb.h index a34fa89..06f10ea 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -348,6 +348,16 @@ struct usb_bus { */ int bandwidth_int_reqs; /* number of Interrupt requests */ int bandwidth_isoc_reqs; /* number of Isoc. requests */ + struct mutex bandwidth_lock; /* Mutex should be taken before adding + * any new bandwidth constraints: + * before adding a configuration for a + * new device, selecting an alternate + * interface setting, or a different + * configuration. Mutex should be + * dropped after a successful control + * message to the device, or resetting + * the bandwidth after a failed attempt. + */ #ifdef CONFIG_USB_DEVICEFS struct dentry *usbfs_dentry; /* usbfs dentry entry for the bus */ -- 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