[PATCH ] cell: enable pause(0) in cpu_idle

Milton Miller miltonm at bga.com
Fri Dec 16 02:26:34 EST 2005


On Dec 15, 2005, at 12:43 AM, Paul Mackerras wrote:

> Arnd Bergmann writes:
>
>> This patch enables support for pause(0) power management state
>> for the Cell Broadband Processor, which is import for power efficient
>> operation. The pervasive infrastructure will in the future enable
>> us to introduce more functionality specific to the Cell's
>> pervasive unit.
>
> I put this in, but then took a closer look at this and reverted the
> commit:
>
>> +/* This is a new system reset handler for the BE processor.
>> + * SRR1 stores wake information that must be decoded to determine why
>> + * the processor was at the system reset handler.
>> + */
>> +
>> +	.align 7
>> +	.globl system_reset_check_common
>> +system_reset_check_common:
>> +BEGIN_FTR_SECTION
>> +	mr	r22,r12    /* r12 has SRR1 saved */
>> +	srwi	r22,r22,16
>> +	andi.	r22,r22,MSR_WAKEMASK
>> +	cmpwi	r22,MSR_WAKEEE
>> +	beq	hardware_interrupt_common
>> +	cmpwi	r22,MSR_WAKEDEC
>> +	beq	decrementer_common
>> +	cmpwi	r22,MSR_WAKEMT
>> +	bne	system_reset_common
>
> Why are you trashing r22 here?  It hasn't been saved.  You probably
> should use r10 instead, which has been saved.

I'm just guessing here that they missed the update on

http://git.kernel.org/git/?p=linux/kernel/git/torvalds/old-2.6- 
bkcvs.git;a=commitdiff;h=d847914c00b6cef7ad40816a192828cc55fee30d
[PATCH] ppc64: Optimize exception/syscall entry/exit

and I missed it because I just looked at the patch and have looked at  
the old code too much.

>
>> +END_FTR_SECTION_IFSET(CPU_FTR_PAUSE_ZERO)
>> +	EXCEPTION_PROLOG_COMMON(0x100, PACA_EXGEN);
>> +	b	fast_exception_return
>
> This is a change in behaviour from before the patch.  Instead of doing
> the EXCEPTION_PROLOG_COMMON and then just returning, you should branch
> to system_reset_common.
>

Actually, this is not the case, as this is only for the MSR_WAKEMT
case -- they put a bne to the old handler.


I'll raise the question from my first review again: would you rather
see the STD_EXCEPTION_COMMON macro split for system_reset_common or  
leave this extra branch to the second handler?

I didn't push because this exception should be rare for most processors.

milton




More information about the Linuxppc64-dev mailing list