Chenging 2 bits in MSR in ppc6xx_idle() with 1 command?

[Guennadi Liakhovetski]

On Wed, 27 Dec 2006, Benjamin Herrenschmidt wrote:

> On Mon, 2006-12-25 at 21:07 +0100, Guennadi Liakhovetski wrote:
> > 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 at 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