RE: [PATCH] ACPI: Move timer broadcast and pmtimer access before C3 arbiter shutdown

[Pallipadi, Venkatesh]

>-----Original Message-----
>From: Andrew Morton [mailto:akpm@xxxxxxxxxxxxxxxxxxxx] 
>Sent: Wednesday, June 06, 2007 6:39 PM
>To: Thomas Gleixner
>Cc: Pallipadi, Venkatesh; Stable Team; LKML; Len Brown; Ingo 
>Molnar; Arjan van de Ven; Andi Kleen; Udo A. Steinberg; Dave Jones
>Subject: Re: [PATCH] ACPI: Move timer broadcast and pmtimer 
>access before C3 arbiter shutdown
>
>On Wed, 06 Jun 2007 11:37:53 +0200 Thomas Gleixner 
><tglx@xxxxxxxxxxxxx> wrote:
>
>> From: Udo A. Steinberg <us15@xxxxxxxxxxxxxxxxxxxx>
>> 
>> The chip set doc for IHC4 says:
>> 
>> 1.In general, software should not attempt any non-posted 
>accesses during
>> arbiter disable except to the ICH4's power management registers. This
>> implies that interrupt handlers for any unmasked hardware 
>interrupts and
>> SMI/NMI should check ARB_DIS status before reading from ICH devices.
>> 
>> So it's not a good idea to access ICH devices after arbiter 
>shut down. 
>> 
>> Signed-off-by: Udo A. Steinberg <us15@xxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> 
>> ---
>>  drivers/acpi/processor_idle.c |    9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> Index: linux-2.6.22-rc4/drivers/acpi/processor_idle.c
>> ===================================================================
>> --- linux-2.6.22-rc4.orig/drivers/acpi/processor_idle.c	
>2007-06-06 11:47:21.000000000 +0200
>> +++ linux-2.6.22-rc4/drivers/acpi/processor_idle.c	
>2007-06-06 11:48:21.000000000 +0200
>> @@ -488,6 +488,11 @@ static void acpi_processor_idle(void)
>>  
>>  	case ACPI_STATE_C3:
>>  
>> +		/* Get start time (ticks) */
>> +		t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>> +		/* Handle timer broadcast before bus arbiter 
>shutdown ! */
>> +		acpi_state_timer_broadcast(pr, cx, 1);
>> +
>>  		if (pr->flags.bm_check) {
>>  			if (atomic_inc_return(&c3_cpu_count) ==
>>  			    num_online_cpus()) {
>> @@ -502,10 +507,7 @@ static void acpi_processor_idle(void)
>>  			ACPI_FLUSH_CPU_CACHE();
>>  		}
>>  
>> -		/* Get start time (ticks) */
>> -		t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>>  		/* Invoke C3 */
>> -		acpi_state_timer_broadcast(pr, cx, 1);
>>  		acpi_cstate_enter(cx);
>>  		/* Get end time (ticks) */
>>  		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
>
>hm, this needs a bit of help to get it to work against Len's 
>current tree.
>
>However, if by "non-posted accesses" you're referring to that 
>inl(), how
>come the second one which was left in place isn't also a problem?
>

The doc says "except to the ICH4's power management registers".
It seems acpi_gbl_FADT.xpm_timer_block.address is actually OK in this
case
as it is ACPI PM timer register.
The problem we had is the access to HPET registers
inside acpi_state_timer_broadcast() and that is the one that has to be
done
before ARB_DIS.

Thanks,
Venki
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/