Oliver and Sarah:
Thinking about the xHCI remote-wakeup issues made me realize that we
still have a race in usbcore. What happens if a wakeup event occurs on
a root hub at about the same time as the root hub is suspended?
(A) If the event happens after the controller device is
suspended, the controller will signal a wakeup request (PCI
PME# or GPIO interrupt or something similar). The system will
wake up (if it has already gone to sleep) or the system sleep
transition will be aborted or the controller will autoresume.
This case is okay.
(B) If the event happens before the controller device is
suspended but after the root hub is suspended, there will be a
root-hub wakeup interrupt. The HCD's handler will call
usb_hcd_resume_root_hub(), and again the sleep transition will
be aborted or the root hub will autoresume. Again we're okay.
(C) If the event happens before the root hub is suspended but
after the hub driver's suspend routine has run, the root-hub
interrupt URB will have been unlinked. While processing the
status-change interrupt, the HCD will clear the hardware's
interrupt request bit and call usb_hcd_poll_rh_status(), which
will set the HCD_FLAG_POLL_PENDING bit but not do anything
else. The event won't be processed until the hub driver is
resumed -- it won't abort anything or cause an autoresume.
This case is not okay.
(D) If the event happens before the hub driver's suspend
routine runs but after khubd is frozen, the root-hub interrupt
URB will complete and hub_irq() will call kick_khubd(). But
since khubd is frozen, nothing more will happen. Again we're
not okay.
(E) If the event happens before khubd is frozen then
kick_khubd() will wake khubd. The khubd thread checks for
freeze requests only when its event list is empty, so the event
will be processed before khubd is frozen, and this case is
okay.
The patch below is intended to handle cases (C) and (D). I haven't
tested it. Does it seem all right to you? It uses an arbitrary 100-ms
delay for case (D), on the basis that suspending and resuming hubs is a
fairly slow process.
Alan Stern
Index: usb-3.3/drivers/usb/core/hcd.c
===================================================================
--- usb-3.3.orig/drivers/usb/core/hcd.c
+++ usb-3.3/drivers/usb/core/hcd.c
@@ -1978,6 +1978,11 @@ int hcd_bus_suspend(struct usb_device *r
if (status == 0) {
usb_set_device_state(rhdev, USB_STATE_SUSPENDED);
hcd->state = HC_STATE_SUSPENDED;
+
+ /* Did we race with a root-hub wakeup event? */
+ if (unlikely(HCD_POLL_PENDING(hcd) &&
+ rhdev->do_remote_wakeup))
+ usb_hcd_resume_root_hub(hcd);
} else {
spin_lock_irq(&hcd_root_hub_lock);
if (!HCD_DEAD(hcd)) {
Index: usb-3.3/drivers/usb/core/hub.c
===================================================================
--- usb-3.3.orig/drivers/usb/core/hub.c
+++ usb-3.3/drivers/usb/core/hub.c
@@ -399,6 +399,11 @@ static void kick_khubd(struct usb_hub *h
usb_autopm_get_interface_no_resume(
to_usb_interface(hub->intfdev));
wake_up(&khubd_wait);
+
+ /* Did a wakeup event arrive while khubd was frozen? */
+ if (unlikely(frozen(khubd_task) &&
+ device_may_wakeup(&hub->hdev->dev)))
+ pm_wakeup_event(&hub->hdev->dev, 100);
}
spin_unlock_irqrestore(&hub_event_lock, flags);
}
--
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
