Thread (16 messages) 16 messages, 4 authors, 2014-11-07

Re: [PATCH v9 2/4] amba: Don't unprepare the clocks if device driver wants IRQ safe runtime PM

From: Krzysztof Kozlowski <hidden>
Date: 2014-11-07 12:18:12
Also in: lkml

On pią, 2014-11-07 at 13:13 +0100, Pavel Machek wrote:
On Wed 2014-11-05 09:42:58, Krzysztof Kozlowski wrote:
quoted
On wto, 2014-11-04 at 21:18 +0100, Pavel Machek wrote:
quoted
On Tue 2014-11-04 13:52:48, Krzysztof Kozlowski wrote:
quoted
The AMBA bus driver defines runtime Power Management functions which
disable and unprepare AMBA bus clock. This is problematic for runtime PM
because unpreparing a clock might sleep so it is not interrupt safe.

However some drivers may want to implement runtime PM functions in
interrupt-safe way (see pm_runtime_irq_safe()). In such case the AMBA
bus driver should only disable/enable the clock in runtime suspend and
resume callbacks.

quoted
 /*
  * Hooks to provide runtime PM of the pclk (bus clock).  It is safe to
  * enable/disable the bus clock at runtime PM suspend/resume as this
@@ -95,8 +102,14 @@ static int amba_pm_runtime_suspend(struct device *dev)
 	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = pm_generic_runtime_suspend(dev);
 
-	if (ret == 0 && dev->driver)
-		clk_disable_unprepare(pcdev->pclk);
+	if (ret == 0 && dev->driver) {
+		pcdev->irq_safe = get_pm_runtime_irq_safe(dev);
+
+		if (pcdev->irq_safe)
+			clk_disable(pcdev->pclk);
+		else
+			clk_disable_unprepare(pcdev->pclk);
+	}
So you can handle the case of !pcdev->irq_safe. What is the penalty
for always assuming !pcdev->irq_safe?
The penalty (for pl330 driver) would be that the runtime resume/suspend
cannot happen from atomic context
  => pm_runtime_get_sync() cannot be called from atomic context
    => complete rework of runtime PM for pl330 DMA driver because now
       one of pm_runtime_get_sync() calls is in device_issue_pending
       callback which may not sleep. And by "rework" I also mean that
       I do not know how to do this... yet.
I still don't get it. You say that you don't know how to handle
!pcdev->irq_safe case... Yet have code above that tries to handle it.

If that case can't be sanely handled, I'd expect
BUG_ON(!pcdev->irq_safe).
Hmmm... I could misunderstand your question. The amba/bus.c driver can
handle both cases. However this varies for child drivers (which use
these runtime PM callbacks too). For pl330 cannot handle non-irq-safe.
Other drivers can.

Is it the answer for your question?

Best regards,
Krzysztof

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help