David: Attached are two patches. The first contains a number of cleanups and some small bug fixes for the hub driver, things I will submit separately (along with additional cleanups) in the near future. The only aspect of interest here is that it adds code to set udev->dev.power.power_state when udev is suspended or resumed. Although not really necessary, it helps during debugging to be able to see the correct current state of the device. Eventually power_state will change or disappear, but for now it may as well be accurate. (In my opinion, we could do a lot worse than change it to a string containing a readable form of udev->state; we might even use it to replace udev->state entirely.)
The first patch is a prerequisite for the second patch, which implements hub autosuspend. It is functional, so you can try it out. Several pieces of it are worth discussing. The hub inactivity timeout is controlled by a module parameter, named hub_autosuspend. It can be set to the number of seconds to wait before suspending an inactive hub, or 0 to prevent autosuspend. One problem not addressed here is what should happen if the user changes the parameter using sysfs, since existing timers won't be updated to reflect the new timeout. This issue will affect every subsystem choosing to implement autosuspend timeouts this way; no doubt some common solution will be arranged but I don't know what it will be. The autosuspend_try() routine includes a test to disable autosuspend for root hubs. I put that in because UHCI doesn't support root hub autoresume correctly. When I commented out the test and used it with an OHCI controller, the new code didn't play well with ohci-hcd's autosuspend code. Both routines tried to suspend the root hub, and then plugging in a USB device caused a nice hard lockup with nothing written to the console log. I'm convinced this is because of the way the OHCI driver suspends the root hub without calling usb_suspend_device. On the other hand, autosuspend of external hubs works very well. The autosuspend stuff isn't completely integrated with the HCDs. In particular, the parts that deal with a controller that has lost power by marking all the children as NOTATTACHED will mess up the count of active children. The correct solution is to have the HCDs treat the situation in a higher-level fashion, such as calling usb_reset_device for the root hub. The hub_pre_reset routine will then do everything necessary. A similar tactic should be used in HC_died... something to work on in the future. Finally, locking is problematic. One possible approach would be to make the new hub->unsuspended_children_count an atomic_t so that it could safely be updated without locking the hub. Instead, I'm coming to favor the technique of always locking the parent hub as well as the device whenever a device is suspended or resumed. An advantage to this technique is that it makes locktree unnecessary. A disadvantage is that (assuming the USB style of locking is adopted throughout the driver-model device tree) other subsystems may not need or want to go to the trouble of acquiring two locks during suspend/resume. Clearly this question won't be resolved right now, but it's something to think about. Anyway, the code is here. Maybe it will help clarify the requirements for HCD support of suspending/resuming root hubs. Alan Stern
Numerous cleanups for the hub driver, including a few small bug fixes, nothing very important: Pass hub rather than hdev as an argument to internal routines. Don't set extraneous bits in event_bits and change_bits. Use the proper spinlock in hub_ioctl. Call usb_set_device_state() in the suspend/resume routines rather than directly altering udev->state. Set udev->dev.power.power_state in the suspend/resume routines. Note that in hub_port_resume(), udev may be NULL. In hub_events(), don't try to print debugging messages while not holding some kind of lock for protection. Also in hub_events(), take a reference to the hub interface rather than the hub device; this simplifies checking for disconnects. Don't neglect to release the reference if locktree() fails. --- a/drivers/usb/core/hub.c Fri Nov 19 17:20:55 2004 +++ b/drivers/usb/core/hub.c Thu Nov 25 22:58:39 2004 @@ -94,10 +94,10 @@ } #endif -/* for dev_info, dev_dbg, etc */ -static inline struct device *hubdev (struct usb_device *hdev) +/* Note that hdev or one of its children must be locked! */ +static inline struct usb_hub *hdev_to_hub(struct usb_device *hdev) { - return &hdev->actconfig->interface[0]->dev; + return usb_get_intfdata(hdev->actconfig->interface[0]); } /* USB 2.0 spec Section 11.24.4.5 */ @@ -148,15 +148,15 @@ * for info about using port indicators */ static void set_port_led( - struct usb_device *hdev, + struct usb_hub *hub, int port, int selector ) { - int status = set_port_feature(hdev, (selector << 8) | port, + int status = set_port_feature(hub->hdev, (selector << 8) | port, USB_PORT_FEAT_INDICATOR); if (status < 0) - dev_dbg (hubdev (hdev), + dev_dbg (&hub->intf->dev, "port %d indicator %s status %d\n", port, ({ char *s; switch (selector) { @@ -226,13 +226,13 @@ } if (selector != HUB_LED_AUTO) changed = 1; - set_port_led(hdev, i + 1, selector); + set_port_led(hub, i + 1, selector); hub->indicator[i] = mode; } if (!changed && blinkenlights) { cursor++; cursor %= hub->descriptor->bNbrPorts; - set_port_led(hdev, cursor + 1, HUB_LED_GREEN); + set_port_led(hub, cursor + 1, HUB_LED_GREEN); hub->indicator[cursor] = INDICATOR_CYCLE; changed++; } @@ -451,7 +451,7 @@ schedule_delayed_work(&hub->leds, LED_CYCLE_PERIOD); /* scan all ports ASAP */ - hub->event_bits[0] = ~0; + hub->event_bits[0] = (1UL << (hub->descriptor->bNbrPorts + 1)) - 1; kick_khubd(hub); } @@ -674,7 +674,7 @@ hub->indicator [0] = INDICATOR_CYCLE; hub_power_on(hub); - hub->change_bits[0] = ~0; + hub->change_bits[0] = (1UL << (hub->descriptor->bNbrPorts)) - 1; hub_activate(hub); return 0; @@ -802,10 +802,9 @@ switch (code) { case USBDEVFS_HUB_PORTINFO: { struct usbdevfs_hub_portinfo *info = user_data; - unsigned long flags; int i; - spin_lock_irqsave(&hub_event_lock, flags); + spin_lock_irq(&device_state_lock); if (hdev->devnum <= 0) info->nports = 0; else { @@ -818,7 +817,7 @@ hdev->children[i]->devnum; } } - spin_unlock_irqrestore(&hub_event_lock, flags); + spin_unlock_irq(&device_state_lock); return info->nports + 1; } @@ -829,9 +828,9 @@ } /* caller has locked the hub device */ -static void hub_pre_reset(struct usb_device *hdev) +static void hub_pre_reset(struct usb_hub *hub) { - struct usb_hub *hub = usb_get_intfdata(hdev->actconfig->interface[0]); + struct usb_device *hdev = hub->hdev; int i; for (i = 0; i < hdev->maxchild; ++i) { @@ -842,10 +841,8 @@ } /* caller has locked the hub device */ -static void hub_post_reset(struct usb_device *hdev) +static void hub_post_reset(struct usb_hub *hub) { - struct usb_hub *hub = usb_get_intfdata(hdev->actconfig->interface[0]); - hub_activate(hub); hub_power_on(hub); } @@ -1275,16 +1272,12 @@ } -static int hub_port_status(struct usb_device *hdev, int port, +static int hub_port_status(struct usb_hub *hub, int port, u16 *status, u16 *change) { - struct usb_hub *hub = usb_get_intfdata(hdev->actconfig->interface[0]); int ret; - if (!hub) - return -ENODEV; - - ret = get_port_status(hdev, port + 1, &hub->status->port); + ret = get_port_status(hub->hdev, port + 1, &hub->status->port); if (ret < 0) dev_err (&hub->intf->dev, "%s failed (err = %d)\n", __FUNCTION__, ret); @@ -1307,7 +1300,7 @@ #define HUB_LONG_RESET_TIME 200 #define HUB_RESET_TIMEOUT 500 -static int hub_port_wait_reset(struct usb_device *hdev, int port, +static int hub_port_wait_reset(struct usb_hub *hub, int port, struct usb_device *udev, unsigned int delay) { int delay_time, ret; @@ -1321,7 +1314,7 @@ msleep(delay); /* read and decode port status */ - ret = hub_port_status(hdev, port, &portstatus, &portchange); + ret = hub_port_status(hub, port, &portstatus, &portchange); if (ret < 0) return ret; @@ -1349,7 +1342,7 @@ if (delay_time >= 2 * HUB_SHORT_RESET_TIME) delay = HUB_LONG_RESET_TIME; - dev_dbg (hubdev (hdev), + dev_dbg (&hub->intf->dev, "port %d not reset yet, waiting %dms\n", port + 1, delay); } @@ -1357,27 +1350,28 @@ return -EBUSY; } -static int hub_port_reset(struct usb_device *hdev, int port, +static int hub_port_reset(struct usb_hub *hub, int port, struct usb_device *udev, unsigned int delay) { int i, status; - struct device *hub_dev = hubdev (hdev); + struct device *hub_dev = &hub->intf->dev; /* Reset the port */ for (i = 0; i < PORT_RESET_TRIES; i++) { - status = set_port_feature(hdev, port + 1, USB_PORT_FEAT_RESET); + status = set_port_feature(hub->hdev, + port + 1, USB_PORT_FEAT_RESET); if (status) dev_err(hub_dev, "cannot reset port %d (err = %d)\n", port + 1, status); else - status = hub_port_wait_reset(hdev, port, udev, delay); + status = hub_port_wait_reset(hub, port, udev, delay); /* return on disconnect or reset */ switch (status) { case 0: case -ENOTCONN: case -ENODEV: - clear_port_feature(hdev, + clear_port_feature(hub->hdev, port + 1, USB_PORT_FEAT_C_RESET); /* FIXME need disconnect() for NOTATTACHED device */ usb_set_device_state(udev, status @@ -1399,8 +1393,9 @@ return status; } -static int hub_port_disable(struct usb_device *hdev, int port) +static int hub_port_disable(struct usb_hub *hub, int port) { + struct usb_device *hdev = hub->hdev; int ret; if (hdev->children[port]) { @@ -1409,7 +1404,7 @@ } ret = clear_port_feature(hdev, port + 1, USB_PORT_FEAT_ENABLE); if (ret) - dev_err(hubdev(hdev), "cannot disable port %d (err = %d)\n", + dev_err(&hub->intf->dev, "cannot disable port %d (err = %d)\n", port + 1, ret); return ret; @@ -1420,12 +1415,10 @@ * time later khubd will disconnect() any existing usb_device on the port * and will re-enumerate if there actually is a device attached. */ -static void hub_port_logical_disconnect(struct usb_device *hdev, int port) +static void hub_port_logical_disconnect(struct usb_hub *hub, int port) { - struct usb_hub *hub; - - dev_dbg(hubdev(hdev), "logical disconnect on port %d\n", port + 1); - hub_port_disable(hdev, port); + dev_dbg(&hub->intf->dev, "logical disconnect on port %d\n", port + 1); + hub_port_disable(hub, port); /* FIXME let caller ask to power down the port: * - some devices won't enumerate without a VBUS power cycle @@ -1436,7 +1429,6 @@ * Powerdown must be optional, because of reset/DFU. */ - hub = usb_get_intfdata(hdev->actconfig->interface[0]); set_bit(port, hub->change_bits); kick_khubd(hub); } @@ -1454,13 +1446,12 @@ * tree above them to deliver data, such as a keypress or packet. In * some cases, this wakes the USB host. */ -static int hub_port_suspend(struct usb_device *hdev, int port) +static int hub_port_suspend(struct usb_hub *hub, int port, + struct usb_device *udev) { int status; - struct usb_device *udev; - udev = hdev->children[port]; - // dev_dbg(hubdev(hdev), "suspend port %d\n", port + 1); + // dev_dbg(&hub->intf->dev, "suspend port %d\n", port + 1); /* enable remote wakeup when appropriate; this lets the device * wake up the upstream hub (including maybe the root hub). @@ -1485,9 +1476,9 @@ } /* see 7.1.7.6 */ - status = set_port_feature(hdev, port + 1, USB_PORT_FEAT_SUSPEND); + status = set_port_feature(hub->hdev, port + 1, USB_PORT_FEAT_SUSPEND); if (status) { - dev_dbg(hubdev(hdev), + dev_dbg(&hub->intf->dev, "can't suspend port %d, status %d\n", port + 1, status); /* paranoia: "should not happen" */ @@ -1499,7 +1490,7 @@ } else { /* device has up to 10 msec to fully suspend */ dev_dbg(&udev->dev, "usb suspend\n"); - udev->state = USB_STATE_SUSPENDED; + usb_set_device_state(udev, USB_STATE_SUSPENDED); msleep(10); } return status; @@ -1606,8 +1597,11 @@ } else status = -EOPNOTSUPP; } else - status = hub_port_suspend(udev->parent, port); + status = hub_port_suspend(hdev_to_hub(udev->parent), port, + udev); + if (status == 0) + udev->dev.power.power_state = state; return status; } EXPORT_SYMBOL(__usb_suspend_device); @@ -1661,9 +1655,10 @@ * first two on the host side; they'd be inside hub_port_init() * during many timeouts, but khubd can't suspend until later. */ - udev->state = udev->actconfig - ? USB_STATE_CONFIGURED - : USB_STATE_ADDRESS; + usb_set_device_state(udev, udev->actconfig + ? USB_STATE_CONFIGURED + : USB_STATE_ADDRESS); + udev->dev.power.power_state = PM_SUSPEND_ON; /* 10.5.4.5 says be sure devices in the tree are still there. * For now let's assume the device didn't go crazy on resume, @@ -1731,18 +1726,17 @@ } static int -hub_port_resume(struct usb_device *hdev, int port) +hub_port_resume(struct usb_hub *hub, int port, struct usb_device *udev) { int status; - struct usb_device *udev; - udev = hdev->children[port]; - // dev_dbg(hubdev(hdev), "resume port %d\n", port + 1); + // dev_dbg(&hub->intf->dev, "resume port %d\n", port + 1); /* see 7.1.7.7; affects power usage, but not budgeting */ - status = clear_port_feature(hdev, port + 1, USB_PORT_FEAT_SUSPEND); + status = clear_port_feature(hub->hdev, + port + 1, USB_PORT_FEAT_SUSPEND); if (status) { - dev_dbg(&hdev->actconfig->interface[0]->dev, + dev_dbg(&hub->intf->dev, "can't resume port %d, status %d\n", port + 1, status); } else { @@ -1750,7 +1744,8 @@ u16 portchange; /* drive resume for at least 20 msec */ - dev_dbg(&udev->dev, "RESUME\n"); + if (udev) + dev_dbg(&udev->dev, "RESUME\n"); msleep(25); #define LIVE_FLAGS ( USB_PORT_STAT_POWER \ @@ -1762,23 +1757,24 @@ * sequence. */ devstatus = portchange = 0; - status = hub_port_status(hdev, port, + status = hub_port_status(hub, port, &devstatus, &portchange); if (status < 0 || (devstatus & LIVE_FLAGS) != LIVE_FLAGS || (devstatus & USB_PORT_STAT_SUSPEND) != 0 ) { - dev_dbg(&hdev->actconfig->interface[0]->dev, + dev_dbg(&hub->intf->dev, "port %d status %04x.%04x after resume, %d\n", port + 1, portchange, devstatus, status); } else { /* TRSMRCY = 10 msec */ msleep(10); - status = finish_port_resume(udev); + if (udev) + status = finish_port_resume(udev); } } if (status < 0) - hub_port_logical_disconnect(hdev, port); + hub_port_logical_disconnect(hub, port); return status; } @@ -1824,7 +1820,8 @@ } } else if (udev->state == USB_STATE_SUSPENDED) { // NOTE this fails if parent is also suspended... - status = hub_port_resume(udev->parent, port); + status = hub_port_resume(hdev_to_hub(udev->parent), + port, udev); } else { status = 0; } @@ -1906,7 +1903,7 @@ u16 portstat, portchange; udev = hdev->children [port]; - status = hub_port_status(hdev, port, &portstat, &portchange); + status = hub_port_status(hub, port, &portstat, &portchange); if (status == 0) { if (portchange & USB_PORT_STAT_C_SUSPEND) { clear_port_feature(hdev, port + 1, @@ -1923,13 +1920,13 @@ continue; down (&udev->serialize); if (portstat & USB_PORT_STAT_SUSPEND) - status = hub_port_resume(hdev, port); + status = hub_port_resume(hub, port, udev); else { status = finish_port_resume(udev); if (status < 0) { dev_dbg(&intf->dev, "resume port %d --> %d\n", port + 1, status); - hub_port_logical_disconnect(hdev, port); + hub_port_logical_disconnect(hub, port); } } up(&udev->serialize); @@ -1983,7 +1980,7 @@ #define HUB_DEBOUNCE_STEP 25 #define HUB_DEBOUNCE_STABLE 100 -static int hub_port_debounce(struct usb_device *hdev, int port) +static int hub_port_debounce(struct usb_hub *hub, int port) { int ret; int total_time, stable_time = 0; @@ -1991,7 +1988,7 @@ unsigned connection = 0xffff; for (total_time = 0; ; total_time += HUB_DEBOUNCE_STEP) { - ret = hub_port_status(hdev, port, &portstatus, &portchange); + ret = hub_port_status(hub, port, &portstatus, &portchange); if (ret < 0) return ret; @@ -2006,7 +2003,7 @@ } if (portchange & USB_PORT_STAT_C_CONNECTION) { - clear_port_feature(hdev, port+1, + clear_port_feature(hub->hdev, port+1, USB_PORT_FEAT_C_CONNECTION); } @@ -2015,7 +2012,7 @@ msleep(HUB_DEBOUNCE_STEP); } - dev_dbg (hubdev (hdev), + dev_dbg (&hub->intf->dev, "debounce: port %d: total %dms stable %dms status 0x%x\n", port + 1, total_time, stable_time, portstatus); @@ -2061,11 +2058,12 @@ * pointers, it's not necessary to lock the device. */ static int -hub_port_init (struct usb_device *hdev, struct usb_device *udev, int port, +hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port, int retry_counter) { static DECLARE_MUTEX(usb_address0_sem); + struct usb_device *hdev = hub->hdev; int i, j, retval; unsigned delay = HUB_SHORT_RESET_TIME; enum usb_device_speed oldspeed = udev->speed; @@ -2087,7 +2085,7 @@ down(&usb_address0_sem); /* Reset the device; full speed may morph to high speed */ - retval = hub_port_reset(hdev, port, udev, delay); + retval = hub_port_reset(hub, port, udev, delay); if (retval < 0) /* error or disconnect */ goto fail; /* success, speed is known */ @@ -2139,9 +2137,6 @@ udev->ttport = hdev->ttport; } else if (udev->speed != USB_SPEED_HIGH && hdev->speed == USB_SPEED_HIGH) { - struct usb_hub *hub; - - hub = usb_get_intfdata(hdev->actconfig->interface[0]); udev->tt = &hub->tt; udev->ttport = port + 1; } @@ -2185,7 +2180,7 @@ buf->bMaxPacketSize0; kfree(buf); - retval = hub_port_reset(hdev, port, udev, delay); + retval = hub_port_reset(hub, port, udev, delay); if (retval < 0) /* error or disconnect */ goto fail; if (oldspeed != udev->speed) { @@ -2352,7 +2347,7 @@ port + 1, portstatus, portchange, portspeed (portstatus)); if (hub->has_indicators) { - set_port_led(hdev, port + 1, HUB_LED_AUTO); + set_port_led(hub, port + 1, HUB_LED_AUTO); hub->indicator[port] = INDICATOR_AUTO; } @@ -2368,7 +2363,7 @@ #endif if (portchange & USB_PORT_STAT_C_CONNECTION) { - status = hub_port_debounce(hdev, port); + status = hub_port_debounce(hub, port); if (status < 0) { dev_err (hub_dev, "connect-debounce failed, port %d disabled\n", @@ -2393,11 +2388,13 @@ } #ifdef CONFIG_USB_SUSPEND - /* If something is connected, but the port is suspended, wake it up.. */ + /* If something is connected, but the port is suspended, wake it up. */ if (portstatus & USB_PORT_STAT_SUSPEND) { - status = hub_port_resume(hdev, port); + status = hub_port_resume(hub, port, NULL); if (status < 0) - dev_dbg(hub_dev, "can't clear suspend on port %d; %d\n", port+1, status); + dev_dbg(hub_dev, + "can't clear suspend on port %d; %d\n", + port+1, status); } #endif @@ -2425,7 +2422,7 @@ } /* reset and get descriptor */ - status = hub_port_init(hdev, udev, port, i); + status = hub_port_init(hub, udev, port, i); if (status < 0) goto loop; @@ -2507,7 +2504,7 @@ return; loop: - hub_port_disable(hdev, port); + hub_port_disable(hub, port); usb_disable_endpoint(udev, 0 + USB_DIR_IN); usb_disable_endpoint(udev, 0 + USB_DIR_OUT); release_address(udev); @@ -2517,13 +2514,14 @@ } done: - hub_port_disable(hdev, port); + hub_port_disable(hub, port); } static void hub_events(void) { struct list_head *tmp; struct usb_device *hdev; + struct usb_interface *intf; struct usb_hub *hub; struct device *hub_dev; u16 hubstatus; @@ -2553,10 +2551,8 @@ hub = list_entry(tmp, struct usb_hub, event_list); hdev = hub->hdev; - hub_dev = &hub->intf->dev; - - usb_get_dev(hdev); - spin_unlock_irq(&hub_event_lock); + intf = hub->intf; + hub_dev = &intf->dev; dev_dbg(hub_dev, "state %d ports %d chg %04x evt %04x\n", hdev->state, hub->descriptor @@ -2566,14 +2562,16 @@ (u16) hub->change_bits[0], (u16) hub->event_bits[0]); + usb_get_intf(intf); + spin_unlock_irq(&hub_event_lock); + /* Lock the device, then check to see if we were * disconnected while waiting for the lock to succeed. */ - if (locktree(hdev) < 0) - break; - if (hdev->state != USB_STATE_CONFIGURED || - !hdev->actconfig || - hub != usb_get_intfdata( - hdev->actconfig->interface[0])) + if (locktree(hdev) < 0) { + usb_put_intf(intf); + continue; + } + if (hub != usb_get_intfdata(intf) || hub->quiescing) goto loop; if (hub->error) { @@ -2598,7 +2596,7 @@ !connect_change) continue; - ret = hub_port_status(hdev, i, + ret = hub_port_status(hub, i, &portstatus, &portchange); if (ret < 0) continue; @@ -2645,7 +2643,7 @@ connect_change = 1; } else { ret = -ENODEV; - hub_port_disable(hdev, i); + hub_port_disable(hub, i); } dev_dbg (hub_dev, "resume on port %d, status %d\n", @@ -2694,7 +2692,7 @@ loop: usb_unlock_device(hdev); - usb_put_dev(hdev); + usb_put_intf(intf); } /* end while (1) */ } @@ -2851,10 +2849,11 @@ */ int usb_reset_device(struct usb_device *udev) { - struct usb_device *parent = udev->parent; - struct usb_device_descriptor descriptor = udev->descriptor; - int i, ret = 0, port = -1; - int udev_is_a_hub = 0; + 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, port = -1; if (udev->state == USB_STATE_NOTATTACHED || udev->state == USB_STATE_SUSPENDED) { @@ -2863,14 +2862,14 @@ return -EINVAL; } - if (!parent) { + if (!parent_hdev) { /* this requires hcd-specific logic; see OHCI hc_restart() */ dev_dbg(&udev->dev, "%s for root hub!\n", __FUNCTION__); return -EISDIR; } - for (i = 0; i < parent->maxchild; i++) - if (parent->children[i] == udev) { + for (i = 0; i < parent_hdev->maxchild; i++) + if (parent_hdev->children[i] == udev) { port = i; break; } @@ -2880,13 +2879,14 @@ dev_err(&udev->dev, "Can't locate device's port!\n"); return -ENOENT; } + parent_hub = hdev_to_hub(parent_hdev); /* If we're resetting an active hub, take some special actions */ if (udev->actconfig && udev->actconfig->interface[0]->dev.driver == - &hub_driver.driver) { - udev_is_a_hub = 1; - hub_pre_reset(udev); + &hub_driver.driver && + (hub = hdev_to_hub(udev)) != NULL) { + hub_pre_reset(hub); } for (i = 0; i < SET_CONFIG_TRIES; ++i) { @@ -2895,7 +2895,7 @@ * Other endpoints will be handled by re-enumeration. */ usb_disable_endpoint(udev, 0 + USB_DIR_IN); usb_disable_endpoint(udev, 0 + USB_DIR_OUT); - ret = hub_port_init(parent, udev, port, i); + ret = hub_port_init(parent_hub, udev, port, i); if (ret >= 0) break; } @@ -2946,11 +2946,11 @@ } done: - if (udev_is_a_hub) - hub_post_reset(udev); + if (hub) + hub_post_reset(hub); return 0; re_enumerate: - hub_port_logical_disconnect(parent, port); + hub_port_logical_disconnect(parent_hub, port); return -ENODEV; }
Add support for autosuspend to the hub driver. The inactivity timeout is specified as a module parameter. --- a/drivers/usb/core/hub.h Fri Oct 15 16:11:12 2004 +++ b/drivers/usb/core/hub.h Wed Nov 24 16:26:15 2004 @@ -11,6 +11,7 @@ #include <linux/list.h> #include <linux/workqueue.h> #include <linux/compiler.h> /* likely()/unlikely() */ +#include <linux/timer.h> /* * Hub request types @@ -219,6 +220,12 @@ unsigned has_indicators:1; enum hub_led_mode indicator[USB_MAXCHILDREN]; struct work_struct leds; + +#ifdef CONFIG_USB_SUSPEND + int unsuspended_children_count; + unsigned long last_event_time; + struct timer_list autosuspend_timer; +#endif }; /* use this for low-powered root hubs */ --- a/drivers/usb/core/hub.c Thu Nov 25 22:58:39 2004 +++ b/drivers/usb/core/hub.c Thu Nov 25 22:58:40 2004 @@ -81,6 +81,13 @@ "try the other device initialization scheme if the " "first one fails"); +#ifdef CONFIG_USB_SUSPEND +static unsigned int hub_autosuspend = 2; +module_param(hub_autosuspend, uint, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(hub_autosuspend, + "timeout in seconds for autosuspending an inactive hub " + "(0 for no autosuspend)"); +#endif #ifdef DEBUG static inline char *portspeed (int portstatus) @@ -321,6 +328,104 @@ dev_err (&hub->intf->dev, "resubmit --> %d\n", status); } + +#ifdef CONFIG_USB_SUSPEND + +/* Return 1 if it's time to autosuspend; otherwise set the timer if needed */ +static int autosuspend_update(struct usb_hub *hub) +{ + unsigned long timeout; + + if (hub->unsuspended_children_count || + hub_autosuspend == 0 || + !list_empty(&hub->event_list) || + // !hub->hdev->dev.power.wakeup_enabled || + hub->quiescing) + return 0; + timeout = hub->last_event_time + hub_autosuspend * HZ; + if (time_before(jiffies, timeout)) { + mod_timer(&hub->autosuspend_timer, timeout); + return 0; + } + return 1; +} + +/* An autosuspend event occurred. Update the active-children count + * and restart the timer. The hub device _should_ be locked. */ +static void autosuspend_event(struct usb_hub *hub, int children_change) +{ + hub->unsuspended_children_count += children_change; + hub->last_event_time = jiffies; + autosuspend_update(hub); +} + +/* It may be time to autosuspend. Do it if everything is right. + * Must be in process context (able to sleep) with hub unlocked. */ +static void autosuspend_try(struct usb_device *hdev, + struct usb_interface *intf, struct usb_hub *hub) { + int port; + static int locktree(struct usb_device *); + static int __usb_suspend_device(struct usb_device *, + int port, u32 state); + + // Lock hdev's parent (?) and hdev itself + port = locktree(hdev); + if (port < 0) + return; + + /* Don't try to do anything if we were disconnected */ + if (hub != usb_get_intfdata(intf)) + ; + else if (autosuspend_update(hub) + && hdev->parent ) { + dev_dbg(&hdev->dev, "hub autosuspend\n"); + __usb_suspend_device(hdev, port, PM_SUSPEND_MEM); + } + + // Unlock hdev and parent + usb_unlock_device(hdev); +} + +/* It may be time to autosuspend. Tell khubd if we're ready. */ +static void autosuspend_timer_func(unsigned long __hub) +{ + struct usb_hub *hub = (struct usb_hub *) __hub; + + if (autosuspend_update(hub)) + kick_khubd(hub); +} + +/* The hub has been reset, so the children_count needs to be zeroed */ +static inline void autosuspend_reset(struct usb_hub *hub) +{ + hub->unsuspended_children_count = 0; +} + +static inline void autosuspend_activate(struct usb_hub *hub) +{ + init_timer(&hub->autosuspend_timer); + hub->autosuspend_timer.function = autosuspend_timer_func; + hub->autosuspend_timer.data = (unsigned long) hub; +} + +static inline void autosuspend_quiesce(struct usb_hub *hub) +{ + del_timer_sync(&hub->autosuspend_timer); +} + +#else /* CONFIG_USB_SUSPEND */ + +static int autosuspend_update(struct usb_hub *hub) { return 0; } +static void autosuspend_event(struct usb_hub *hub, int children_change) {} +static void autosuspend_try(struct usb_device *hdev, + struct usb_interface *iface, struct usb_hub *hub) {} +static inline void autosuspend_reset(struct usb_hub *hub) {} +static inline void autosuspend_activate(struct usb_hub *hub) {} +static inline void autosuspend_quiesce(struct usb_hub *hub) {} + +#endif + + /* USB 2.0 spec Section 11.24.2.3 */ static inline int hub_clear_tt_buffer (struct usb_device *hdev, u16 devinfo, u16 tt) @@ -437,6 +542,7 @@ cancel_delayed_work(&hub->leds); if (hub->has_indicators || hub->tt.hub) flush_scheduled_work(); + autosuspend_quiesce(hub); } static void hub_activate(struct usb_hub *hub) @@ -444,6 +550,7 @@ int status; hub->quiescing = 0; + autosuspend_activate(hub); status = usb_submit_urb(hub->urb, GFP_NOIO); if (status < 0) dev_err(&hub->intf->dev, "activate --> %d\n", status); @@ -837,6 +944,7 @@ if (hdev->children[i]) usb_disconnect(&hdev->children[i]); } + autosuspend_reset(hub); hub_quiesce(hub); } @@ -1491,6 +1599,7 @@ /* device has up to 10 msec to fully suspend */ dev_dbg(&udev->dev, "usb suspend\n"); usb_set_device_state(udev, USB_STATE_SUSPENDED); + autosuspend_event(hub, -1); msleep(10); } return status; @@ -1655,6 +1764,8 @@ * first two on the host side; they'd be inside hub_port_init() * during many timeouts, but khubd can't suspend until later. */ + if (udev->state == USB_STATE_SUSPENDED) + autosuspend_event(hdev_to_hub(udev->parent), +1); usb_set_device_state(udev, udev->actconfig ? USB_STATE_CONFIGURED : USB_STATE_ADDRESS); @@ -1773,8 +1884,11 @@ status = finish_port_resume(udev); } } - if (status < 0) + if (status < 0) { + if (udev && udev->state == USB_STATE_SUSPENDED) + autosuspend_event(hub, +1); hub_port_logical_disconnect(hub, port); + } return status; } @@ -1926,6 +2040,8 @@ if (status < 0) { dev_dbg(&intf->dev, "resume port %d --> %d\n", port + 1, status); + if (udev->state == USB_STATE_SUSPENDED) + autosuspend_event(hub, +1); hub_port_logical_disconnect(hub, port); } } @@ -2352,8 +2468,11 @@ } /* Disconnect any existing devices under this port */ - if (hdev->children[port]) + if (hdev->children[port]) { + if (hdev->children[port]->state != USB_STATE_SUSPENDED) + autosuspend_event(hub, -1); usb_disconnect(&hdev->children[port]); + } clear_bit(port, hub->change_bits); #ifdef CONFIG_USB_OTG @@ -2495,6 +2614,8 @@ if (status) goto loop; + autosuspend_event(hub, +1); + status = hub_power_remaining(hub); if (status) dev_dbg(hub_dev, @@ -2530,6 +2651,7 @@ u16 portchange; int i, ret; int connect_change; + int any_events; /* * We restart the list every time to avoid a deadlock with @@ -2564,6 +2686,7 @@ usb_get_intf(intf); spin_unlock_irq(&hub_event_lock); + any_events = 0; /* Lock the device, then check to see if we were * disconnected while waiting for the lock to succeed. */ @@ -2587,6 +2710,7 @@ hub->nerrors = 0; hub->error = 0; + any_events = 1; } /* deal with port status changes */ @@ -2595,6 +2719,7 @@ if (!test_and_clear_bit(i+1, hub->event_bits) && !connect_change) continue; + any_events = 1; ret = hub_port_status(hub, i, &portstatus, &portchange); @@ -2678,6 +2803,7 @@ else if (hub_hub_status(hub, &hubstatus, &hubchange) < 0) dev_err (hub_dev, "get_hub_status failed\n"); else { + any_events = 1; if (hubchange & HUB_CHANGE_LOCAL_POWER) { dev_dbg (hub_dev, "power change\n"); clear_hub_feature(hdev, C_HUB_LOCAL_POWER); @@ -2691,7 +2817,14 @@ } loop: + if (any_events) { + autosuspend_event(hub, 0); + i = 0; + } else + i = autosuspend_update(hub); usb_unlock_device(hdev); + if (i) + autosuspend_try(hdev, intf, hub); usb_put_intf(intf); } /* end while (1) */