PM / Sleep / Runtime: Fixup some driver's system suspend

This patch provides two new runtime PM helper functions which intend to be used from system suspend/resume callbacks, to make sure devices are put into low power state during system suspend and brought back to full power at system resume.

The prerequisite is to have all levels of a device's runtime PM callbacks to be defined through the SET_PM_RUNTIME_PM_OPS macro, which means these are available for CONFIG_PM.

By using the new runtime PM helper functions especially the two scenarios below will be addressed.

1) The PM core prevents .runtime_suspend callbacks from being invoked during system suspend. That means even for a runtime PM centric subsystem and driver, the device needs to be put into low power state from a system suspend callback. Otherwise it may very well be left in full power state (runtime resumed) while the system is suspended. By using the new helper functions, we make sure to walk the hierarchy of a device's power domain, subsystem and driver.

2) Subsystems and drivers need to cope with all the combinations of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. The two new helper functions smothly addresses this.

Cc Kevin Hilman <[email protected]>

Cc Alan Stern <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Cc Mark Brown <[email protected]>

Cc Russell King <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Wolfram Sang <[email protected]>

Cc Alessandro Rubini <[email protected]>

Signed-off-by Ulf Hansson <[email protected]>

Convert to the SET_PM_RUNTIME_PM macro while defining the runtime PM callbacks. This means the callbacks becomes available for both CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME, which is needed to handle the combinations of these scenarios.

Previously only the resources controlled by the driver were put into low power state at system suspend. Both the amba bus and a potential power domain were ignored.

For !CONFIG_PM_RUNTIME, the device were never put back into active state while resuming.

For CONFIG_PM_RUNTIME, we blindly trusted the device to be inactive while we were about to handle it at suspend late, which is just too optimistic.

Even if the driver uses pm_runtime_put_sync() after each tranfer to return it's runtime PM resources, there are no guarantees this will actually mean the device will inactivated. The reason is that the PM core will prevent runtime suspend during system suspend, and thus when a transfer occurs during the early phases of system suspend the device will be kept active after the transfer.

To handle both issues above, use pm_runtime_force_suspend|resume() from the system suspend|resume callbacks.

Cc Alessandro Rubini <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Wolfram Sang <[email protected]>

Signed-off-by Ulf Hansson <[email protected]>

---
 drivers/i2c/busses/i2c-nomadik.c |   14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

drivers/i2c/busses/i2c-nomadik.c +7 -8

index 8082f5c..519f5b8 100644
@@ -879,19 +879,19 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 #ifdef CONFIG_PM_SLEEP
 static int nmk_i2c_suspend_late(struct device *dev)
 {
-       pinctrl_pm_select_sleep_state(dev);
+       int ret;
 
+       ret = pm_runtime_force_suspend(dev);
+       if (ret)
+               return ret;
+
+       pinctrl_pm_select_sleep_state(dev);
        return 0;
 }
 
 static int nmk_i2c_resume_early(struct device *dev)
 {
-       /* First go to the default state */
-       pinctrl_pm_select_default_state(dev);
-       /* Then let's idle the pins until the next transfer happens */
-       pinctrl_pm_select_idle_state(dev);
-
-       return 0;
+       return pm_runtime_force_resume(dev);
 }
 #endif
 
--

1.7.9.5

In runtime suspended state, we are not expecting IRQs and thus we can safely mask them, not only for pwrreg_nopower variants but for all.

Obviously we then also need to make sure we restore the IRQ mask while becoming runtime resumed.

Cc Russell King <[email protected]>

Signed-off-by Ulf Hansson <[email protected]>

---
 drivers/mmc/host/mmci.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

drivers/mmc/host/mmci.c +11 -13

