[PATCH] i2c: nomadik: Fixup system suspend

[PATCH] i2c: nomadik: Fixup system suspend

[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 <rubini-9wsNiZum9E8@public.gmane.org>
Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

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(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 28cbe1b..7d27eee 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -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

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Linus Walleij]

On Thu, Apr 10, 2014 at 3:59 PM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

> 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 <rubini-9wsNiZum9E8@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Yours,
Linus Walleij

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Ulf Hansson]

On 10 April 2014 20:49, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Apr 10, 2014 at 3:59 PM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>> 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 <rubini-9wsNiZum9E8@public.gmane.org>
>> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
>> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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

>
> Yours,
> Linus Walleij

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Ulf Hansson]

On 6 May 2014 11:31, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 10 April 2014 20:49, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On Thu, Apr 10, 2014 at 3:59 PM, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>>> 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 <rubini-9wsNiZum9E8@public.gmane.org>
>>> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
>>> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> Acked-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> 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

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]


> 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
;)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Ulf Hansson]

On 15 May 2014 09:48, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>
>> 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

>

Re: [PATCH] i2c: nomadik: Fixup system suspend

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

On Thu, Apr 10, 2014 at 03:59:36PM +0200, Ulf Hansson wrote:
> 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 <rubini-9wsNiZum9E8@public.gmane.org>
> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Applied to for-next, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]