[RFC PATCH] ARM: OMAP2+: fix omap_device with noirq_suspend

[RFC PATCH] ARM: OMAP2+: fix omap_device with noirq_suspend

[Andreas Kemnade]

Fixes cases where device runtime gets resumed in _suspend but then
pm_runtime_force_suspend is used.  So the device is disabled, but runtime
active in _noirq_suspend leading to duplicate runtime_suspend calls. So
rather check for the device state directly to indicate if changes are
needed.
This relies on pm_runtime_force_resume being used in a balanced way.

Fixes: 7da7fd7e66ac9 ("i2c: omap: wakeup the controller during suspend() callback")
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
This really needs intensive scrutiny but might be a less invasive patch
solution better suited for stable and paves the way to get out of the
omap_device theatre without too much hurry.

 arch/arm/mach-omap2/omap_device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 77a75b0b9ae69..52afc9089a5a0 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -393,7 +393,7 @@ static int _od_suspend_noirq(struct device *dev)
 
 	ret = pm_generic_suspend_noirq(dev);
 
-	if (!ret && !pm_runtime_status_suspended(dev)) {
+	if (!ret && (od->_state == OMAP_DEVICE_STATE_ENABLED)) {
 		if (pm_generic_runtime_suspend(dev) == 0) {
 			omap_device_idle(pdev);
 			od->flags |= OMAP_DEVICE_SUSPENDED;
-- 
2.47.3

Re: [RFC PATCH] ARM: OMAP2+: fix omap_device with noirq_suspend

[Kevin Hilman]

Andreas Kemnade <andreas@kemnade.info> writes:

> Fixes cases where device runtime gets resumed in _suspend but then
> pm_runtime_force_suspend is used.  So the device is disabled, but runtime
> active in _noirq_suspend leading to duplicate runtime_suspend calls. So
> rather check for the device state directly to indicate if changes are
> needed.
> This relies on pm_runtime_force_resume being used in a balanced way.
>
> Fixes: 7da7fd7e66ac9 ("i2c: omap: wakeup the controller during suspend() callback")
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> This really needs intensive scrutiny but might be a less invasive patch
> solution better suited for stable and paves the way to get out of the
> omap_device theatre without too much hurry.

This looks & feels correct to me.

I also tested this (along with my WIP patch for that NEON count
mismatch) and I have a very stable suspend/resume on OMAP3.

I tested on omap3-beagle (retention) and omap3-beagle-xm (off-mode) and
both are now reliable across multiple suspend/resume sequences with
these two patches.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>
Tested-by: Kevin Hilman <khilman@baylibre.com>

Kevin

>  arch/arm/mach-omap2/omap_device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
> index 77a75b0b9ae69..52afc9089a5a0 100644
> --- a/arch/arm/mach-omap2/omap_device.c
> +++ b/arch/arm/mach-omap2/omap_device.c
> @@ -393,7 +393,7 @@ static int _od_suspend_noirq(struct device *dev)
>  
>  	ret = pm_generic_suspend_noirq(dev);
>  
> -	if (!ret && !pm_runtime_status_suspended(dev)) {
> +	if (!ret && (od->_state == OMAP_DEVICE_STATE_ENABLED)) {
>  		if (pm_generic_runtime_suspend(dev) == 0) {
>  			omap_device_idle(pdev);
>  			od->flags |= OMAP_DEVICE_SUSPENDED;
> -- 
> 2.47.3