On 18 November 2013 16:17, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 18 Nov 2013, Ulf Hansson wrote: > >> I favour the pm_runtime_no_prevent_suspend API approach, since it >> would mean a change in behaviour of the PM core. > > That's exactly why I don't favor it! :-) I will try my best to convince you then. :-) > >> I also think that in >> some cases it could make runtime PM better understandable, for those >> who are deploying it. > > I don't see how it would make runtime PM either more or less > understandable, since the proposed change affects system PM, not > runtime PM. On the other hand, it would make the Runtime PM > documentation more complicated. At the moment, system PM is already affecting behaviour of runtime PM since it is preventing runtime suspend during system suspend. To me and and those driver authors, that don't see the direct need for why the PM core behaves like this (because we are not working with PCI drivers for example), I would guess this is probably the piece in the documentation we have more difficult to understand and adapt to. I can point at some examples, were I believe drivers that use runtime PM, tries to adapt to the PM core behaviour, but I fear they for too many times have got it wrong. Here are some various examples I have found, which made me think like this. 1. Not many are using the .suspend_late callback at all, which should be the most proper solution to use. 2. Drivers are using the synchronise pm_runtime_put_sync in their .suspend callbacks. 3. Drivers do "pm_runtime_get_sync" from their .suspend callbacks, but don't put the device into inactive state from anywhere as a part of the system suspend. 4. Drivers only handles system suspend tasks from it's .suspend callbacks and don't implement .suspend_late, thus the device will not be fully inactivated. 5. Drivers relies on pm_runtime_suspended or worse pm_runtime_status_suspend, to check if they shall put their device into inactive state from their .suspend callbacks or leave it as is. While they put the device into inactive state, the runtime state is not properly updated to reflect this. Most important, at this point runtime PM has not yet been disabled from the PM core. > > (Also, I think the name "pm_runtime_no_prevent_suspend" is a very bad > choice. It should be something more like > "allow_runtime_suspend_during_system_sleep".) I am not so happy with the naming myself, we should work out a better one then. > >> Another advantage of "pm_runtime_no_prevent_suspend" is that driver's >> can still easily prevent runtime suspend, during system suspend, by an >> extra call to "pm_runtime_get*". Typically this could be used when the >> device is configured for wakeup. > > How is this an advantage? Drivers can accomplish the same thing right > now by not doing anything at all. > >> In this case the generic suspend_late >> approach would not work, since the driver would need to implement it's >> own .suspend_late callback to handle this. > > Actually, drivers would simply need to _avoid_ setting the > .suspend_late callback. I should have stated, that I means those scenarios were the wakeup configuration is decided in runtime and depending on the current situation. For example depending on what kind of peripheral that is connected. In this scenario, I think the advantage still stands. Kind regards Ulf Hansson > > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
