From 4489a5712b086621a6c3f669057d2996245cd3fb Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 27 Apr 2006 15:54:22 -0400 Subject: [PATCH] USB: usbcore: always turn on hub port power Some hubs claim not to support port-power switching, and right now the hub driver believes them and does not enable power to their ports. However it turns out that even though they don't actually switch power, they do ignore all events on a port until told to turn on the power! This problem has been reported by several users. This revised patch (as672b) makes the hub driver always try to turn on port power to all hubs, regardless of what the hub descriptor says. It also adds a comment explaining the need for this. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 90b8d43c6b3..1b1846e6473 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -432,15 +432,22 @@ static void hub_power_on(struct usb_hub *hub) { int port1; unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2; - u16 wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics); - - /* if hub supports power switching, enable power on each port */ - if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2) { + u16 wHubCharacteristics = + le16_to_cpu(hub->descriptor->wHubCharacteristics); + + /* Enable power on each port. Some hubs have reserved values + * of LPSM (> 2) in their descriptors, even though they are + * USB 2.0 hubs. Some hubs do not implement port-power switching + * but only emulate it. In all cases, the ports won't work + * unless we send these messages to the hub. + */ + if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2) dev_dbg(hub->intfdev, "enabling power on all ports\n"); - for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) - set_port_feature(hub->hdev, port1, - USB_PORT_FEAT_POWER); - } + else + dev_dbg(hub->intfdev, "trying to enable port power on " + "non-switchable hub\n"); + for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) + set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER); /* Wait at least 100 msec for power to become stable */ msleep(max(pgood_delay, (unsigned) 100)); -- cgit v1.2.3-70-g09d2 From 1fbe75e12f0dd567c86533e13ba2605f3ecad2e1 Mon Sep 17 00:00:00 2001 From: Daniel Drake Date: Fri, 26 May 2006 21:36:28 +0100 Subject: [PATCH] USB: print message when device is rejected due to insufficient power 2.6.16 introduces USB power budgeting in the Linux kernel, and since then, a fair number of users have observed that some of their devices no longer work in unpowered hubs (this is not a bug, the devices claim that they need more than 100mA). The very least we can do is print an informational message to the kernel log when this happens, otherwise it is not at all clear why the device was not accepted. Signed-off-by: Daniel Drake Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 1b1846e6473..89ebe6a23a9 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1176,6 +1176,7 @@ static int choose_configuration(struct usb_device *udev) { int i; int num_configs; + int insufficient_power = 0; struct usb_host_config *c, *best; best = NULL; @@ -1228,8 +1229,10 @@ static int choose_configuration(struct usb_device *udev) */ /* Rule out configs that draw too much bus current */ - if (c->desc.bMaxPower * 2 > udev->bus_mA) + if (c->desc.bMaxPower * 2 > udev->bus_mA) { + insufficient_power++; continue; + } /* If the first config's first interface is COMM/2/0xff * (MSFT RNDIS), rule it out unless Linux has host-side @@ -1263,6 +1266,11 @@ static int choose_configuration(struct usb_device *udev) best = c; } + if (insufficient_power > 0) + dev_info(&udev->dev, "rejected %d configuration%s " + "due to insufficient available bus power\n", + insufficient_power, plural(insufficient_power)); + if (best) { i = best->desc.bConfigurationValue; dev_info(&udev->dev, -- cgit v1.2.3-70-g09d2 From 3f8f4a18f4f76a67fe5b80da1d3a3940e41a14a8 Mon Sep 17 00:00:00 2001 From: Giridhar Pemmasani Date: Tue, 30 May 2006 10:12:30 -0400 Subject: [PATCH] usbcore: Fix broken RNDIS config selection RNDIS devices don't get configured owing to a typo in choose_configuration(). This patch from Giridhar Pemmasani fixes the typo. From: Giridhar Pemmasani Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 89ebe6a23a9..f41c08946a5 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1241,7 +1241,7 @@ static int choose_configuration(struct usb_device *udev) && desc->bInterfaceClass == USB_CLASS_COMM && desc->bInterfaceSubClass == 2 && desc->bInterfaceProtocol == 0xff) { -#ifndef CONFIG_USB_NET_RNDIS +#ifndef CONFIG_USB_NET_RNDIS_HOST continue; #else best = c; -- cgit v1.2.3-70-g09d2 From 79efa097e75018a2918155f343f0e08e61ee8a8c Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 1 Jun 2006 13:33:42 -0400 Subject: [PATCH] usbcore: port reset for composite devices This patch (as699) adds usb_reset_composite_device(), a routine for sending a USB port reset to a device with multiple interfaces owned by different drivers. Drivers are notified about impending and completed resets through two new methods in the usb_driver structure. The patch modifieds the usbfs ioctl code to make it use the new routine instead of usb_reset_device(). Follow-up patches will modify the hub, usb-storage, and usbhid drivers so they can utilize this new API. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/devio.c | 3 +- drivers/usb/core/hub.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++-- drivers/usb/core/usb.c | 1 + include/linux/usb.h | 9 ++++++ 4 files changed, 92 insertions(+), 5 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index b04ede772f2..df3fb57d71e 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -823,8 +823,7 @@ static int proc_connectinfo(struct dev_state *ps, void __user *arg) static int proc_resetdevice(struct dev_state *ps) { - return usb_reset_device(ps->dev); - + return usb_reset_composite_device(ps->dev, NULL); } static int proc_setintf(struct dev_state *ps, void __user *arg) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index f41c08946a5..37c67d7e8b8 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -3007,9 +3007,9 @@ static int config_descriptors_changed(struct usb_device *udev) * usb_reset_device - perform a USB port reset to reinitialize a device * @udev: device to reset (not in SUSPENDED or NOTATTACHED state) * - * WARNING - don't reset any device unless drivers for all of its - * interfaces are expecting that reset! Maybe some driver->reset() - * method should eventually help ensure sufficient cooperation. + * WARNING - don't use this routine to reset a composite device + * (one with multiple interfaces owned by separate drivers)! + * Use usb_reset_composite_device() instead. * * Do a port reset, reassign the device's address, and establish its * former operating configuration. If the reset fails, or the device's @@ -3125,3 +3125,81 @@ re_enumerate: hub_port_logical_disconnect(parent_hub, port1); return -ENODEV; } + +/** + * usb_reset_composite_device - warn interface drivers and perform a USB port reset + * @udev: device to reset (not in SUSPENDED or NOTATTACHED state) + * @iface: interface bound to the driver making the request (optional) + * + * Warns all drivers bound to registered interfaces (using their pre_reset + * method), performs the port reset, and then lets the drivers know that + * the reset is over (using their post_reset method). + * + * Return value is the same as for usb_reset_device(). + * + * The caller must own the device lock. For example, it's safe to use + * this from a driver probe() routine after downloading new firmware. + * For calls that might not occur during probe(), drivers should lock + * the device using usb_lock_device_for_reset(). + * + * The interface locks are acquired during the pre_reset stage and released + * during the post_reset stage. However if iface is not NULL and is + * currently being probed, we assume that the caller already owns its + * lock. + */ +int usb_reset_composite_device(struct usb_device *udev, + struct usb_interface *iface) +{ + int ret; + struct usb_host_config *config = udev->actconfig; + + if (udev->state == USB_STATE_NOTATTACHED || + udev->state == USB_STATE_SUSPENDED) { + dev_dbg(&udev->dev, "device reset not allowed in state %d\n", + udev->state); + return -EINVAL; + } + + if (iface && iface->condition != USB_INTERFACE_BINDING) + iface = NULL; + + if (config) { + int i; + struct usb_interface *cintf; + struct usb_driver *drv; + + for (i = 0; i < config->desc.bNumInterfaces; ++i) { + cintf = config->interface[i]; + if (cintf != iface) + down(&cintf->dev.sem); + if (device_is_registered(&cintf->dev) && + cintf->dev.driver) { + drv = to_usb_driver(cintf->dev.driver); + if (drv->pre_reset) + (drv->pre_reset)(cintf); + } + } + } + + ret = usb_reset_device(udev); + + if (config) { + int i; + struct usb_interface *cintf; + struct usb_driver *drv; + + for (i = config->desc.bNumInterfaces - 1; i >= 0; --i) { + cintf = config->interface[i]; + if (device_is_registered(&cintf->dev) && + cintf->dev.driver) { + drv = to_usb_driver(cintf->dev.driver); + if (drv->post_reset) + (drv->post_reset)(cintf); + } + if (cintf != iface) + up(&cintf->dev.sem); + } + } + + return ret; +} diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c index b7fdc1cd134..51531075130 100644 --- a/drivers/usb/core/usb.c +++ b/drivers/usb/core/usb.c @@ -1207,6 +1207,7 @@ EXPORT_SYMBOL(usb_ifnum_to_if); EXPORT_SYMBOL(usb_altnum_to_altsetting); EXPORT_SYMBOL(usb_reset_device); +EXPORT_SYMBOL(usb_reset_composite_device); EXPORT_SYMBOL(__usb_get_extra_descriptor); diff --git a/include/linux/usb.h b/include/linux/usb.h index 317ec9f28bc..5ad30cefe7b 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -386,6 +386,8 @@ 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 int usb_reset_composite_device(struct usb_device *dev, + struct usb_interface *iface); extern struct usb_device *usb_find_device(u16 vendor_id, u16 product_id); @@ -554,6 +556,10 @@ struct usb_dynids { * do (or don't) show up otherwise in the filesystem. * @suspend: Called when the device is going to be suspended by the system. * @resume: Called when the device is being resumed by the system. + * @pre_reset: Called by usb_reset_composite_device() when the device + * is about to be reset. + * @post_reset: Called by usb_reset_composite_device() after the device + * has been reset. * @id_table: USB drivers use ID table to support hotplugging. * Export this with MODULE_DEVICE_TABLE(usb,...). This must be set * or your driver's probe function will never get called. @@ -592,6 +598,9 @@ struct usb_driver { int (*suspend) (struct usb_interface *intf, pm_message_t message); int (*resume) (struct usb_interface *intf); + void (*pre_reset) (struct usb_interface *intf); + void (*post_reset) (struct usb_interface *intf); + const struct usb_device_id *id_table; struct usb_dynids dynids; -- cgit v1.2.3-70-g09d2 From 7de18d8bf4b470752e799e219b38b6dff4b49993 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 1 Jun 2006 13:37:24 -0400 Subject: [PATCH] USB hub: use usb_reset_composite_device This patch (as700) modifies the hub driver to take advantage of the new usb_reset_composite_device API. The existing code had special-case calls stuck into usb_reset_device, just before and after the reset. With the new version there's no need for special-case stuff; it all happens naturally in the form of pre_reset and post_reset notifications. Signed-off-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/hub.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) (limited to 'drivers/usb/core/hub.c') diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 37c67d7e8b8..e1731ff8af4 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -525,15 +525,16 @@ static int hub_port_disable(struct usb_hub *hub, int port1, int set_state) /* caller has locked the hub device */ -static void hub_pre_reset(struct usb_hub *hub, int disable_ports) +static void hub_pre_reset(struct usb_interface *intf) { + struct usb_hub *hub = usb_get_intfdata(intf); struct usb_device *hdev = hub->hdev; int port1; for (port1 = 1; port1 <= hdev->maxchild; ++port1) { if (hdev->children[port1 - 1]) { usb_disconnect(&hdev->children[port1 - 1]); - if (disable_ports) + if (hub->error == 0) hub_port_disable(hub, port1, 0); } } @@ -541,8 +542,10 @@ static void hub_pre_reset(struct usb_hub *hub, int disable_ports) } /* caller has locked the hub device */ -static void hub_post_reset(struct usb_hub *hub) +static void hub_post_reset(struct usb_interface *intf) { + struct usb_hub *hub = usb_get_intfdata(intf); + hub_activate(hub); hub_power_on(hub); } @@ -802,15 +805,16 @@ static void hub_disconnect(struct usb_interface *intf) struct usb_hub *hub = usb_get_intfdata (intf); struct usb_device *hdev; + /* Disconnect all children and quiesce the hub */ + hub->error = 0; + hub_pre_reset(intf); + usb_set_intfdata (intf, NULL); hdev = hub->hdev; if (hdev->speed == USB_SPEED_HIGH) highspeed_hubs--; - /* Disconnect all children and quiesce the hub */ - hub_pre_reset(hub, 1); - usb_free_urb(hub->urb); hub->urb = NULL; @@ -2747,7 +2751,8 @@ static void hub_events(void) /* If the hub has died, clean up after it */ if (hdev->state == USB_STATE_NOTATTACHED) { - hub_pre_reset(hub, 0); + hub->error = -ENODEV; + hub_pre_reset(intf); goto loop; } @@ -2759,7 +2764,7 @@ static void hub_events(void) dev_dbg (hub_dev, "resetting for error %d\n", hub->error); - ret = usb_reset_device(hdev); + ret = usb_reset_composite_device(hdev, intf); if (ret) { dev_dbg (hub_dev, "error resetting hub: %d\n", ret); @@ -2928,6 +2933,8 @@ static struct usb_driver hub_driver = { .disconnect = hub_disconnect, .suspend = hub_suspend, .resume = hub_resume, + .pre_reset = hub_pre_reset, + .post_reset = hub_post_reset, .ioctl = hub_ioctl, .id_table = hub_id_table, }; @@ -3033,7 +3040,6 @@ int usb_reset_device(struct usb_device *udev) struct usb_device *parent_hdev = udev->parent; struct usb_hub *parent_hub; struct usb_device_descriptor descriptor = udev->descriptor; - struct usb_hub *hub = NULL; int i, ret = 0; int port1 = udev->portnum; @@ -3051,14 +3057,6 @@ int usb_reset_device(struct usb_device *udev) } parent_hub = hdev_to_hub(parent_hdev); - /* If we're resetting an active hub, take some special actions */ - if (udev->actconfig && udev->actconfig->desc.bNumInterfaces > 0 && - udev->actconfig->interface[0]->dev.driver == - &hub_driver.driver && - (hub = hdev_to_hub(udev)) != NULL) { - hub_pre_reset(hub, 0); - } - set_bit(port1, parent_hub->busy_bits); for (i = 0; i < SET_CONFIG_TRIES; ++i) { @@ -3117,8 +3115,6 @@ int usb_reset_device(struct usb_device *udev) } done: - if (hub) - hub_post_reset(hub); return 0; re_enumerate: -- cgit v1.2.3-70-g09d2