From da2bbdcc3838ce75c30bda8c3f9a6e55ece47ee1 Mon Sep 17 00:00:00 2001 From: Harvey Harrison Date: Wed, 29 Oct 2008 14:25:51 -0700 Subject: USB: avoid needless address-taking of function parameters There's no need to take the address of the function params or local variables when the direct value byteswapping routines are available. Signed-off-by: Harvey Harrison Signed-off-by: Andrew Morton Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/message.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 6d1048faf08..cc47d36798b 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -139,9 +139,9 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request, dr->bRequestType = requesttype; dr->bRequest = request; - dr->wValue = cpu_to_le16p(&value); - dr->wIndex = cpu_to_le16p(&index); - dr->wLength = cpu_to_le16p(&size); + dr->wValue = cpu_to_le16(value); + dr->wIndex = cpu_to_le16(index); + dr->wLength = cpu_to_le16(size); /* dbg("usb_control_msg"); */ -- cgit v1.2.3-70-g09d2 From dc023dceec861c60bc1d1a17a2c6496ddac26ee7 Mon Sep 17 00:00:00 2001 From: Inaky Perez-Gonzalez Date: Thu, 13 Nov 2008 10:31:35 -0800 Subject: USB: Introduce usb_queue_reset() to do resets from atomic contexts This patch introduces a new call to be able to do a USB reset from an atomic contect. This is quite helpful in USB callbacks to handle errors (when the only thing that can be done is to do a device reset). It is done queuing a work struct that will do the actual reset. The struct is "attached" to an interface so pending requests from an interface are removed when said interface is unbound from the driver. The call flow then becomes: usb_queue_reset_device() __usb_queue_reset_device() [workqueue] usb_reset_device() usb_probe_interface() usb_cancel_queue_reset() [error path] usb_unbind_interface() usb_cancel_queue_reset() usb_driver_release_interface() usb_cancel_queue_reset() Note usb_cancel_queue_reset() needs smarts to try not to unqueue when it is actually being executed. This happens when we run the reset from the workqueue: usb_reset_device() is called and on interface unbind time, usb_cancel_queue_reset() would be called. That would deadlock on cancel_work_sync(). To avoid that, we set (before running usb_reset_device()) usb_intf->reset_running and clear it inmediately after returning. Patch is against 2.6.28-rc2 and depends on http://marc.info/?l=linux-usb&m=122581634925308&w=2 (as submitted by Alan Stern). Signed-off-by: Inaky Perez-Gonzalez Cc: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 23 +++++++++++++++++++++-- drivers/usb/core/hub.c | 43 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/core/message.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/usb.h | 8 ++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 23b3c7e79d4..7e26fb3c275 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -184,6 +184,20 @@ static int usb_unbind_device(struct device *dev) return 0; } +/* + * Cancel any pending scheduled resets + * + * [see usb_queue_reset_device()] + * + * Called after unconfiguring / when releasing interfaces. See + * comments in __usb_queue_reset_device() regarding + * udev->reset_running. + */ +static void usb_cancel_queued_reset(struct usb_interface *iface) +{ + if (iface->reset_running == 0) + cancel_work_sync(&iface->reset_ws); +} /* called from driver core with dev locked */ static int usb_probe_interface(struct device *dev) @@ -242,6 +256,7 @@ static int usb_probe_interface(struct device *dev) mark_quiesced(intf); intf->needs_remote_wakeup = 0; intf->condition = USB_INTERFACE_UNBOUND; + usb_cancel_queued_reset(intf); } else intf->condition = USB_INTERFACE_BOUND; @@ -272,6 +287,7 @@ static int usb_unbind_interface(struct device *dev) usb_disable_interface(udev, intf); driver->disconnect(intf); + usb_cancel_queued_reset(intf); /* Reset other interface state. * We cannot do a Set-Interface if the device is suspended or @@ -380,8 +396,10 @@ void usb_driver_release_interface(struct usb_driver *driver, if (device_is_registered(dev)) { iface->condition = USB_INTERFACE_UNBINDING; device_release_driver(dev); + } else { + iface->condition = USB_INTERFACE_UNBOUND; + usb_cancel_queued_reset(iface); } - dev->driver = NULL; usb_set_intfdata(iface, NULL); @@ -942,7 +960,8 @@ static int usb_suspend_interface(struct usb_device *udev, if (udev->state == USB_STATE_NOTATTACHED || !is_active(intf)) goto done; - if (intf->condition == USB_INTERFACE_UNBOUND) /* This can't happen */ + /* This can happen; see usb_driver_release_interface() */ + if (intf->condition == USB_INTERFACE_UNBOUND) goto done; driver = to_usb_driver(intf->dev.driver); diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 95fb3104ba4..e65881899c8 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3518,3 +3518,46 @@ int usb_reset_device(struct usb_device *udev) return ret; } EXPORT_SYMBOL_GPL(usb_reset_device); + + +/** + * usb_queue_reset_device - Reset a USB device from an atomic context + * @iface: USB interface belonging to the device to reset + * + * This function can be used to reset a USB device from an atomic + * context, where usb_reset_device() won't work (as it blocks). + * + * Doing a reset via this method is functionally equivalent to calling + * usb_reset_device(), except for the fact that it is delayed to a + * workqueue. This means that any drivers bound to other interfaces + * might be unbound, as well as users from usbfs in user space. + * + * Corner cases: + * + * - Scheduling two resets at the same time from two different drivers + * attached to two different interfaces of the same device is + * possible; depending on how the driver attached to each interface + * handles ->pre_reset(), the second reset might happen or not. + * + * - If a driver is unbound and it had a pending reset, the reset will + * be cancelled. + * + * - This function can be called during .probe() or .disconnect() + * times. On return from .disconnect(), any pending resets will be + * cancelled. + * + * There is no no need to lock/unlock the @reset_ws as schedule_work() + * does its own. + * + * NOTE: We don't do any reference count tracking because it is not + * needed. The lifecycle of the work_struct is tied to the + * usb_interface. Before destroying the interface we cancel the + * work_struct, so the fact that work_struct is queued and or + * running means the interface (and thus, the device) exist and + * are referenced. + */ +void usb_queue_reset_device(struct usb_interface *iface) +{ + schedule_work(&iface->reset_ws); +} +EXPORT_SYMBOL_GPL(usb_queue_reset_device); diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index cc47d36798b..aadf29f09c4 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1441,6 +1441,46 @@ static struct usb_interface_assoc_descriptor *find_iad(struct usb_device *dev, return retval; } + +/* + * Internal function to queue a device reset + * + * This is initialized into the workstruct in 'struct + * usb_device->reset_ws' that is launched by + * message.c:usb_set_configuration() when initializing each 'struct + * usb_interface'. + * + * It is safe to get the USB device without reference counts because + * the life cycle of @iface is bound to the life cycle of @udev. Then, + * this function will be ran only if @iface is alive (and before + * freeing it any scheduled instances of it will have been cancelled). + * + * We need to set a flag (usb_dev->reset_running) because when we call + * the reset, the interfaces might be unbound. The current interface + * cannot try to remove the queued work as it would cause a deadlock + * (you cannot remove your work from within your executing + * workqueue). This flag lets it know, so that + * usb_cancel_queued_reset() doesn't try to do it. + * + * See usb_queue_reset_device() for more details + */ +void __usb_queue_reset_device(struct work_struct *ws) +{ + int rc; + struct usb_interface *iface = + container_of(ws, struct usb_interface, reset_ws); + struct usb_device *udev = interface_to_usbdev(iface); + + rc = usb_lock_device_for_reset(udev, iface); + if (rc >= 0) { + iface->reset_running = 1; + usb_reset_device(udev); + iface->reset_running = 0; + usb_unlock_device(udev); + } +} + + /* * usb_set_configuration - Makes a particular device setting be current * @dev: the device whose configuration is being updated @@ -1611,6 +1651,7 @@ free_interfaces: intf->dev.type = &usb_if_device_type; intf->dev.groups = usb_interface_groups; intf->dev.dma_mask = dev->dev.dma_mask; + INIT_WORK(&intf->reset_ws, __usb_queue_reset_device); device_initialize(&intf->dev); mark_quiesced(intf); dev_set_name(&intf->dev, "%d-%s:%d.%d", diff --git a/include/linux/usb.h b/include/linux/usb.h index 859a88e6ce9..c8e55aa979d 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -120,6 +120,11 @@ enum usb_interface_condition { * to the sysfs representation for that device. * @pm_usage_cnt: PM usage counter for this interface; autosuspend is not * allowed unless the counter is 0. + * @reset_ws: Used for scheduling resets from atomic context. + * @reset_running: set to 1 if the interface is currently running a + * queued reset so that usb_cancel_queued_reset() doesn't try to + * remove from the workqueue when running inside the worker + * thread. See __usb_queue_reset_device(). * * USB device drivers attach to interfaces on a physical device. Each * interface encapsulates a single high level function, such as feeding @@ -168,10 +173,12 @@ struct usb_interface { unsigned needs_remote_wakeup:1; /* driver requires remote wakeup */ unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ unsigned needs_binding:1; /* needs delayed unbind/rebind */ + unsigned reset_running:1; struct device dev; /* interface specific device info */ struct device *usb_dev; int pm_usage_cnt; /* usage counter for autosuspend */ + struct work_struct reset_ws; /* for resets in atomic context */ }; #define to_usb_interface(d) container_of(d, struct usb_interface, dev) #define interface_to_usbdev(intf) \ @@ -507,6 +514,7 @@ extern int usb_lock_device_for_reset(struct usb_device *udev, /* USB port reset for device reinitialization */ extern int usb_reset_device(struct usb_device *dev); +extern void usb_queue_reset_device(struct usb_interface *dev); extern struct usb_device *usb_find_device(u16 vendor_id, u16 product_id); -- cgit v1.2.3-70-g09d2 From 3b23dd6f8a718e5339de4f7d86ce76a078b5f771 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 5 Dec 2008 14:10:34 -0500 Subject: USB: utilize the bus notifiers This patch (as1185) makes usbcore take advantage of the bus notifications sent out by the driver core. Now we can create all our device and interface attribute files before the device or interface uevent is broadcast. A side effect is that we no longer create the endpoint "pseudo" devices at the same time as a device or interface is registered -- it seems like a bad idea to try registering an endpoint before the registration of its parent is complete. So the routines for creating and removing endpoint devices have been split out and renamed, and they are called explicitly when needed. A new bitflag is used for keeping track of whether or not the interface's endpoint devices have been created, since (just as with the interface attributes) they vary with the altsetting and hence can be changed at random times. Signed-off-by: Alan Stern Cc: Kay Sievers Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/endpoint.c | 4 ++-- drivers/usb/core/hub.c | 18 ++++++---------- drivers/usb/core/message.c | 50 ++++++++++++++++++++++++++++++++++++++------- drivers/usb/core/sysfs.c | 22 +------------------- drivers/usb/core/usb.c | 37 +++++++++++++++++++++++++++++++++ drivers/usb/core/usb.h | 4 ++-- include/linux/usb.h | 2 ++ 7 files changed, 93 insertions(+), 44 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/endpoint.c b/drivers/usb/core/endpoint.c index 946fae43d62..e1710f260b4 100644 --- a/drivers/usb/core/endpoint.c +++ b/drivers/usb/core/endpoint.c @@ -276,7 +276,7 @@ static void ep_device_release(struct device *dev) kfree(ep_dev); } -int usb_create_ep_files(struct device *parent, +int usb_create_ep_devs(struct device *parent, struct usb_host_endpoint *endpoint, struct usb_device *udev) { @@ -340,7 +340,7 @@ exit: return retval; } -void usb_remove_ep_files(struct usb_host_endpoint *endpoint) +void usb_remove_ep_devs(struct usb_host_endpoint *endpoint) { struct ep_device *ep_dev = endpoint->ep_dev; diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 5abdc11be1e..756b8d9993f 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1437,17 +1437,12 @@ void usb_disconnect(struct usb_device **pdev) usb_disable_device(udev, 0); usb_hcd_synchronize_unlinks(udev); + usb_remove_ep_devs(&udev->ep0); usb_unlock_device(udev); - /* Remove the device-specific files from sysfs. This must be - * done with udev unlocked, because some of the attribute - * routines try to acquire the device lock. - */ - usb_remove_sysfs_dev_files(udev); - /* Unregister the device. The device driver is responsible - * for removing the device files from usbfs and sysfs and for - * de-configuring the device. + * for de-configuring the device and invoking the remove-device + * notifier chain (used by usbfs and possibly others). */ device_del(&udev->dev); @@ -1654,8 +1649,8 @@ int usb_new_device(struct usb_device *udev) announce_device(udev); /* Register the device. The device driver is responsible - * for adding the device files to sysfs and for configuring - * the device. + * for configuring the device and invoking the add-device + * notifier chain (used by usbfs and possibly others). */ err = device_add(&udev->dev); if (err) { @@ -1663,8 +1658,7 @@ int usb_new_device(struct usb_device *udev) goto fail; } - /* put device-specific files into sysfs */ - usb_create_sysfs_dev_files(udev); + (void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev); return err; fail: diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index aadf29f09c4..7943901c641 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1004,6 +1004,34 @@ int usb_clear_halt(struct usb_device *dev, int pipe) } EXPORT_SYMBOL_GPL(usb_clear_halt); +static int create_intf_ep_devs(struct usb_interface *intf) +{ + struct usb_device *udev = interface_to_usbdev(intf); + struct usb_host_interface *alt = intf->cur_altsetting; + int i; + + if (intf->ep_devs_created || intf->unregistering) + return 0; + + for (i = 0; i < alt->desc.bNumEndpoints; ++i) + (void) usb_create_ep_devs(&intf->dev, &alt->endpoint[i], udev); + intf->ep_devs_created = 1; + return 0; +} + +static void remove_intf_ep_devs(struct usb_interface *intf) +{ + struct usb_host_interface *alt = intf->cur_altsetting; + int i; + + if (!intf->ep_devs_created) + return; + + for (i = 0; i < alt->desc.bNumEndpoints; ++i) + usb_remove_ep_devs(&alt->endpoint[i]); + intf->ep_devs_created = 0; +} + /** * usb_disable_endpoint -- Disable an endpoint by address * @dev: the device whose endpoint is being disabled @@ -1092,7 +1120,7 @@ void usb_disable_device(struct usb_device *dev, int skip_ep0) dev_dbg(&dev->dev, "unregistering interface %s\n", dev_name(&interface->dev)); interface->unregistering = 1; - usb_remove_sysfs_intf_files(interface); + remove_intf_ep_devs(interface); device_del(&interface->dev); } @@ -1235,8 +1263,10 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) */ /* prevent submissions using previous endpoint settings */ - if (iface->cur_altsetting != alt) + if (iface->cur_altsetting != alt) { + remove_intf_ep_devs(iface); usb_remove_sysfs_intf_files(iface); + } usb_disable_interface(dev, iface); iface->cur_altsetting = alt; @@ -1272,9 +1302,10 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) * (Likewise, EP0 never "halts" on well designed devices.) */ usb_enable_interface(dev, iface); - if (device_is_registered(&iface->dev)) + if (device_is_registered(&iface->dev)) { usb_create_sysfs_intf_files(iface); - + create_intf_ep_devs(iface); + } return 0; } EXPORT_SYMBOL_GPL(usb_set_interface); @@ -1334,7 +1365,6 @@ int usb_reset_configuration(struct usb_device *dev) struct usb_interface *intf = config->interface[i]; struct usb_host_interface *alt; - usb_remove_sysfs_intf_files(intf); alt = usb_altnum_to_altsetting(intf, 0); /* No altsetting 0? We'll assume the first altsetting. @@ -1345,10 +1375,16 @@ int usb_reset_configuration(struct usb_device *dev) if (!alt) alt = &intf->altsetting[0]; + if (alt != intf->cur_altsetting) { + remove_intf_ep_devs(intf); + usb_remove_sysfs_intf_files(intf); + } intf->cur_altsetting = alt; usb_enable_interface(dev, intf); - if (device_is_registered(&intf->dev)) + if (device_is_registered(&intf->dev)) { usb_create_sysfs_intf_files(intf); + create_intf_ep_devs(intf); + } } return 0; } @@ -1682,7 +1718,7 @@ free_interfaces: dev_name(&intf->dev), ret); continue; } - usb_create_sysfs_intf_files(intf); + create_intf_ep_devs(intf); } usb_autosuspend_device(dev); diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 0f0ccf64011..4cc2456ef3b 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -629,9 +629,6 @@ int usb_create_sysfs_dev_files(struct usb_device *udev) struct device *dev = &udev->dev; int retval; - /* Unforunately these attributes cannot be created before - * the uevent is broadcast. - */ retval = device_create_bin_file(dev, &dev_bin_attr_descriptors); if (retval) goto error; @@ -643,11 +640,7 @@ int usb_create_sysfs_dev_files(struct usb_device *udev) retval = add_power_attributes(dev); if (retval) goto error; - - retval = usb_create_ep_files(dev, &udev->ep0, udev); - if (retval) - goto error; - return 0; + return retval; error: usb_remove_sysfs_dev_files(udev); return retval; @@ -657,7 +650,6 @@ void usb_remove_sysfs_dev_files(struct usb_device *udev) { struct device *dev = &udev->dev; - usb_remove_ep_files(&udev->ep0); remove_power_attributes(dev); remove_persist_attributes(dev); device_remove_bin_file(dev, &dev_bin_attr_descriptors); @@ -816,36 +808,24 @@ int usb_create_sysfs_intf_files(struct usb_interface *intf) { struct usb_device *udev = interface_to_usbdev(intf); struct usb_host_interface *alt = intf->cur_altsetting; - int i; int retval; if (intf->sysfs_files_created || intf->unregistering) return 0; - /* The interface string may be present in some altsettings - * and missing in others. Hence its attribute cannot be created - * before the uevent is broadcast. - */ if (alt->string == NULL) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string) retval = device_create_file(&intf->dev, &dev_attr_interface); - for (i = 0; i < alt->desc.bNumEndpoints; ++i) - usb_create_ep_files(&intf->dev, &alt->endpoint[i], udev); intf->sysfs_files_created = 1; return 0; } void usb_remove_sysfs_intf_files(struct usb_interface *intf) { - struct usb_host_interface *alt = intf->cur_altsetting; - int i; - if (!intf->sysfs_files_created) return; - for (i = 0; i < alt->desc.bNumEndpoints; ++i) - usb_remove_ep_files(&alt->endpoint[i]); device_remove_file(&intf->dev, &dev_attr_interface); intf->sysfs_files_created = 0; } diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index 4c98f3975af..c0821564a3f 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -970,6 +970,37 @@ int usb_disabled(void) } EXPORT_SYMBOL_GPL(usb_disabled); +/* + * Notifications of device and interface registration + */ +static int usb_bus_notify(struct notifier_block *nb, unsigned long action, + void *data) +{ + struct device *dev = data; + + switch (action) { + case BUS_NOTIFY_ADD_DEVICE: + if (dev->type == &usb_device_type) + (void) usb_create_sysfs_dev_files(to_usb_device(dev)); + else if (dev->type == &usb_if_device_type) + (void) usb_create_sysfs_intf_files( + to_usb_interface(dev)); + break; + + case BUS_NOTIFY_DEL_DEVICE: + if (dev->type == &usb_device_type) + usb_remove_sysfs_dev_files(to_usb_device(dev)); + else if (dev->type == &usb_if_device_type) + usb_remove_sysfs_intf_files(to_usb_interface(dev)); + break; + } + return 0; +} + +static struct notifier_block usb_bus_nb = { + .notifier_call = usb_bus_notify, +}; + /* * Init */ @@ -987,6 +1018,9 @@ static int __init usb_init(void) retval = bus_register(&usb_bus_type); if (retval) goto bus_register_failed; + retval = bus_register_notifier(&usb_bus_type, &usb_bus_nb); + if (retval) + goto bus_notifier_failed; retval = usb_host_init(); if (retval) goto host_init_failed; @@ -1021,6 +1055,8 @@ driver_register_failed: major_init_failed: usb_host_cleanup(); host_init_failed: + bus_unregister_notifier(&usb_bus_type, &usb_bus_nb); +bus_notifier_failed: bus_unregister(&usb_bus_type); bus_register_failed: ksuspend_usb_cleanup(); @@ -1044,6 +1080,7 @@ static void __exit usb_exit(void) usb_devio_cleanup(); usb_hub_cleanup(); usb_host_cleanup(); + bus_unregister_notifier(&usb_bus_type, &usb_bus_nb); bus_unregister(&usb_bus_type); ksuspend_usb_cleanup(); } diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 9fb195665fa..381eae90c3b 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -6,10 +6,10 @@ extern int usb_create_sysfs_dev_files(struct usb_device *dev); extern void usb_remove_sysfs_dev_files(struct usb_device *dev); extern int usb_create_sysfs_intf_files(struct usb_interface *intf); extern void usb_remove_sysfs_intf_files(struct usb_interface *intf); -extern int usb_create_ep_files(struct device *parent, +extern int usb_create_ep_devs(struct device *parent, struct usb_host_endpoint *endpoint, struct usb_device *udev); -extern void usb_remove_ep_files(struct usb_host_endpoint *endpoint); +extern void usb_remove_ep_devs(struct usb_host_endpoint *endpoint); extern void usb_enable_endpoint(struct usb_device *dev, struct usb_host_endpoint *ep); diff --git a/include/linux/usb.h b/include/linux/usb.h index 74d0b9990c7..e9d63562325 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -108,6 +108,7 @@ enum usb_interface_condition { * (in probe()), bound to a driver, or unbinding (in disconnect()) * @is_active: flag set when the interface is bound and not suspended. * @sysfs_files_created: sysfs attributes exist + * @ep_devs_created: endpoint child pseudo-devices exist * @unregistering: flag set when the interface is being unregistered * @needs_remote_wakeup: flag set when the driver requires remote-wakeup * capability during autosuspend. @@ -169,6 +170,7 @@ struct usb_interface { enum usb_interface_condition condition; /* state of binding */ unsigned is_active:1; /* the interface is not suspended */ unsigned sysfs_files_created:1; /* the sysfs attributes exist */ + unsigned ep_devs_created:1; /* endpoint "devices" exist */ unsigned unregistering:1; /* unregistration is in progress */ unsigned needs_remote_wakeup:1; /* driver requires remote wakeup */ unsigned needs_altsetting0:1; /* switch to altsetting 0 is pending */ -- cgit v1.2.3-70-g09d2 From df718962bf91c7bd345060aadaa24b03f6140b07 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Fri, 19 Dec 2008 10:27:56 -0500 Subject: USB: cancel pending Set-Config requests if userspace gets there first This patch (as1195) eliminates a potential problem identified by Oliver Neukum. When a driver queues an asynchronous Set-Config request using usb_driver_set_configuration(), the request should be cancelled if userspace changes the configuration first. The patch introduces a linked list of pending async Set-Config requests, and uses it to invalidate the requests for a particular device whenever that device's configuration is set. Signed-off-by: Alan Stern Cc: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/message.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 7943901c641..5589686981f 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -18,6 +18,8 @@ #include "hcd.h" /* for usbcore internals */ #include "usb.h" +static void cancel_async_set_config(struct usb_device *udev); + struct api_context { struct completion done; int status; @@ -1636,6 +1638,9 @@ free_interfaces: if (dev->state != USB_STATE_ADDRESS) usb_disable_device(dev, 1); /* Skip ep0 */ + /* Get rid of pending async Set-Config requests for this device */ + cancel_async_set_config(dev); + ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), USB_REQ_SET_CONFIGURATION, 0, configuration, 0, NULL, 0, USB_CTRL_SET_TIMEOUT); @@ -1725,10 +1730,14 @@ free_interfaces: return 0; } +static LIST_HEAD(set_config_list); +static DEFINE_SPINLOCK(set_config_lock); + struct set_config_request { struct usb_device *udev; int config; struct work_struct work; + struct list_head node; }; /* Worker routine for usb_driver_set_configuration() */ @@ -1736,14 +1745,35 @@ static void driver_set_config_work(struct work_struct *work) { struct set_config_request *req = container_of(work, struct set_config_request, work); + struct usb_device *udev = req->udev; - usb_lock_device(req->udev); - usb_set_configuration(req->udev, req->config); - usb_unlock_device(req->udev); - usb_put_dev(req->udev); + usb_lock_device(udev); + spin_lock(&set_config_lock); + list_del(&req->node); + spin_unlock(&set_config_lock); + + if (req->config >= -1) /* Is req still valid? */ + usb_set_configuration(udev, req->config); + usb_unlock_device(udev); + usb_put_dev(udev); kfree(req); } +/* Cancel pending Set-Config requests for a device whose configuration + * was just changed + */ +static void cancel_async_set_config(struct usb_device *udev) +{ + struct set_config_request *req; + + spin_lock(&set_config_lock); + list_for_each_entry(req, &set_config_list, node) { + if (req->udev == udev) + req->config = -999; /* Mark as cancelled */ + } + spin_unlock(&set_config_lock); +} + /** * usb_driver_set_configuration - Provide a way for drivers to change device configurations * @udev: the device whose configuration is being updated @@ -1775,6 +1805,10 @@ int usb_driver_set_configuration(struct usb_device *udev, int config) req->config = config; INIT_WORK(&req->work, driver_set_config_work); + spin_lock(&set_config_lock); + list_add(&req->node, &set_config_list); + spin_unlock(&set_config_lock); + usb_get_dev(udev); schedule_work(&req->work); return 0; -- cgit v1.2.3-70-g09d2 From 2caf7fcdb8532045680f06b67b9e63f0c9613aaa Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 31 Dec 2008 11:31:33 -0500 Subject: USB: re-enable interface after driver unbinds This patch (as1197) fixes an error introduced recently. Since a significant number of devices can't handle Set-Interface requests, we no longer call usb_set_interface() when a driver unbinds from an interface, provided the interface is already in altsetting 0. However the interface still does get disabled, and the call to usb_set_interface() was the only thing re-enabling it. Since the interface doesn't get re-enabled, further attempts to use it fail. So the patch adds a call to usb_enable_interface() when a driver unbinds and the interface is in altsetting 0. For this to work right, the interface's endpoints have to be re-enabled but their toggles have to be left alone. Therefore an additional argument is added to usb_enable_endpoint() and usb_enable_interface(), a flag indicating whether or not the endpoint toggles should be reset. This is a forward-ported version of a patch which fixes Bugzilla #12301. Signed-off-by: Alan Stern Reported-by: David Roka Reported-by: Erik Ekman Tested-by: Erik Ekman Tested-by: Alon Bar-Lev Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/driver.c | 9 ++++++--- drivers/usb/core/hub.c | 2 +- drivers/usb/core/message.c | 25 +++++++++++++++---------- drivers/usb/core/usb.c | 2 +- drivers/usb/core/usb.h | 4 +++- 5 files changed, 26 insertions(+), 16 deletions(-) (limited to 'drivers/usb/core/message.c') diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 41c06025506..98760553bc9 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -295,9 +295,12 @@ static int usb_unbind_interface(struct device *dev) * altsetting means creating new endpoint device entries). * When either of these happens, defer the Set-Interface. */ - if (intf->cur_altsetting->desc.bAlternateSetting == 0) - ; /* Already in altsetting 0 so skip Set-Interface */ - else if (!error && intf->dev.power.status == DPM_ON) + if (intf->cur_altsetting->desc.bAlternateSetting == 0) { + /* Already in altsetting 0 so skip Set-Interface. + * Just re-enable it without affecting the endpoint toggles. + */ + usb_enable_interface(udev, intf, false); + } else if (!error && intf->dev.power.status == DPM_ON) usb_set_interface(udev, intf->altsetting[0]. desc.bInterfaceNumber, 0); else diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 756b8d9993f..d5d0e40b1e2 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2384,7 +2384,7 @@ void usb_ep0_reinit(struct usb_device *udev) { usb_disable_endpoint(udev, 0 + USB_DIR_IN); usb_disable_endpoint(udev, 0 + USB_DIR_OUT); - usb_enable_endpoint(udev, &udev->ep0); + usb_enable_endpoint(udev, &udev->ep0, true); } EXPORT_SYMBOL_GPL(usb_ep0_reinit); diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 5589686981f..de51667dd64 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1143,22 +1143,26 @@ void usb_disable_device(struct usb_device *dev, int skip_ep0) * usb_enable_endpoint - Enable an endpoint for USB communications * @dev: the device whose interface is being enabled * @ep: the endpoint + * @reset_toggle: flag to set the endpoint's toggle back to 0 * - * Resets the endpoint toggle, and sets dev->ep_{in,out} pointers. + * Resets the endpoint toggle if asked, and sets dev->ep_{in,out} pointers. * For control endpoints, both the input and output sides are handled. */ -void usb_enable_endpoint(struct usb_device *dev, struct usb_host_endpoint *ep) +void usb_enable_endpoint(struct usb_device *dev, struct usb_host_endpoint *ep, + bool reset_toggle) { int epnum = usb_endpoint_num(&ep->desc); int is_out = usb_endpoint_dir_out(&ep->desc); int is_control = usb_endpoint_xfer_control(&ep->desc); if (is_out || is_control) { - usb_settoggle(dev, epnum, 1, 0); + if (reset_toggle) + usb_settoggle(dev, epnum, 1, 0); dev->ep_out[epnum] = ep; } if (!is_out || is_control) { - usb_settoggle(dev, epnum, 0, 0); + if (reset_toggle) + usb_settoggle(dev, epnum, 0, 0); dev->ep_in[epnum] = ep; } ep->enabled = 1; @@ -1168,17 +1172,18 @@ void usb_enable_endpoint(struct usb_device *dev, struct usb_host_endpoint *ep) * usb_enable_interface - Enable all the endpoints for an interface * @dev: the device whose interface is being enabled * @intf: pointer to the interface descriptor + * @reset_toggles: flag to set the endpoints' toggles back to 0 * * Enables all the endpoints for the interface's current altsetting. */ -static void usb_enable_interface(struct usb_device *dev, - struct usb_interface *intf) +void usb_enable_interface(struct usb_device *dev, + struct usb_interface *intf, bool reset_toggles) { struct usb_host_interface *alt = intf->cur_altsetting; int i; for (i = 0; i < alt->desc.bNumEndpoints; ++i) - usb_enable_endpoint(dev, &alt->endpoint[i]); + usb_enable_endpoint(dev, &alt->endpoint[i], reset_toggles); } /** @@ -1303,7 +1308,7 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) * during the SETUP stage - hence EP0 toggles are "don't care" here. * (Likewise, EP0 never "halts" on well designed devices.) */ - usb_enable_interface(dev, iface); + usb_enable_interface(dev, iface, true); if (device_is_registered(&iface->dev)) { usb_create_sysfs_intf_files(iface); create_intf_ep_devs(iface); @@ -1382,7 +1387,7 @@ int usb_reset_configuration(struct usb_device *dev) usb_remove_sysfs_intf_files(intf); } intf->cur_altsetting = alt; - usb_enable_interface(dev, intf); + usb_enable_interface(dev, intf, true); if (device_is_registered(&intf->dev)) { usb_create_sysfs_intf_files(intf); create_intf_ep_devs(intf); @@ -1685,7 +1690,7 @@ free_interfaces: alt = &intf->altsetting[0]; intf->cur_altsetting = alt; - usb_enable_interface(dev, intf); + usb_enable_interface(dev, intf, true); intf->dev.parent = &dev->dev; intf->dev.driver = NULL; intf->dev.bus = &usb_bus_type; diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index c0821564a3f..dcfc072630c 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -362,7 +362,7 @@ struct usb_device *usb_alloc_dev(struct usb_device *parent, dev->ep0.desc.bLength = USB_DT_ENDPOINT_SIZE; dev->ep0.desc.bDescriptorType = USB_DT_ENDPOINT; /* ep0 maxpacket comes later, from device descriptor */ - usb_enable_endpoint(dev, &dev->ep0); + usb_enable_endpoint(dev, &dev->ep0, true); dev->can_submit = 1; /* Save readable and stable topology id, distinguishing devices diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 381eae90c3b..386177867a8 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -12,7 +12,9 @@ extern int usb_create_ep_devs(struct device *parent, extern void usb_remove_ep_devs(struct usb_host_endpoint *endpoint); extern void usb_enable_endpoint(struct usb_device *dev, - struct usb_host_endpoint *ep); + struct usb_host_endpoint *ep, bool reset_toggle); +extern void usb_enable_interface(struct usb_device *dev, + struct usb_interface *intf, bool reset_toggles); extern void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr); extern void usb_disable_interface(struct usb_device *dev, struct usb_interface *intf); -- cgit v1.2.3-70-g09d2