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 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/message.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) (limited to 'drivers/usb/core/message.c') 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", -- cgit v1.2.3 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/message.c | 50 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 7 deletions(-) (limited to 'drivers/usb/core/message.c') 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); -- cgit v1.2.3 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 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/message.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers/usb/core/message.c') 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; -- cgit v1.2.3