From: Arnd Bergmann <arnd@arndb•de>
To: kvm-ppc-devel@lists•sourceforge.net, jyoung5@us•ibm.com
Cc: linuxppc-dev@ozlabs•org, Hollis Blanchard <hollisb@us•ibm.com>
Subject: Re: [kvm-ppc-devel] [PATCH] Add idle power save for ppc 4xx
Date: Tue, 1 Apr 2008 14:14:07 +0200 [thread overview]
Message-ID: <200804011414.07549.arnd@arndb.de> (raw)
In-Reply-To: <1206982322.9165.7.camel@thinkpadL>
On Monday 31 March 2008, Jerone Young wrote:
> >=20
> > > +{
> > > +=A0=A0=A0unsigned long msr_save;
> > > +
> > > +=A0=A0=A0/* set wait state MSR */
> > > +=A0=A0=A0local_irq_enable();
> > > +=A0=A0=A0msr_save =3D mfmsr();
> > > +=A0=A0=A0mtmsr(msr_save|MSR_WE);
> >=20
> > Why don't you |MSR_WE|MSR_EE at the same time?
>=20
> You technically can do this. But the question is do all 4xx cpus use
> MSR_EE to enable interrupts? I can assume they do (from what I know),
> but figured it would be safer to make the local_irq_enable() call.
> I can change it to just set the MSR_EE bit though, since all 4xx cpus
> (as far as I know) use it.
=46or correctness reasons, you actually have to set both EE and WE
simultaneously. Otherwise, an interrupt can come in between the two
mtmsr(), mark some thread as runnable and then go to sleep here
without ever checking need_resched() until the next interrupt,
which may take an indefinite time.
> >=20
> > > +=A0=A0=A0local_irq_disable();
> > > +}
> >=20
> > None of the other power_save() implementations need this. In fact many
> > of them don't even seem to return; they just loop around mtmsr.
>=20
> Sure it can be removed. Though with the comment in the mach_dep
> structure about power_save. It specifically says that interrupts are off
> when it is called. So I was following it here mainly. But I can remove
> the disabling of interrupts, since mtmsr is the only used under
> power_save.
With the current code, it shouldn't matter, because cpu_idle enables
the interrupts right after calling ppc_md->power_save(), but it would
be reasonable to disable the interrupts here anyway, if only to make
the function return with the same state that it was entered.
IMHO, the function should be
void ppc4xx_idle()
{
unsigned long msr_save;
msr_save =3D mfmsr();
/* enter idle mode */
mtmsr(msr_save|MSR_WE|MSR_EE);
/* disable interrupts again */
mtmsr(msr_save);
}
Arnd <><
next prev parent reply other threads:[~2008-04-01 12:14 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-31 13:12 [PATCH] Add idle power save for ppc 4xx Jerone Young
2008-03-31 16:27 ` [kvm-ppc-devel] " Hollis Blanchard
2008-03-31 16:52 ` Jerone Young
2008-03-31 17:48 ` Josh Boyer
2008-04-01 12:14 ` Arnd Bergmann [this message]
2008-03-31 17:07 ` Josh Boyer
2008-03-31 18:05 ` Josh Boyer
2008-03-31 18:19 ` Jerone Young
2008-04-01 1:04 ` Michael Ellerman
2008-04-01 3:15 ` Josh Boyer
2008-04-01 3:30 ` Michael Ellerman
2008-04-01 12:01 ` [kvm-ppc-devel] " Jimi Xenidis
2008-04-01 12:03 ` Josh Boyer
2008-03-31 19:24 ` Hollis Blanchard
2008-03-31 20:28 ` Josh Boyer
2008-03-31 20:34 ` Hollis Blanchard
2008-04-01 4:00 ` Paul Mackerras
2008-04-01 11:03 ` Josh Boyer
2008-03-31 18:23 ` Jerone Young
2008-03-31 19:11 ` Josh Boyer
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=200804011414.07549.arnd@arndb.de \
--to=arnd@arndb$(echo .)de \
--cc=hollisb@us$(echo .)ibm.com \
--cc=jyoung5@us$(echo .)ibm.com \
--cc=kvm-ppc-devel@lists$(echo .)sourceforge.net \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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