Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usb: resume child device when port is powered on

Unconditionally wake up the child device when the power session is
recovered.

This addresses the following scenarios:

1/ The device may need a reset on power-session loss, without this
change port power-on recovery exposes khubd to scenarios that
usb_port_resume() is set to handle. Prior to port power control the
only time a power session would be lost is during dpm_suspend of the
hub. In that scenario usb_port_resume() is guaranteed to be called
prior to khubd running for that port. With this change we wakeup the
child device as soon as possible (prior to khubd running again for this
port).

Although khubd has facilities to wake a child device it will only do
so if the portstatus / portchange indicates a suspend state. In the
case of port power control we are not coming from a hub-port-suspend
state. This implementation simply uses pm_request_resume() to wake the
device and relies on the port_dev->status_lock to prevent any collisions
between khubd and usb_port_resume().

2/ This mechanism rate limits port power toggling. The minimum port
power on/off period is now gated by the child device suspend/resume
latency. Empirically this mitigates devices downgrading their connection
on perceived instability of the host connection. This ratelimiting is
really only relevant to port power control testing, but it is a nice
side effect of closing the above race. Namely, the race of khubd for
the given port running while a usb_port_resume() event is pending.

3/ Going forward we are finding that power-session recovery requires
warm-resets (http://marc.info/?t=138659232900003&r=1&w=2). This
mechanism allows for warm-resets to be requested at the same point in
the resume path for hub dpm_suspend power session losses, or port
rpm_suspend power session losses.

4/ If the device *was* disconnected the only time we'll know for sure is
after a failed resume, so it's necessary for usb_port_runtime_resume()
to expedite a usb_port_resume() to clean up the removed device. The
reasoning for this is "least surprise" for the user. Turning on a port
means that hotplug detection is again enabled for the port, it is
surprising that devices that were removed while the port was off are not
disconnected until they are attempted to be used. As a user "why would
I try to use a device I removed from the system?"

1, 2, and 4 are not a problem in the system dpm_resume() case because,
although the power-session is lost, khubd is frozen until after device
resume. For the rpm_resume() case pm_request_resume() is used to
request re-validation of the device, and if it happens to collide with a
khubd run we rely on the port_dev->status_lock to synchronize those
operations.

Besides testing, the primary scenario where this mechanism is expected
to be triggered is when the user changes the port power policy
(control/pm_qos_no_poweroff, or power/control). Each time power is
enabled want to revalidate the child device, where the revalidation is
handled by usb_port_resume().

Given that this arranges for port_dev->child to be de-referenced in
usb_port_runtime_resume() we need to make sure not to collide with
usb_disconnect() that frees the usb_device. To this end we hold the
port active with the "child_usage" reference across the disconnect
event. Subsequently, the need to access hub->child_usage_bits lead to
the creation of hub_disconnect_children() to remove any ambiguity of
which "hub" is being acted on in usb_disconnect() (prompted-by sharp
eyes from Alan).

Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Dan Williams and committed by
Greg Kroah-Hartman
7027df36 7e73be22

+37 -14
+29 -13
drivers/usb/core/hub.c
··· 2039 2039 hcd->driver->free_dev(hcd, udev); 2040 2040 } 2041 2041 2042 + static void hub_disconnect_children(struct usb_device *udev) 2043 + { 2044 + struct usb_hub *hub = usb_hub_to_struct_hub(udev); 2045 + int i; 2046 + 2047 + /* Free up all the children before we remove this device */ 2048 + for (i = 0; i < udev->maxchild; i++) { 2049 + if (hub->ports[i]->child) 2050 + usb_disconnect(&hub->ports[i]->child); 2051 + } 2052 + } 2053 + 2042 2054 /** 2043 2055 * usb_disconnect - disconnect a device (usbcore-internal) 2044 2056 * @pdev: pointer to device being disconnected ··· 2069 2057 */ 2070 2058 void usb_disconnect(struct usb_device **pdev) 2071 2059 { 2072 - struct usb_device *udev = *pdev; 2073 - struct usb_hub *hub = usb_hub_to_struct_hub(udev); 2074 - int i; 2060 + struct usb_port *port_dev = NULL; 2061 + struct usb_device *udev = *pdev; 2062 + struct usb_hub *hub; 2063 + int port1; 2075 2064 2076 2065 /* mark the device as inactive, so any further urb submissions for 2077 2066 * this device (and any of its children) will fail immediately. ··· 2084 2071 2085 2072 usb_lock_device(udev); 2086 2073 2087 - /* Free up all the children before we remove this device */ 2088 - for (i = 0; i < udev->maxchild; i++) { 2089 - if (hub->ports[i]->child) 2090 - usb_disconnect(&hub->ports[i]->child); 2091 - } 2074 + hub_disconnect_children(udev); 2092 2075 2093 2076 /* deallocate hcd/hardware state ... nuking all pending urbs and 2094 2077 * cleaning up all state associated with the current configuration ··· 2095 2086 usb_hcd_synchronize_unlinks(udev); 2096 2087 2097 2088 if (udev->parent) { 2098 - int port1 = udev->portnum; 2099 - struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); 2100 - struct usb_port *port_dev = hub->ports[port1 - 1]; 2089 + port1 = udev->portnum; 2090 + hub = usb_hub_to_struct_hub(udev->parent); 2091 + port_dev = hub->ports[port1 - 1]; 2101 2092 2102 2093 sysfs_remove_link(&udev->dev.kobj, "port"); 2103 2094 sysfs_remove_link(&port_dev->dev.kobj, "device"); 2104 2095 2105 - if (test_and_clear_bit(port1, hub->child_usage_bits)) 2106 - pm_runtime_put(&port_dev->dev); 2096 + /* 2097 + * As usb_port_runtime_resume() de-references udev, make 2098 + * sure no resumes occur during removal 2099 + */ 2100 + if (!test_and_set_bit(port1, hub->child_usage_bits)) 2101 + pm_runtime_get_sync(&port_dev->dev); 2107 2102 } 2108 2103 2109 2104 usb_remove_ep_devs(&udev->ep0); ··· 2128 2115 spin_lock_irq(&device_state_lock); 2129 2116 *pdev = NULL; 2130 2117 spin_unlock_irq(&device_state_lock); 2118 + 2119 + if (port_dev && test_and_clear_bit(port1, hub->child_usage_bits)) 2120 + pm_runtime_put(&port_dev->dev); 2131 2121 2132 2122 hub_free_dev(udev); 2133 2123
+8 -1
drivers/usb/core/port.c
··· 76 76 struct usb_device *hdev = to_usb_device(dev->parent->parent); 77 77 struct usb_interface *intf = to_usb_interface(dev->parent); 78 78 struct usb_hub *hub = usb_hub_to_struct_hub(hdev); 79 + struct usb_device *udev = port_dev->child; 79 80 struct usb_port *peer = port_dev->peer; 80 81 int port1 = port_dev->portnum; 81 82 int retval; ··· 98 97 usb_autopm_get_interface(intf); 99 98 retval = usb_hub_set_port_power(hdev, hub, port1, true); 100 99 msleep(hub_power_on_good_delay(hub)); 101 - if (port_dev->child && !retval) { 100 + if (udev && !retval) { 102 101 /* 103 102 * Attempt to wait for usb hub port to be reconnected in order 104 103 * to make the resume procedure successful. The device may have ··· 110 109 dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", 111 110 retval); 112 111 retval = 0; 112 + 113 + /* Force the child awake to revalidate after the power loss. */ 114 + if (!test_and_set_bit(port1, hub->child_usage_bits)) { 115 + pm_runtime_get_noresume(&port_dev->dev); 116 + pm_request_resume(&udev->dev); 117 + } 113 118 } 114 119 115 120 usb_autopm_put_interface(intf);