Unconditionally wake up the child device when the power session is recovered. This address 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 extends usb_wake_notification() for the port rpm_resume case. 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() and runtime-pm-synchronization is used to guarantee the same sequence of events. When pm_request_resume() is invoked on the child usb_device port device is in the RPM_RESUMING state. khubd in turn performs a pm_runtime_barrier() on the port device to flush the port recovery, and holds the port active while it resumes the child with another pm_runtime_barrier(). This guarantees that the portstatus khubd evaluates via port_event() is always on an active port and a usb_device that has recovered from power loss. Besides testing the primary scenario where this mechanism is expected to be triggered is when the user changes the port power policy (pm_qos_no_poweroff). When that is set to 1 we want to revalidate the child device, where the revalidation is handled by usb_port_resume(). Given that this patch de-references port_dev->child we need to make sure not to collide with usb_disconnect(). The hub_port_reconnect() case is handled by the fact that port_event() is run with the port_dev pinned active. Cc: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/usb/core/hub.c | 31 ++++++++++++++++++++++++++----- drivers/usb/core/port.c | 11 ++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 45ded752c2d8..5b855c263d6d 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -2057,9 +2057,10 @@ static void hub_free_dev(struct usb_device *udev) */ void usb_disconnect(struct usb_device **pdev) { - struct usb_device *udev = *pdev; - struct usb_hub *hub = usb_hub_to_struct_hub(udev); - int i; + int i; + struct usb_device *udev = *pdev; + struct usb_port *port_dev = NULL; + struct usb_hub *hub = usb_hub_to_struct_hub(udev); /* mark the device as inactive, so any further urb submissions for * this device (and any of its children) will fail immediately. @@ -2088,13 +2089,18 @@ void usb_disconnect(struct usb_device **pdev) if (udev->parent) { int port1 = udev->portnum; struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); - struct usb_port *port_dev = hub->ports[port1 - 1]; + port_dev = hub->ports[port1 - 1]; sysfs_remove_link(&udev->dev.kobj, "port"); sysfs_remove_link(&port_dev->dev.kobj, "device"); if (test_and_clear_bit(port1, hub->child_usage_bits)) pm_runtime_put(&port_dev->dev); + /* + * As usb_port_runtime_resume() de-references udev, make + * sure no resumes occur during removal + */ + pm_runtime_disable(&port_dev->dev); } usb_remove_ep_devs(&udev->ep0); @@ -2116,6 +2122,9 @@ void usb_disconnect(struct usb_device **pdev) *pdev = NULL; spin_unlock_irq(&device_state_lock); + if (port_dev) + pm_runtime_enable(&port_dev->dev); + hub_free_dev(udev); put_device(&udev->dev); @@ -4754,7 +4763,6 @@ static void port_event(struct usb_hub *hub, int port1) connect_change = test_bit(port1, hub->change_bits); clear_bit(port1, hub->event_bits); - clear_bit(port1, hub->wakeup_bits); if (hub_port_status(hub, port1, &portstatus, &portchange) < 0) return; @@ -4955,6 +4963,8 @@ static void hub_events(void) if (test_bit(i, hub->event_bits) || test_bit(i, hub->change_bits) || test_bit(i, hub->wakeup_bits)) { + struct usb_device *udev = port_dev->child; + /* * The get_noresume and barrier ensure that if * the port was in the process of resuming, we @@ -4966,6 +4976,17 @@ static void hub_events(void) */ pm_runtime_get_noresume(&port_dev->dev); pm_runtime_barrier(&port_dev->dev); + + /* + * Revalidate the device if it was requested by + * usb_port_runtime_resume. + */ + if (test_and_clear_bit(i, hub->wakeup_bits) && udev) { + pm_runtime_get_noresume(&udev->dev); + pm_runtime_barrier(&udev->dev); + pm_runtime_put_autosuspend(&udev->dev); + } + usb_lock_port(port_dev); port_event(hub, i); usb_unlock_port(port_dev); diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 8b1655700104..77e590c20a48 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -76,6 +76,7 @@ static int usb_port_runtime_resume(struct device *dev) struct usb_device *hdev = to_usb_device(dev->parent->parent); struct usb_interface *intf = to_usb_interface(dev->parent); struct usb_hub *hub = usb_hub_to_struct_hub(hdev); + struct usb_device *udev = port_dev->child; struct usb_port *peer = port_dev->peer; int port1 = port_dev->portnum; int retval; @@ -97,7 +98,7 @@ static int usb_port_runtime_resume(struct device *dev) usb_autopm_get_interface(intf); retval = usb_hub_set_port_power(hdev, hub, port1, true); msleep(hub_power_on_good_delay(hub)); - if (port_dev->child && !retval) { + if (udev && !retval) { /* * Attempt to wait for usb hub port to be reconnected in order * to make the resume procedure successful. The device may have @@ -109,6 +110,14 @@ static int usb_port_runtime_resume(struct device *dev) dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", retval); retval = 0; + + /* Force the child awake to revalidate after the power loss. */ + if (!test_and_set_bit(port1, hub->child_usage_bits)) { + pm_runtime_get_noresume(&port_dev->dev); + set_bit(port1, hub->wakeup_bits); + pm_request_resume(&udev->dev); + usb_kick_khubd(hdev); + } } usb_autopm_put_interface(intf); -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
