i2c: nomadik: Fixup system suspend

PATCH New Ulf Hansson 3 participants 7 messages Apr 10, 2014 – May 21, 2014

Subsystems (2)

I2C subsystem host drivers Arm/nomadik/Ux500 architectures

lore

Share & tools

lore

Ulf Hansson

author

2014-04-10 13:59 UTC #0 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

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]>

---

Do note, this patch were sent during the previous kernel release cycle, as a part of another patchset on the PM core. Back then, it provided proof of concept, for the new runtime PM helper functions:
pm_runtime_force_suspend|resume().

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

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

index 28cbe1b..7d27eee 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

Signature · 1 line

1.7.9.5

Linus Walleij

2014-04-10 18:49 UTC #1 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

Quoted 14 lines · Ulf Hansson

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.

Quoted 7 lines

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]>

Acked-by Linus Walleij <[email protected]>

Yours,
Linus Walleij

Ulf Hansson

author

2014-05-06 09:31 UTC #2 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

Quoted 19 lines · Linus Walleij

Quoted 17 lines · Ulf Hansson
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.

Quoted 6 lines

Quoted 4 lines
Cc Alessandro Rubini <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Wolfram Sang <[email protected]>
Signed-off-by Ulf Hansson <[email protected]>
Acked-by Linus Walleij <[email protected]>

Hi Wolfram,

This patch as has been around for a while (in some other versions as well), would you mind picking it up for 3.16?

Kind regards
Ulf Hansson

Quoted 3 lines

Yours,
Linus Walleij

Ulf Hansson

author

2014-05-15 07:15 UTC #3 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

Quoted 25 lines · Ulf Hansson

Quoted 24 lines · Linus Walleij
Quoted 21 lines · Ulf Hansson
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]>

Quoted 7 lines

Quoted 1 line
Acked-by Linus Walleij <[email protected]>
Hi Wolfram,
This patch as has been around for a while (in some other versions as well), would you mind picking it up for 3.16?

Ping.

I noticed you merged my other i2c-nomadik patch yesterday - thought I might remind you about this one as well.

Kind regards
Ulf Hansson

Wolfram Sang

2014-05-15 07:48 UTC #4 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

Quoted 3 lines

I noticed you merged my other i2c-nomadik patch yesterday - thought I might remind you about this one as well.

I was working on for-current. I'll start working on for-next these days. I use patchwork, as long as you cc the list, patches won't be forgotten ;)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140515/5aba0614/attachment.sig>

Ulf Hansson

author

2014-05-15 15:36 UTC #5 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

Quoted 7 lines · Wolfram Sang

Quoted 3 lines
I noticed you merged my other i2c-nomadik patch yesterday - thought I might remind you about this one as well.
I was working on for-current. I'll start working on for-next these days. I use patchwork, as long as you cc the list, patches won't be forgotten ;)

Thanks Wolfram, sorry for nagging! :-)

Kind regards
Ulf Hansson

Quoted 1 line

Wolfram Sang

Applied

2014-05-21 08:33 UTC #6 lore raw

PATCH 1/1 i2c: nomadik: Fixup system suspend

In reply to Ulf Hansson

Quoted 14 lines · Ulf Hansson

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.

Quoted 7 lines

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]>

Applied to for-next, thanks!