On Thu, 18 Oct 2012, vikram pandita wrote: > >> Why on enable/disable would we not want to ensure PSS state is same as PSE? > > > > Because it would be a waste of time. Eventually the controller will > > set PSS equal to PSE; why should the driver wait for it? The driver > > can go on to do other useful work in the meantime. > > > > Thanks for your comments. > > In a general working case of USB auto-suspend followed by > auto-resume/remote-wake, this looks fine and makes sense on PC/intel > controllers. > > Lets consider EHCI controller on an ARM soc in a phone say attached to > modem with multiple IN interrupt EPs, and system wide suspend happens > (aka echo mem > /sys/power/state), as we do want to save power. > > Scenario: > --- > Active USB traffic - SOFs going on > USB auto-suspend: usb_runtime_suspend() > > Device is first resumed to send int-urb: usb_runtime_resume() > |-> calls: enable_periodic() > Checks PSS==1, Sets PSE=1, [No check for PSS==1] > > Trigger System Wide Suspend : echo mem > /sys/power/state > > Device suspend started: usb_dev_suspend() > |-> calls: disable_periodic() > Checks PSS==1, Sets PSE=0, [No check for PSS==0] You left something out here. ehci_hub_suspend() calls ehci_quiesce(), which waits for PSS to become 0. However it stops waiting after 2 ms; you may want to increase this limit. I don't know why the hardware should take any longer than that, though, since there no active transfers at all at this time -- both the async and periodic schedules are completely empty. > Chip goes to low power mode like OFF. > *(Now at this stage we cannot guarantee PSS state: 0 or 1) > > Wakeup the devcie, restore state of EHCI, etc.... > --- > > We have seen in the past that time to disable Periodic schedule can be > variable on different hw. > > Given we have left a window open by not checking PSS, its not guaranteed > in different EHCI implementations, what state would be restored: PSS 1 > or PSS 0 - When we know we always wanted to restore in state PSS=0. > > I have done this analysis on 3.0 kernel which does not have yet the 10ms > delay implementation in disable path. I am not sure how this 10ms > timeout will play with system wide suspend and could there be a race?? What 10-ms delay are you talking about? And what disable path? Do you mean the msleep() call at the start of ehci_suspend()? Alan Stern -- 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
