On 7 November 2014 15:50, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 7 Nov 2014, Krzysztof Kozlowski wrote: > >> > Well, that is a good reason to introduce a wrapper around power.irq_safe in my >> > view. >> > >> > And define the wrapper so that it always returns false for CONFIG_PM_RUNTIME >> > unset. >> > >> > This way not only you wouldn't need to move the flag from under the #ifdef, >> > but also you would make the compiler skip the relevant pieces of code >> > entiretly for CONFIG_PM_RUNTIME unset. >> >> Few days ago I would be happy with your opinion :), but know I think >> this is better solution than wrapper. Consider case: >> 1. PM_RUNTIME unset. >> 2. System suspends. >> 3. The pl330 in its suspend callback calls force_runtime_suspend which >> leads us to amba/bus. >> 4. The amba/bus.c in runtime suspend checks for irq_safe (it is FALSE), >> so it disables and unprepares the clock. >> 5. The pl330 in probe requested irq_safe so it assumes amba/bus will >> only disable the clock. So the pl330 unprepares the clock. Again. > > To me, this sounds like a good reason to avoid using > force_runtime_suspend(). In fact, it sounds like a good reason to > avoid relying on the runtime PM mechanism to handle non-runtime-PM > things (like a system suspend callback). If CONFIG_PM_RUNTIME isn't > enabled then the runtime PM stack simply should not be used. There are an important advantage of using the pm_runtime_force_suspend() here. For the driver to handle clock gating at system PM suspend, it first needs to bring the device into full power, through pm_runtime_get_sync(). Otherwise it's not safe to gate the clock, since it may already be gated. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