index b931226..178868a 100644
@@ -1758,35 +1758,34 @@ static void mmci_save(struct mmci_host *host)
 {
        unsigned long flags;
 
-       if (host->variant->pwrreg_nopower) {
-               spin_lock_irqsave(&host->lock, flags);
+       spin_lock_irqsave(&host->lock, flags);
 
-               writel(0, host->base + MMCIMASK0);
+       writel(0, host->base + MMCIMASK0);
+       if (host->variant->pwrreg_nopower) {
                writel(0, host->base + MMCIDATACTRL);
                writel(0, host->base + MMCIPOWER);
                writel(0, host->base + MMCICLOCK);
-               mmci_reg_delay(host);
-
-               spin_unlock_irqrestore(&host->lock, flags);
        }
+       mmci_reg_delay(host);
 
+       spin_unlock_irqrestore(&host->lock, flags);
 }
 
 static void mmci_restore(struct mmci_host *host)
 {
        unsigned long flags;
 
-       if (host->variant->pwrreg_nopower) {
-               spin_lock_irqsave(&host->lock, flags);
+       spin_lock_irqsave(&host->lock, flags);
 
+       if (host->variant->pwrreg_nopower) {
                writel(host->clk_reg, host->base + MMCICLOCK);
                writel(host->datactrl_reg, host->base + MMCIDATACTRL);
                writel(host->pwr_reg, host->base + MMCIPOWER);
-               writel(MCI_IRQENABLE, host->base + MMCIMASK0);
-               mmci_reg_delay(host);
-
-               spin_unlock_irqrestore(&host->lock, flags);
        }
+       writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+       mmci_reg_delay(host);
+
+       spin_unlock_irqrestore(&host->lock, flags);
 }
 
 static int mmci_runtime_suspend(struct device *dev)
--

1.7.9.5

Quoted 8 lines · Ulf Hansson

In runtime suspended state, we are not expecting IRQs and thus we can safely mask them, not only for pwrreg_nopower variants but for all.

Obviously we then also need to make sure we restore the IRQ mask while becoming runtime resumed.

Cc Russell King <[email protected]>

Signed-off-by Ulf Hansson <[email protected]>

Hi Russell,

I suppose we have discussed and sorted out the concerns you had around this patch!?

It's available in your patch tracker, would be nice to get it merged for 3.15, if that's possible.

Kind regards
Uffe

Quoted 59 lines

---
 drivers/mmc/host/mmci.c |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

drivers/mmc/host/mmci.c +11 -13

index b931226..178868a 100644
@@ -1758,35 +1758,34 @@ static void mmci_save(struct mmci_host *host)
 {
        unsigned long flags;
 
-       if (host->variant->pwrreg_nopower) {
-               spin_lock_irqsave(&host->lock, flags);
+       spin_lock_irqsave(&host->lock, flags);
 
-               writel(0, host->base + MMCIMASK0);
+       writel(0, host->base + MMCIMASK0);
+       if (host->variant->pwrreg_nopower) {
                writel(0, host->base + MMCIDATACTRL);
                writel(0, host->base + MMCIPOWER);
                writel(0, host->base + MMCICLOCK);
-               mmci_reg_delay(host);
-
-               spin_unlock_irqrestore(&host->lock, flags);
        }
+       mmci_reg_delay(host);
 
+       spin_unlock_irqrestore(&host->lock, flags);
 }
 
 static void mmci_restore(struct mmci_host *host)
 {
        unsigned long flags;
 
-       if (host->variant->pwrreg_nopower) {
-               spin_lock_irqsave(&host->lock, flags);
+       spin_lock_irqsave(&host->lock, flags);
 
+       if (host->variant->pwrreg_nopower) {
                writel(host->clk_reg, host->base + MMCICLOCK);
                writel(host->datactrl_reg, host->base + MMCIDATACTRL);
                writel(host->pwr_reg, host->base + MMCIPOWER);
-               writel(MCI_IRQENABLE, host->base + MMCIMASK0);
-               mmci_reg_delay(host);
-
-               spin_unlock_irqrestore(&host->lock, flags);
        }
+       writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+       mmci_reg_delay(host);
+
+       spin_unlock_irqrestore(&host->lock, flags);
 }
 
 static int mmci_runtime_suspend(struct device *dev)
--

1.7.9.5

Patch 1 -> 2:
These patches provides two new runtime PM helper functions which intend to be used from system suspend/resume callbacks, to make sure devices are put into low power state during system suspend and brought back to full power at system resume.

The prerequisite is to have all levels of a device's runtime PM callbacks to be defined through the SET_PM_RUNTIME_PM_OPS macro, which means these are available for CONFIG_PM.

By using the new runtime PM helper functions especially the two scenarios below will be addressed.

1) The PM core prevents .runtime_suspend callbacks from being invoked during system suspend. That means even for a runtime PM centric subsystem and driver, the device needs to be put into low power state from a system suspend callback. Otherwise it may very well be left in full power state (runtime resumed) while the system is suspended. By using the new helper functions, we make sure to walk the hierarchy of a device's power domain, subsystem and driver.

2) Subsystems and drivers need to cope with all the combinations of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. The two new helper functions smothly addresses this.

Quoted 20 lines · Ulf Hansson

Patch 1 -> 2:
These patches provides two new runtime PM helper functions which intend to be used from system suspend/resume callbacks, to make sure devices are put into low power state during system suspend and brought back to full power at system resume.

The prerequisite is to have all levels of a device's runtime PM callbacks to be defined through the SET_PM_RUNTIME_PM_OPS macro, which means these are available for CONFIG_PM.

By using the new runtime PM helper functions especially the two scenarios below will be addressed.

1) The PM core prevents .runtime_suspend callbacks from being invoked during system suspend. That means even for a runtime PM centric subsystem and driver, the device needs to be put into low power state from a system suspend callback. Otherwise it may very well be left in full power state (runtime resumed) while the system is suspended. By using the new helper functions, we make sure to walk the hierarchy of a device's power domain, subsystem and driver.

Quoted 3 lines

2) Subsystems and drivers need to cope with all the combinations of CONFIG_PM_SLEEP and CONFIG_PM_RUNTIME. The two new helper functions smothly addresses this.

OK, I've queued up [1-2/8] for 3.15. The rest generally didn't apply for me and I think it'd be better to push them through the respective subsystem trees later anyway.

Thanks!

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

OK, I've queued up [1-2/8] for 3.15. The rest generally didn't apply for me and I think it'd be better to push them through the respective subsystem trees later anyway.

Quoted 3 lines · Rafael J. Wysocki

OK, I've queued up [1-2/8] for 3.15. The rest generally didn't apply for me and I think it'd be better to push them through the respective subsystem trees later anyway.

Is there a tag we can pull to get those patches? The driver changes depend on the new API introduced in patch 2.

Quoted 2 lines · Mark Brown

Is there a tag we can pull to get those patches? The driver changes depend on the new API introduced in patch 2.

They are in the pm-drivers branch of linux-pm.git.