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 implemenation 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 port rpm_resume use runtime-pm-synchronization to guarantee the same sequence of events. When a usb_wakeup_notification() is invoked the 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, holds the port active while it resumes the child, and completes child device resume before acting on the current portstatus. Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/usb/core/hub.c | 40 +++++++++++++++++++++++++++++----------- drivers/usb/core/port.c | 2 ++ 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 194ff3303ee9..6c666796fd21 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -606,9 +606,11 @@ void usb_kick_khubd(struct usb_device *hdev) * USB 3.0 hubs do not report the port link state change from U3 to U0 when the * device initiates resume, so the USB core will not receive notice of the * resume through the normal hub interrupt URB. + * + * This is also called by usb_port_runtime_resume() to arrange for the child + * device to be woken up as part of the power session recovery for the port. */ -void usb_wakeup_notification(struct usb_device *hdev, - unsigned int portnum) +void usb_wakeup_notification(struct usb_device *hdev, unsigned int port1) { struct usb_hub *hub; @@ -617,7 +619,11 @@ void usb_wakeup_notification(struct usb_device *hdev, hub = usb_hub_to_struct_hub(hdev); if (hub) { - set_bit(portnum, hub->wakeup_bits); + struct usb_port *port_dev = hub->ports[port1 - 1]; + + dev_dbg(&port_dev->dev, "%s\n", __func__); + if (!test_and_set_bit(port1, hub->wakeup_bits)) + pm_runtime_get(&port_dev->dev); kick_khubd(hub); } } @@ -4677,28 +4683,33 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1, /* Returns 1 if there was a remote wakeup and a connect status change. */ static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port, - u16 portstatus, u16 portchange) + u16 portstatus, u16 portchange, int wakeup_change) __must_hold(&port_dev->status_lock) { struct usb_port *port_dev = hub->ports[port - 1]; + int connect_change = 0, do_wakeup = 1; struct usb_device *hdev; struct usb_device *udev; - int connect_change = 0; int ret; hdev = hub->hdev; udev = port_dev->child; if (!hub_is_superspeed(hdev)) { if (!(portchange & USB_PORT_STAT_C_SUSPEND)) - return 0; + do_wakeup = 0; usb_clear_port_feature(hdev, port, USB_PORT_FEAT_C_SUSPEND); } else { if (!udev || udev->state != USB_STATE_SUSPENDED || (portstatus & USB_PORT_STAT_LINK_STATE) != USB_SS_PORT_LS_U0) - return 0; + do_wakeup = 0; } + if (wakeup_change || do_wakeup) + /* pass */; + else + return 0; + if (udev) { /* TRSMRCY = 10 msec */ msleep(10); @@ -4726,7 +4737,11 @@ static void port_event(struct usb_hub *hub, int port1) u16 portstatus, portchange; connect_change = test_bit(port1, hub->change_bits); - wakeup_change = test_and_clear_bit(port1, hub->wakeup_bits); + if (test_and_clear_bit(port1, hub->wakeup_bits)) { + /* balance usb_wakeup_notification(), we won't suspend here */ + pm_runtime_put(&port_dev->dev); + wakeup_change = 1; + } if (hub_port_status(hub, port1, &portstatus, &portchange) < 0) return; @@ -4790,9 +4805,12 @@ static void port_event(struct usb_hub *hub, int port1) /* take port actions that require the port to be powered on */ if (pm_runtime_active(&port_dev->dev)) { - if (hub_handle_remote_wakeup(hub, port1, - portstatus, portchange)) - connect_change = 1; + /* + * There will be a wakeup pending if the device signalled it, + * or if we are re-powering a port + */ + connect_change |= hub_handle_remote_wakeup(hub, port1, + portstatus, portchange, wakeup_change); /* * Warm reset a USB3 protocol port if it's in diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index db32315a8cd8..d01499a33bd7 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -104,6 +104,8 @@ static int usb_port_runtime_resume(struct device *dev) if (retval < 0) dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", retval); + + usb_wakeup_notification(hdev, port1); retval = 0; } -- 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
