[BUG, REGRESSION] ACPI: Set _PDC

[BUG, REGRESSION] ACPI: Set _PDC

[Jan Pogadl]

Hi,

with commit commit 5d554a7bb0643a6151a84319bfeba8270bf5269e (ACPI: processor: 
add internal processor_physically_present()) a new function got introduced which 
always fails on machines with CONFIG_SMP disabled. Thus _PDC will never be 
initialized? set?. 

The problem of the processor_physically_present() is, that it calls the 
acpi_get_cpuid(...) function which returns on NON-SMP machines always -1.

Note that i don't have any real knowledge what _PDC actualy realy do, all i 
gatherd so far is that its for switching C-Stats. I Stumbled across this by 
bisecting the problem, that the undervolting patches from
linux-phc.org aren't working anymore with 2.6.34 (2.6.35-rc3). Whereas they 
worked fine with 2.6.33 on my  Samsung X20 Notebook with a Pentium M precossor. 
The acpi-cpufreq is now using System-IO instead of MSR Registers. 

Since i don't know what should be the correct behaviour here i can't
provide any patches. But i doub't it's the correct behaviour of 
processor_physically_present() to always fail on non-SMP machines. 
 
Deleting the function call, to this new function in early_init_pdc(...), 
and i get back the old behaviour.

cheers,
jan

Re: [BUG, REGRESSION] ACPI: Set _PDC

[Alex Chiang]

Hi Jan,

* Jan Pogadl <pogadl.jan@googlemail.com>:
> Hi,
> 
> with commit commit 5d554a7bb0643a6151a84319bfeba8270bf5269e (ACPI: processor: 
> add internal processor_physically_present()) a new function got introduced which 
> always fails on machines with CONFIG_SMP disabled. Thus _PDC will never be 
> initialized? set?. 

My apologies for the regression and the slow response. Please
note my new email address.

Does this patch fix it?

---
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 5128435..bcf0cf8 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -223,7 +223,7 @@ static bool processor_physically_present(acpi_handle handle)
 	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
 	cpuid = acpi_get_cpuid(handle, type, acpi_id);
 
-	if (cpuid == -1)
+	if ((cpuid == -1) && (num_online_cpus() > 1))
 		return false;
 
 	return true;

Re: [BUG, REGRESSION] ACPI: Set _PDC

[Jan Pogadl]

On Thursday 17 June 2010 05:24:14 Alex Chiang wrote:

> > with commit commit 5d554a7bb0643a6151a84319bfeba8270bf5269e (ACPI: processor: 
> > add internal processor_physically_present()) a new function got introduced which 
> > always fails on machines with CONFIG_SMP disabled. Thus _PDC will never be 
> > initialized? set?. 
 
> Does this patch fix it?

works great 
thanks

> ---
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index 5128435..bcf0cf8 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -223,7 +223,7 @@ static bool processor_physically_present(acpi_handle handle)
>  	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
>  	cpuid = acpi_get_cpuid(handle, type, acpi_id);
>  
> -	if (cpuid == -1)
> +	if ((cpuid == -1) && (num_online_cpus() > 1))
>  		return false;
>  
>  	return true;
> 

Re: [BUG, REGRESSION] ACPI: Set _PDC

[Chen Gong]

于 6/17/2010 3:51 PM, Jan Pogadl 写道:
> On Thursday 17 June 2010 05:24:14 Alex Chiang wrote:
>
>>> with commit commit 5d554a7bb0643a6151a84319bfeba8270bf5269e (ACPI: processor:
>>> add internal processor_physically_present()) a new function got introduced which
>>> always fails on machines with CONFIG_SMP disabled. Thus _PDC will never be
>>> initialized? set?.
>
>> Does this patch fix it?
>
> works great
> thanks
>
>> ---
>> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
>> index 5128435..bcf0cf8 100644
>> --- a/drivers/acpi/processor_core.c
>> +++ b/drivers/acpi/processor_core.c
>> @@ -223,7 +223,7 @@ static bool processor_physically_present(acpi_handle handle)
>>   	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
>>   	cpuid = acpi_get_cpuid(handle, type, acpi_id);
>>
>> -	if (cpuid == -1)
>> +	if ((cpuid == -1)&&  (num_online_cpus()>  1))
>>   		return false;
>>
>>   	return true;
>>
I have a puzzle why num_online_cpus is used here, instead of
num_possible_cpus. It will be possible under a hotplug scenario.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG, REGRESSION] ACPI: Set _PDC

[Alex Chiang]

* Chen Gong <gong.chen@linux.intel.com>:
> >On Thursday 17 June 2010 05:24:14 Alex Chiang wrote:
> >>diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> >>index 5128435..bcf0cf8 100644
> >>--- a/drivers/acpi/processor_core.c
> >>+++ b/drivers/acpi/processor_core.c
> >>@@ -223,7 +223,7 @@ static bool processor_physically_present(acpi_handle handle)
> >>  	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
> >>  	cpuid = acpi_get_cpuid(handle, type, acpi_id);
> >>
> >>-	if (cpuid == -1)
> >>+	if ((cpuid == -1)&&  (num_online_cpus()>  1))
> >>  		return false;
> >>
> >>  	return true;
> >>
> I have a puzzle why num_online_cpus is used here, instead of
> num_possible_cpus. It will be possible under a hotplug scenario.

Hm, you're probably right.

I was following the example of the other usage of
acpi_get_cpuid() in acpi_processor_get_info(), but based on your
observation, it's probably better to use num_possible_cpus.

I'll change it when I submit a proper patch.

Thanks for the review.

/ac