On sob, 2014-11-01 at 01:42 +0100, Rafael J. Wysocki wrote: > On Friday, October 31, 2014 11:04:52 PM Russell King - ARM Linux wrote: > > On Sat, Nov 01, 2014 at 12:11:05AM +0100, Rafael J. Wysocki wrote: > > > [CC list trimmed + added Kevin Hilman] > > > > > > On Monday, October 20, 2014 11:04:44 AM Krzysztof Kozlowski wrote: > > > > Add a simple getter pm_runtime_is_irq_safe() for querying whether > > > > runtime > > > > PM IRQ safe was set or not. > > > > > > > > Various bus drivers implementing runtime PM may use choose to suspend > > > > differently based on IRQ safeness status of child driver (e.g. do not > > > > unprepare the clock if IRQ safe is not set). > > > > > > > > Signed-off-by: Krzysztof Kozlowski <k.kozlow...@samsung.com> > > > > Reviewed-by: Ulf Hansson <ulf.hans...@linaro.org> > > > > > > So why do we need to add the wrapper? > > > > > > And it goes kind of against the intention which was to set irq_safe when > > > we knew that the callbacks were safe to be executed from interrupt context > > > and not when we wished that to be the case. > > > > This was provided in the covering email - I quote: > > > > This patchset adds runtime and system PM to the pl330 driver. > > > > The runtime PM of pl330 driver requires interrupt safe suspend/resume > > callbacks which is in conflict with current amba bus driver. > > The latter also unprepares and prepares the AMBA bus clock which > > is not safe for atomic context. > > > > The patchset solves this in patch 3/5 by handling clocks in different > > way if device driver set interrupt safe runtime PM. > > So I'm still unsure why we need the wrapper. IMHO this check in particular: > > WARN_ON(pcdev->irq_safe != pm_runtime_is_irq_safe(dev)); > > (and should that be WARN_ON_ONCE(), for that matter?), looks better this way: > > WARN_ON(pcdev->irq_safe != dev->power.irq_safe); > > and so on, pretty much.
I used the wrapper only to hide the actual code behind interface but it don't really matter to me. > Besides, these special "irq safe" code paths in the bus type look > considerably ugly to me. I'd probably use an "irq safe" PM domain for > that device and put it in there instead of doing the > > pcdev->irq_safe = pm_runtime_is_irq_safe(dev); > > thing in amba_probe(). But that's just me. :-) The device is not attached to any domain and there is no hardware domain matching. Thanks for feedback! Best regards, Krzysztof > > There's one weak point in [3/5], but let me comment it in there. > > Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
