On Thu, 4 Aug 2016, Felipe Balbi wrote: > Hi, > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: > > > On Thu, 4 Aug 2016, Felipe Balbi wrote: > > > >> > What are you trying to accomplish? Is the problem that wakeup signals > >> > only cause the platform device to be runtime-resumed, but you also need > >> > the HCD to wake up? And conversely, whenever the HCD gets > >> > runtime-suspended you also want the platform device to go into runtime > >> > suspend? > >> > >> this is DWC3 as peripheral-only. PCI Wakeup (PME) wakes up dwc3-pci, but > >> we need dwc3 core (a platform device) to be resumed as well. > > > > Oh, so I got the types wrong. But the basic idea is still the same: > > When the parent wakes up, it does a runtime-resume of the child. The > > child is then responsible for making sure it stays awake as long as > > necessary, and when it goes back to low power the parent naturally does > > the same. > > right. Here's a doubt though: if dwc3-pci does a pm_runtime_resume() of > the child, doesn't that mean child will try to sleep right away again > since its usage_counter will be zero? We _do_ have a 5s timeout which > should be enough to trigger enumeration, but still... I guess I'm > missing something here. That is the subtle part of the scheme. It depends on why the wakeup occurred. For example, suppose the wakeup signal was triggered by a port-connect (Vbus) event. When the child dwc3-core driver's runtime-resume callback runs, it should check the port status. When it sees the Vbus change, it will need to increment the runtime-PM usage counter until the connect event has been handled (or until the driver decides it can safely let the dwc3-core device go back to low power, which might be some time later). The same sort of thing should happen for other kinds of wakeup events. This is how the PM core expects all drivers to work. When the runtime-resume callback is invoked, the core guarantees there won't be any other runtime-PM callbacks until runtime-resume returns. But once the callback does return, all bets are off -- if the driver wants to keep the device active, it needs to increment the usage counter. > I guess it'll work just fine since we set a driver flag when we see a > bus reset and that gets cleared once a disconnect shows up. Seems like > the whole idea is to avoid an extra pm_runtime_put() in > ->runtime_resume()? The whole idea of doing pm_runtime_put() in the runtime_resume callback is just bizarre. It means that somebody had previously incremented the usage counter for no reason (since the counter won't be used for keeping the device at full power). Instead of having some other code call pm_runtime_get() and then calling pm_runtime_put() in the resume handler, why not simply have that other code call pm_request_resume() and the resume handler do nothing? > What if user sets autosuspend_delay to 0 in sysfs? Seems like this is > likely to break, no? No, the driver should be written to work properly even in that case. Setting the autosuspend_delay to 0 might cause a bunch of unnecessary or very short suspend/resume cycles to occur, but it shouldn't break anything. 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
