Re: [PATCH] OMAP3: disable idle during the entire suspend/resume sequence

2010-11-22 Thread Kevin Hilman
jean.pi...@newoldbits.com writes:

> From: Jean Pihet 
>
> Idle path should be disabled during the entire suspend/resume sequence.
> Currently it is disabled in ->prepare() and re-enabled in ->finish(),
> but the suspend sequence starts with ->begin() and ends with ->end(),
> leaving windows where the suspend/resume sequence is still underway and
> idle code could execute.
>
> The fix is to move the idle disable (disable_hlt) and enable (enable_hlt)
> into ->begin() and ->end() respectively to ensure that the idle path
> is disabled for the entire suspend/resume sequence.
> Remvoving omap3_pm_prepare and omap3_pm_finish since there are now empty.
>
> Tested with RET and OFF on Beagle and OMAP3EVM.
>
> Signed-off-by: Jean Pihet 
>
> Comment rework suggested by Kevin.
>
> Cc: Kevin Hilman 

Thanks, 

Unless there are any objections, I'll queue for .38 instead of .37-rc
since this isn't technically a regression.

Kevin

> ---
>  arch/arm/mach-omap2/pm34xx.c |   15 ++-
>  1 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 75c0cd1..2e0621e 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -506,12 +506,6 @@ out:
>  #ifdef CONFIG_SUSPEND
>  static suspend_state_t suspend_state;
>  
> -static int omap3_pm_prepare(void)
> -{
> - disable_hlt();
> - return 0;
> -}
> -
>  static int omap3_pm_suspend(void)
>  {
>   struct power_state *pwrst;
> @@ -574,14 +568,10 @@ static int omap3_pm_enter(suspend_state_t unused)
>   return ret;
>  }
>  
> -static void omap3_pm_finish(void)
> -{
> - enable_hlt();
> -}
> -
>  /* Hooks to enable / disable UART interrupts during suspend */
>  static int omap3_pm_begin(suspend_state_t state)
>  {
> + disable_hlt();
>   suspend_state = state;
>   omap_uart_enable_irqs(0);
>   return 0;
> @@ -591,15 +581,14 @@ static void omap3_pm_end(void)
>  {
>   suspend_state = PM_SUSPEND_ON;
>   omap_uart_enable_irqs(1);
> + enable_hlt();
>   return;
>  }
>  
>  static struct platform_suspend_ops omap_pm_ops = {
>   .begin  = omap3_pm_begin,
>   .end= omap3_pm_end,
> - .prepare= omap3_pm_prepare,
>   .enter  = omap3_pm_enter,
> - .finish = omap3_pm_finish,
>   .valid  = suspend_valid_only_mem,
>  };
>  #endif /* CONFIG_SUSPEND */
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] OMAP3: disable idle during the entire suspend/resume sequence

2010-11-22 Thread jean . pihet
From: Jean Pihet 

Idle path should be disabled during the entire suspend/resume sequence.
Currently it is disabled in ->prepare() and re-enabled in ->finish(),
but the suspend sequence starts with ->begin() and ends with ->end(),
leaving windows where the suspend/resume sequence is still underway and
idle code could execute.

The fix is to move the idle disable (disable_hlt) and enable (enable_hlt)
into ->begin() and ->end() respectively to ensure that the idle path
is disabled for the entire suspend/resume sequence.
Remvoving omap3_pm_prepare and omap3_pm_finish since there are now empty.

Tested with RET and OFF on Beagle and OMAP3EVM.

Signed-off-by: Jean Pihet 

Comment rework suggested by Kevin.

Cc: Kevin Hilman 
---
 arch/arm/mach-omap2/pm34xx.c |   15 ++-
 1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 75c0cd1..2e0621e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -506,12 +506,6 @@ out:
 #ifdef CONFIG_SUSPEND
 static suspend_state_t suspend_state;
 
-static int omap3_pm_prepare(void)
-{
-   disable_hlt();
-   return 0;
-}
-
 static int omap3_pm_suspend(void)
 {
struct power_state *pwrst;
@@ -574,14 +568,10 @@ static int omap3_pm_enter(suspend_state_t unused)
return ret;
 }
 
-static void omap3_pm_finish(void)
-{
-   enable_hlt();
-}
-
 /* Hooks to enable / disable UART interrupts during suspend */
 static int omap3_pm_begin(suspend_state_t state)
 {
+   disable_hlt();
suspend_state = state;
omap_uart_enable_irqs(0);
return 0;
@@ -591,15 +581,14 @@ static void omap3_pm_end(void)
 {
suspend_state = PM_SUSPEND_ON;
omap_uart_enable_irqs(1);
+   enable_hlt();
return;
 }
 
 static struct platform_suspend_ops omap_pm_ops = {
.begin  = omap3_pm_begin,
.end= omap3_pm_end,
-   .prepare= omap3_pm_prepare,
.enter  = omap3_pm_enter,
-   .finish = omap3_pm_finish,
.valid  = suspend_valid_only_mem,
 };
 #endif /* CONFIG_SUSPEND */
-- 
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html