RE: usbcore and root-hub suspend/wakeup races

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

 



 
 
> 
> > > Here's the revised patch.  Can you test it to make sure it does what
> > > you want?
> > >
> > > Alan Stern
> > >
> > Alan, we have a similar debug yesterday, the system still enters
> > suspend which remote wakeup occurs, and we find it is due to:
> >
> > commit 0af212ba8f123c2eba151af7726c34a50b127962
> > USB: don't let errors prevent system sleep
> >
> > Tony, I know you are working on this issue. please share your results
> > and discuss with Alan about the solution.
> 
> Heh, that's pretty funny.  I was so eager to prevent problems from
> blocking system suspend that now wakeup events can't block it either!
> 
> This patch on top of the other one should help.  Let me know.
> 
> Alan Stern
> 
> 
> Index: usb-3.3/drivers/usb/core/driver.c
> ===================================================================
> --- usb-3.3.orig/drivers/usb/core/driver.c
> +++ usb-3.3/drivers/usb/core/driver.c
> @@ -1190,8 +1190,13 @@ static int usb_suspend_both(struct usb_d
>  	if (status == 0) {
>  		status = usb_suspend_device(udev, msg);
> 
> -		/* Again, ignore errors during system sleep transitions */
> -		if (!PMSG_IS_AUTO(msg))
> +		/*
> +		 * Ignore errors from non-root-hub devices during
> +		 * system sleep transitions.  For the most part,
> +		 * these devices should go to low power anyway when
> +		 * the entire bus is suspended.
> +		 */
> +		if (udev->parent && !PMSG_IS_AUTO(msg))
>  			status = 0;
>  	}
> 
> 
Hi Alan, I got some free time these days, and test your patch.

Index: usb-3.3/drivers/usb/core/driver.c
===================================================================
--- usb-3.3.orig/drivers/usb/core/driver.c
+++ usb-3.3/drivers/usb/core/driver.c
@@ -1190,8 +1190,13 @@ static int usb_suspend_both(struct usb_d
       if (status == 0) {
               status = usb_suspend_device(udev, msg);

-               /* Again, ignore errors during system sleep transitions */
-               if (!PMSG_IS_AUTO(msg))
+               /*
+                * Ignore errors from non-root-hub devices during
+                * system sleep transitions.  For the most part,
+                * these devices should go to low power anyway when
+                * the entire bus is suspended.
+                */
+               if (udev->parent && !PMSG_IS_AUTO(msg))
                       status = 0;

This patch is useful to fix the problem that the system goes on suspending after receiving remote
wakeup signal. Please see below log:

*********************************************Before the patch*************************************
root@freescale ~$ echo standby > /sys/power/state
PM: Syncing filesystems ... done.
PM: Preparing system for standby sleep
Freezing user space processes ... (elapsed 0.01 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
PM: Entering standby sleep
usbhid 1-1:1.0: usb_suspend_interface: status 0
add wake up source irq 142
add wake up source irq 100
udc suspend begins
usb 1-1: usb suspend
usb 1-1: usb_suspend_device: status 0
usb_suspend_both: msg.event is 0x2
usb 1-1: usb_suspend_both: status 0
hub 1-0:1.0: hub_suspend
hub 1-0:1.0: usb_suspend_interface: status 0
usb usb1: bus suspend
fsl-ehci fsl-ehci.0: port 1 remote wakeup
ehci_fsl_bus_suspend begins, DR
fsl-ehci fsl-ehci.0: suspend root hub
fsl-ehci fsl-ehci.0: suspend failed because port 1 is resuming
usb usb1: bus suspend fail, err -16
usb usb1: usb_suspend_device: status -16
usb_suspend_both: msg.event is 0x2
usb usb1: usb_suspend_both: status 0
fsl-ehci fsl-ehci.0: USB Host suspend begins
fsl-ehci fsl-ehci.0: system pm events
fsl-ehci fsl-ehci.0: USB Host suspend ends
add wake up source irq 51
add wake up source irq 59
PM: suspend of devices complete after 117.377 msecs
PM: late suspend of devices complete after 0.518 msecs
Disabling non-boot CPUs ...
CPU1: shutdown
CPU2: shutdown
CPU3: shutdown

******************************************* After the patch ***************************************************
root@freescale ~$ echo standby > /sys/power/state
PM: Syncing filesystems ... done.
PM: Preparing system for standby sleep
Freezing user space processes ... (elapsed 0.01 seconds) done.
Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
PM: Entering standby sleep
usbhid 1-1:1.0: usb_suspend_interface: status 0
add wake up source irq 142
add wake up source irq 100
udc suspend begins
usb 1-1: usb suspend
fsl-ehci fsl-ehci.0: port 1 remote wakeup
usb 1-1: usb_suspend_device: status 0
usb_suspend_both: msg.event is 0x2
usb 1-1: usb_suspend_both: status 0
hub 1-0:1.0: hub_suspend
hub 1-0:1.0: usb_suspend_interface: status 0
usb usb1: bus suspend
ehci_fsl_bus_suspend begins, DR
fsl-ehci fsl-ehci.0: suspend root hub
fsl-ehci fsl-ehci.0: suspend failed because port 1 is resuming
usb usb1: bus suspend fail, err -16
usb usb1: usb_suspend_device: status -16
usb_suspend_both: msg.event is 0x2
hub 1-0:1.0: hub_resume
fsl-ehci fsl-ehci.0: GetStatus port:1 status 14601405 10  ACK POWER sig=k PE CONNECT
hub 1-0:1.0: port 1: status 0303 change 0004
hub 1-0:1.0: usb_resume_interface: status 0
usb usb1: usb_suspend_both: status -16
pm_op(): usb_dev_suspend+0x0/0x8 returns -16
PM: Device usb1 failed to suspend async: error -16
PM: Some devices failed to suspend
USB Gadget resume begins
fsl-ehci fsl-ehci.0: GetStatus port:1 status 14601405 10  ACK POWER sig=k PE CONNECT
usb 1-1: finish resume
usb 1-1: usb_resume_device: status 0
usbhid 1-1:1.0: usb_resume_interface: status 0
usb 1-1: usb_resume_both: status 0
fsl_udc_resume, Wait for wakeup thread finishes
remove wake up source irq 100
remove wake up source irq 142
PM: resume of devices complete after 32.943 msecs
PM: Finishing wakeup.
Restarting tasks ... 
hub 1-0:1.0: state 7 ports 1 chg 0002 evt 0002
done.
hub 1-0:1.0: usb_autopm_get_interface: cnt 2 -> 0
hub 1-0:1.0: port 1, status 0303, change 0000, 1.5 Mb/s
hub 1-0:1.0: usb_autopm_put_interface: cnt 0 -> 0

*************************************************************************************************************
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,13 @@ 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)) {
+                       hcd_bus_resume(rhdev, PMSG_AUTO_RESUME);
+                       status = -EBUSY;
+               }

This patch may not useful, as if the remote wakeup occurs before the bus(roothub) suspend
(taking ehic_bus_suspend as an example), the bus suspend will quit due to there is a resume
signal at one port. If the remote wakeup occurs after bus suspend, but before your patch,
the HCD_FLAG_POLL_PENDING will not be set due to hcd->driver->hub_status_data returns 0 (taking
ehci as an example), as this flag is set after 25ms later after remote wakeup occurs.

For your talked condition B:
(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.

It may fail for system suspend condition due to workqueue is frozen at that time. 
 


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