On Wed, 27 Dec 2006, Benjamin Herrenschmidt wrote:

On Mon, 2006-12-25 at 21:07 +0100, Guennadi Liakhovetski wrote:

quoted

Hi

Here's a code fragment from ppc6xx_idle(), which should send the CPU into 
a powersaving mode (DOZE or NAP) and re-enable interrupts after a 
local_irq_disable():

	mfmsr	r7
	ori	r7,r7,MSR_EE
	oris	r7,r7,MSR_POW@h
1:	sync
	mtmsr	r7
	isync
	b	1b

Whereas MPC8245's user manual says, that when setting the MSR_POW bit in 
the MSR one may not set any other bit in it with the same instruction. 
Does this mean that the above does not actually work on those (and 
similar) CPUs or does it work because of the loop?

That doc bit looks a bit strange. The kernel pretty much relies on
setting MSR:EE and MSR:POW atomicaly.

Hm, wouldn't it just work? In ppc6xx_idle() the _TLF_NAPPING bit is set. 
If as a result of mtmsr only the EE bit is set and we get an interrupt, we 
end up in transfer_to_handler(), check the flag, it is set, so we branch 
to power_save_6xx_restore(). There we clear NAP/DOZE and just jump to 
transfer_to_handler_cont(). Why did you say we'd miss the check for 
need_resched (on IRC)? How is this case difference from if we really did 
go to NAP / DOZE?

Are there other places in the kernel where we rely on setting MSR:POW and 
some other bit atomically?

Thanks
Guennadi
---
Guennadi Liakhovetski