All of lore.kernel.org
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: buggy usage of pm_idle by omap1 code
Date: Mon, 19 Dec 2011 21:34:45 +0000	[thread overview]
Message-ID: <20111219213445.GL14542@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20111219173518.GH6464@atomide.com>

On Mon, Dec 19, 2011 at 09:35:19AM -0800, Tony Lindgren wrote:
> Good catch. Here's a patch to deal with that issue, want to take this
> into your idle series? I'll try out your idle series today at some
> point.

Two points:

1. It's good practice to call cpu_idle_wait() after modifying pm_idle,
even on UP - it allows static checking of this.

> From: Tony Lindgren <tony@atomide.com>
> Date: Mon, 19 Dec 2011 09:11:11 -0800
> Subject: [PATCH] ARM: OMAP1: Fix pm_idle during suspend
> 
> Commit 9ccdac3662dbf3c75e8f8851a214bdf7d365a4bd ([ARM] idle:
> clean up pm_idle calling, obey hlt_counter) removed a check
> for NULL pm_idle.
> 
> On omap1 the system hits an equivalent of suspend during idle.
> If we are suspending, we need to make sure so we don't call
> omap1_pm_idle during the suspend process.
> 
> Fix this by setting the pm_idle function to a an empty
> function while suspending.

2. what's wrong with calling disable_hlt() to prevent the pm_idle function
being called?  Why solve it this complicated way of changing the pm_idle
pointer when there's some infrastructure which can fix it for you already?

> 
> Reported-by: Nicolas Pitre <nico@fluxnic.net>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> diff --git a/arch/arm/mach-omap1/pm.c b/arch/arm/mach-omap1/pm.c
> index 89ea20c..b551a62 100644
> --- a/arch/arm/mach-omap1/pm.c
> +++ b/arch/arm/mach-omap1/pm.c
> @@ -584,6 +584,9 @@ static void omap_pm_init_proc(void)
>  #endif /* DEBUG && CONFIG_PROC_FS */
>  
>  static void (*saved_idle)(void) = NULL;
> +static void omap1_dummy_idle(void)
> +{
> +}
>  
>  /*
>   *	omap_pm_prepare - Do preliminary suspend work.
> @@ -593,7 +596,7 @@ static int omap_pm_prepare(void)
>  {
>  	/* We cannot sleep in idle until we have resumed */
>  	saved_idle = pm_idle;
> -	pm_idle = NULL;
> +	pm_idle = omap1_dummy_idle;
>  
>  	return 0;
>  }
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2011-12-19 21:34 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-17  3:24 buggy usage of pm_idle by omap1 code Nicolas Pitre
2011-12-19 17:35 ` Tony Lindgren
2011-12-19 17:44   ` Nicolas Pitre
2011-12-19 21:34   ` Russell King - ARM Linux [this message]
2011-12-19 23:04     ` Nicolas Pitre
2011-12-19 23:22       ` Tony Lindgren
2011-12-19 23:20     ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111219213445.GL14542@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.