RE: usbcore and root-hub suspend/wakeup races

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 
> 
> 	(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);
At hcd->driver->bus_suspend, it may put PHY to low power mode and close the related code.
Then, the resume signal(or SOF if resume has finished) from the host will be stopped, 
it will confuse the device, we may need to move your code before the hcd->driver->bus_suspend,
and only resume hub driver.

Another corner case is replace system suspend with auto-suspend at your cases,
at that situation, we may need to return -EBUSY for hcd_bus_suspend, and let the
system pm routine to resume hub driver, and others.

I don't know how to distinguish auto-suspend/and system suspend, do you have better
solution to cover these two cases?

>  	} 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


--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux