On Mon, 6 Sep 2010, Rafael J. Wysocki wrote: > On Saturday, September 04, 2010, Alan Stern wrote: > > On Sat, 4 Sep 2010, Rafael J. Wysocki wrote: > > > > > OK, so in the context of my other message, I think we need these types of > > > helpers: > > > > > > (1) "suspend X ms from now" > > > (2) "suspend X ms from now if the device has been inactive for Y ms" > > > > > > where Y is set by user space (and X may be 0). > > > > > > To me, the both of them make sense. > > > > (2) is really: "starting X ms from now, wait until the device has been > > inactive for Y ms and then suspend". Or rather, that's how my patch implements (2). > Not necessarily. If the device has already been inactive for Y ms and this > one is called, I think it doesn't have to wait for another Y ms. It only has > to wait for X ms. That's the same as what I wrote. Suppose the device has already been inactive for Y ms when (2) is called. It will first wait for X ms, and then it will do "wait until the device has been inactive for Y ms and then suspend". But at that point the device has _already_ been inactive for at least Y+X ms; thus it will suspend as soon as the X ms have passed. > > Well and good, but what about pm_runtime_suspend? Should it respect > > the Y ms delay? Even if the last helper called was (1)? > > No, it shouldn't. I don't think it's appropriate to change the behavior of the > exisitng helpers. We can just drop them in the future if nobody uses them. > > > Suppose the driver uses either helper (1) or helper (2) and then some > > other piece of code calls pm_runtime_get_sync followed by > > pm_runtime_put_sync. The get_sync will cancel the timer, but what > > should the put_sync do? > > > > Your helper (1) bypasses the user's requirement for Y ms of inactivity > > before a runtime suspend. When should the bypass end and normal > > behavior (i.e., using the Y-ms delay) be restored? The next time > > either helper is called? > > I'd say this is the driver's problem, really. > > In my opinion the core should provide two sets of helpers, the current one > (ie. no changes to the existing helpers) _and_ a new one allowing last_busy > and friends to be taken into account. We can add a recommendation not to > mix the two, if you like. > > Then, setting use_suspend_delay would be a driver's declaration to user space > "I'm going to use the suspend delay you give to me" and the driver would be > responsible for keeping this promise (by using the appropriate helpers, among > other things). In other words, you're suggesting there should be pm_runtime_autosuspend, pm_runtime_schedule_autosuspend, and pm_runtime_put_autosuspend in addition to the existing pm_runtime_suspend, pm_runtime_schedule_suspend, and pm_runtime_put. I can do that; just copy the existing routines and make the necessary additions. Curiously, it works only because the driver core nevers calls any of the _suspend routines; it always uses the _idle versions. 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
