public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Dave Jones <davej@redhat.com>
Cc: Marcin Slusarz <marcin.slusarz@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux PM List <linux-pm@lists.linux-foundation.org>,
	cpufreq@vger.kernel.org
Subject: Re: 2.6.31-rc2+: Interrupts enabled after cpufreq_suspend
Date: Thu, 16 Jul 2009 13:10:47 +1000	[thread overview]
Message-ID: <1247713847.27937.14.camel@pasglop> (raw)
In-Reply-To: <20090710234646.GA11669@redhat.com>

On Fri, 2009-07-10 at 19:46 -0400, Dave Jones wrote:

> The fail part comes from the fact that interrupts get reenabled.
> And that's something that can easily happen out of our control if
> we call into acpi_cpufreq's ->get method for example, so powernow-k8 isn't
> the sole reason.
> 
>  > I'm happy instead of #ifdef's however to push the logic into the ppc
>  > driver, or use a flag that the ppc driver sets to enable that logic.

So I'm not too familiar with what other platforms expect here, but would
it be ok to do something like bailing our early in cpufreq_suspend() if
there's no cpufreq_driver->suspend method ?

We would need to do the same on resume, though I do wonder whether there
could be a problem with even x86 here.. do we know for sure that we come
back from suspend with the same policy we had before suspending ? IE. We
should probably always do that check on resume, but we can't do it too
early due to irqs being off...

It's all because cpufreq is a sysdev which was imho a mistake in the
first place :-)

Maybe at this stage the best option is to add suspend_prepare and
resume_finish callbacks to sysdevs that are called while IRQs are still
on, and move cpufreq to use these instead ? That probably means we also
need to keep track that we have started suspend in the cpufreq core and
forbid attempts at changing policy etc... from either userspace of
on-demand governor while suspended, so that's more work, but could be
the best approach in the long run, what do you think ?

Otherwise I'm happy to just submit a patch that makes
cpufreq_suspend/resume bail out early if driver->suspend/resume are NULL
(or do you prefer a driver flag ?)

Cheers,
Ben.




      reply	other threads:[~2009-07-16  3:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-09 15:44 2.6.31-rc2+: Interrupts enabled after cpufreq_suspend Marcin Slusarz
2009-07-10 19:25 ` Dave Jones
2009-07-10 20:13   ` Dave Jones
2009-07-11 14:33     ` Marcin Slusarz
2009-07-10 22:23   ` Benjamin Herrenschmidt
2009-07-10 23:46     ` Dave Jones
2009-07-16  3:10       ` Benjamin Herrenschmidt [this message]

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=1247713847.27937.14.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=cpufreq@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=marcin.slusarz@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox