[PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

When bios switch to use Device object instead of Processor statement.

the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.

So try to scan ACPI0007 in addition to Processor.

this fix regression: 2.6.32 is ok.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/processor_core.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_core.c
+++ linux-2.6/drivers/acpi/processor_core.c
@@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc
 	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
 			    ACPI_UINT32_MAX,
 			    early_init_pdc, NULL, NULL, NULL);
+	acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
 }

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Bjorn Helgaas]

On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote:
> 
> When bios switch to use Device object instead of Processor statement.
> 
> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.
> 
> So try to scan ACPI0007 in addition to Processor.
> 
> this fix regression: 2.6.32 is ok.

Can you include the URL of the regression bug report?  And maybe
the commit that introduced the regression?

> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/acpi/processor_core.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux-2.6/drivers/acpi/processor_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_core.c
> +++ linux-2.6/drivers/acpi/processor_core.c
> @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc
>  	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>  			    ACPI_UINT32_MAX,
>  			    early_init_pdc, NULL, NULL, NULL);
> +	acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);

I hate having to walk the namespace.  Usually that's a clue that
there's something wrong with our ACPI device model, because it'd
be better to handle everything in a driver .add() method.  But
maybe this early _PDC thing is so special that it can't be helped
in this case.

But I do think you could probably fix this to walk the namespace
once rather than twice.  Maybe you could use something like
acpi_walk_namespace(ACPI_TYPE_ANY, ...) with a callback that
recognizes both ACPI_TYPE_PROCESSOR and "ACPI_TYPE_DEVICE with
HID ACPI0007".

Bjorn

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

On 09/10/2010 11:10 AM, Bjorn Helgaas wrote:
> On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote:
>>
>> When bios switch to use Device object instead of Processor statement.
>>
>> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.
>>
>> So try to scan ACPI0007 in addition to Processor.
>>
>> this fix regression: 2.6.32 is ok.
> 
> Can you include the URL of the regression bug report?  And maybe
> the commit that introduced the regression?

the commit should be

commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
Author: Alex Chiang <achiang@hp.com>
Date:   Mon Feb 22 12:11:39 2010 -0700

    ACPI: processor: driver doesn't need to evaluate _PDC
    
    Now that the early _PDC evaluation path knows how to correctly
    evaluate _PDC on only physically present processors, there's no
    need for the processor driver to evaluate it later when it loads.
    
    To cover the hotplug case, push _PDC evaluation down into the
    hotplug paths.
    
    Cc: x86@kernel.org
    Cc: Tony Luck <tony.luck@intel.com>
    Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
    Signed-off-by: Alex Chiang <achiang@hp.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

that is between 2.6.34-rc1 and 2.6.34-rc2.

So we need put this patch in stable tree for 2.6.34, .35, .36

Yinghai

> 
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> ---
>>  drivers/acpi/processor_core.c |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> Index: linux-2.6/drivers/acpi/processor_core.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/processor_core.c
>> +++ linux-2.6/drivers/acpi/processor_core.c
>> @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc
>>  	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>>  			    ACPI_UINT32_MAX,
>>  			    early_init_pdc, NULL, NULL, NULL);
>> +	acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
> 
> I hate having to walk the namespace.  Usually that's a clue that
> there's something wrong with our ACPI device model, because it'd
> be better to handle everything in a driver .add() method.  But
> maybe this early _PDC thing is so special that it can't be helped
> in this case.
> 
> But I do think you could probably fix this to walk the namespace
> once rather than twice.  Maybe you could use something like
> acpi_walk_namespace(ACPI_TYPE_ANY, ...) with a callback that
> recognizes both ACPI_TYPE_PROCESSOR and "ACPI_TYPE_DEVICE with
> HID ACPI0007".
> 
> Bjorn

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Andrew Morton]

On Fri, 10 Sep 2010 12:20:36 -0700
Yinghai Lu <yinghai@kernel.org> wrote:

> On 09/10/2010 11:10 AM, Bjorn Helgaas wrote:
> > On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote:
> >>
> >> When bios switch to use Device object instead of Processor statement.
> >>
> >> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.
> >>
> >> So try to scan ACPI0007 in addition to Processor.
> >>
> >> this fix regression: 2.6.32 is ok.
> > 
> > Can you include the URL of the regression bug report?  And maybe
> > the commit that introduced the regression?
> 
> the commit should be
> 
> commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
> Author: Alex Chiang <achiang@hp.com>
> Date:   Mon Feb 22 12:11:39 2010 -0700
> 
>     ACPI: processor: driver doesn't need to evaluate _PDC
>     
>     Now that the early _PDC evaluation path knows how to correctly
>     evaluate _PDC on only physically present processors, there's no
>     need for the processor driver to evaluate it later when it loads.
>     
>     To cover the hotplug case, push _PDC evaluation down into the
>     hotplug paths.
>     
>     Cc: x86@kernel.org
>     Cc: Tony Luck <tony.luck@intel.com>
>     Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>     Signed-off-by: Alex Chiang <achiang@hp.com>
>     Signed-off-by: Len Brown <len.brown@intel.com>
> 
> that is between 2.6.34-rc1 and 2.6.34-rc2.
> 
> So we need put this patch in stable tree for 2.6.34, .35, .36
> 

Maybe.  But first can you please address Bjorn's suggestions below?

> 
> > 
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >>
> >> ---
> >>  drivers/acpi/processor_core.c |    1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> Index: linux-2.6/drivers/acpi/processor_core.c
> >> ===================================================================
> >> --- linux-2.6.orig/drivers/acpi/processor_core.c
> >> +++ linux-2.6/drivers/acpi/processor_core.c
> >> @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc
> >>  	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> >>  			    ACPI_UINT32_MAX,
> >>  			    early_init_pdc, NULL, NULL, NULL);
> >> +	acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
> > 
> > I hate having to walk the namespace.  Usually that's a clue that
> > there's something wrong with our ACPI device model, because it'd
> > be better to handle everything in a driver .add() method.  But
> > maybe this early _PDC thing is so special that it can't be helped
> > in this case.
> > 
> > But I do think you could probably fix this to walk the namespace
> > once rather than twice.  Maybe you could use something like
> > acpi_walk_namespace(ACPI_TYPE_ANY, ...) with a callback that
> > recognizes both ACPI_TYPE_PROCESSOR and "ACPI_TYPE_DEVICE with
> > HID ACPI0007".
> > 
> > Bjorn

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

On 09/10/2010 04:45 PM, Andrew Morton wrote:
> On Fri, 10 Sep 2010 12:20:36 -0700
> Yinghai Lu <yinghai@kernel.org> wrote:
> 
>> On 09/10/2010 11:10 AM, Bjorn Helgaas wrote:
>>> On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote:
>>>>
>>>> When bios switch to use Device object instead of Processor statement.
>>>>
>>>> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.
>>>>
>>>> So try to scan ACPI0007 in addition to Processor.
>>>>
>>>> this fix regression: 2.6.32 is ok.
>>>
>>> Can you include the URL of the regression bug report?  And maybe
>>> the commit that introduced the regression?
>>
>> the commit should be
>>
>> commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
>> Author: Alex Chiang <achiang@hp.com>
>> Date:   Mon Feb 22 12:11:39 2010 -0700
>>
>>     ACPI: processor: driver doesn't need to evaluate _PDC
>>     
>>     Now that the early _PDC evaluation path knows how to correctly
>>     evaluate _PDC on only physically present processors, there's no
>>     need for the processor driver to evaluate it later when it loads.
>>     
>>     To cover the hotplug case, push _PDC evaluation down into the
>>     hotplug paths.
>>     
>>     Cc: x86@kernel.org
>>     Cc: Tony Luck <tony.luck@intel.com>
>>     Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
>>     Signed-off-by: Alex Chiang <achiang@hp.com>
>>     Signed-off-by: Len Brown <len.brown@intel.com>
>>
>> that is between 2.6.34-rc1 and 2.6.34-rc2.
>>
>> So we need put this patch in stable tree for 2.6.34, .35, .36
>>
> 
> Maybe.  But first can you please address Bjorn's suggestions below?

fine, if you prefer 47 lines instead one line changes.

[PATCH -v2] acpi: handle ACPI0007 Device in acpi_early_set_pdc

When bios switch to use Device object instead of Processor statement.

the SSDT for Pstate/Cstate/Tstate can not loaded dynamically.

So try to scan ACPI0007 in addition to Processor.

-v2: add 47 lines instead of 1 line to walk namespace one time only.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/acpica/nsxfeval.c |   43 +++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/processor_core.c  |    8 ++++---
 include/acpi/acpixf.h          |    2 +
 3 files changed, 50 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_core.c
+++ linux-2.6/drivers/acpi/processor_core.c
@@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 #endif
 
-static bool processor_physically_present(acpi_handle handle)
+static bool is_processor_and_physically_present(acpi_handle handle)
 {
 	int cpuid, type;
 	u32 acpi_id;
@@ -211,6 +211,8 @@ static bool processor_physically_present
 		acpi_id = object.processor.proc_id;
 		break;
 	case ACPI_TYPE_DEVICE:
+		if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK)
+			return false;
 		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
 		if (ACPI_FAILURE(status))
 			return false;
@@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc
 static acpi_status
 early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
-	if (processor_physically_present(handle) == false)
+	if (is_processor_and_physically_present(handle) == false)
 		return AE_OK;
 
 	acpi_processor_set_pdc(handle);
@@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc
 	 */
 	dmi_check_system(processor_idle_dmi_table);
 
-	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+	acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT,
 			    ACPI_UINT32_MAX,
 			    early_init_pdc, NULL, NULL, NULL);
 }
Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c
+++ linux-2.6/drivers/acpi/acpica/nsxfeval.c
@@ -524,6 +524,49 @@ ACPI_EXPORT_SYMBOL(acpi_walk_namespace)
 
 /*******************************************************************************
  *
+ * FUNCTION:    acpi_match_device_hid
+ *
+ * PARAMETERS:  handle and HID to be compared
+ *
+ * RETURN:      bool
+ *
+ * DESCRIPTION: check if device coresponding to handle has HID
+ *
+ ******************************************************************************/
+acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID)
+{
+	acpi_status status;
+	struct acpi_namespace_node *node;
+	struct acpica_device_id *hid;
+	int no_match;
+
+	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	node = acpi_ns_validate_handle(obj_handle);
+	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	if (!node)
+		return AE_BAD_PARAMETER;
+
+	status = acpi_ut_execute_HID(node, &hid);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	no_match = ACPI_STRCMP(hid->string, HID);
+	ACPI_FREE(hid);
+
+	if (no_match)
+		return AE_NOT_FOUND;
+
+	return AE_OK;
+}
+
+/*******************************************************************************
+ *
  * FUNCTION:    acpi_ns_get_device_callback
  *
  * PARAMETERS:  Callback from acpi_get_device
Index: linux-2.6/include/acpi/acpixf.h
===================================================================
--- linux-2.6.orig/include/acpi/acpixf.h
+++ linux-2.6/include/acpi/acpixf.h
@@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ
 		    acpi_walk_callback post_order_visit,
 		    void *context, void **return_value);
 
+acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID);
+
 acpi_status
 acpi_get_devices(const char *HID,
 		 acpi_walk_callback user_function,

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Bjorn Helgaas]

On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote:
> On 09/10/2010 04:45 PM, Andrew Morton wrote:
> > On Fri, 10 Sep 2010 12:20:36 -0700
> > Yinghai Lu <yinghai@kernel.org> wrote:
> > 
> >> On 09/10/2010 11:10 AM, Bjorn Helgaas wrote:
> >>> On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote:
> >>>>
> >>>> When bios switch to use Device object instead of Processor statement.
> >>>>
> >>>> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically.
> >>>>
> >>>> So try to scan ACPI0007 in addition to Processor.
> >>>>
> >>>> this fix regression: 2.6.32 is ok.
> >>>
> >>> Can you include the URL of the regression bug report?  And maybe
> >>> the commit that introduced the regression?
> >>
> >> the commit should be
> >>
> >> commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
> >> Author: Alex Chiang <achiang@hp.com>
> >> Date:   Mon Feb 22 12:11:39 2010 -0700
> >>
> >>     ACPI: processor: driver doesn't need to evaluate _PDC
> >>     
> >>     Now that the early _PDC evaluation path knows how to correctly
> >>     evaluate _PDC on only physically present processors, there's no
> >>     need for the processor driver to evaluate it later when it loads.
> >>     
> >>     To cover the hotplug case, push _PDC evaluation down into the
> >>     hotplug paths.
> >>     
> >>     Cc: x86@kernel.org
> >>     Cc: Tony Luck <tony.luck@intel.com>
> >>     Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> >>     Signed-off-by: Alex Chiang <achiang@hp.com>
> >>     Signed-off-by: Len Brown <len.brown@intel.com>
> >>
> >> that is between 2.6.34-rc1 and 2.6.34-rc2.

There's no need to repeat the entire changelog of Alex's commit here;
all we need is the commit ID so we can find it later.

> > Maybe.  But first can you please address Bjorn's suggestions below?
> 
> fine, if you prefer 47 lines instead one line changes.

It's more important for a change to be clear and correct than for
it to be tiny.  But I do see your point, and having seen the patch
below, I prefer the original one-line version.

I timed the original on a 64-CPU box where the namespace walk cost
should be relatively high, and the walk you added takes about 0.01
seconds.  The one already there took about 0.03 seconds, so neither
costs very much.

And I don't like the fact that the patch below changes the ACPI CA
and it only partly implements acpi_get_devices(), since it ignores
_CID and _STA.

Bjorn

> [PATCH -v2] acpi: handle ACPI0007 Device in acpi_early_set_pdc
> 
> When bios switch to use Device object instead of Processor statement.
> 
> the SSDT for Pstate/Cstate/Tstate can not loaded dynamically.
> 
> So try to scan ACPI0007 in addition to Processor.
> 
> -v2: add 47 lines instead of 1 line to walk namespace one time only.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/acpi/acpica/nsxfeval.c |   43 +++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/processor_core.c  |    8 ++++---
>  include/acpi/acpixf.h          |    2 +
>  3 files changed, 50 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/processor_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_core.c
> +++ linux-2.6/drivers/acpi/processor_core.c
> @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  #endif
>  
> -static bool processor_physically_present(acpi_handle handle)
> +static bool is_processor_and_physically_present(acpi_handle handle)
>  {
>  	int cpuid, type;
>  	u32 acpi_id;
> @@ -211,6 +211,8 @@ static bool processor_physically_present
>  		acpi_id = object.processor.proc_id;
>  		break;
>  	case ACPI_TYPE_DEVICE:
> +		if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK)
> +			return false;
>  		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
>  		if (ACPI_FAILURE(status))
>  			return false;
> @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc
>  static acpi_status
>  early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
>  {
> -	if (processor_physically_present(handle) == false)
> +	if (is_processor_and_physically_present(handle) == false)
>  		return AE_OK;
>  
>  	acpi_processor_set_pdc(handle);
> @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc
>  	 */
>  	dmi_check_system(processor_idle_dmi_table);
>  
> -	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> +	acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT,
>  			    ACPI_UINT32_MAX,
>  			    early_init_pdc, NULL, NULL, NULL);
>  }
> Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c
> +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c
> @@ -524,6 +524,49 @@ ACPI_EXPORT_SYMBOL(acpi_walk_namespace)
>  
>  /*******************************************************************************
>   *
> + * FUNCTION:    acpi_match_device_hid
> + *
> + * PARAMETERS:  handle and HID to be compared
> + *
> + * RETURN:      bool
> + *
> + * DESCRIPTION: check if device coresponding to handle has HID
> + *
> + ******************************************************************************/
> +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID)
> +{
> +	acpi_status status;
> +	struct acpi_namespace_node *node;
> +	struct acpica_device_id *hid;
> +	int no_match;
> +
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	node = acpi_ns_validate_handle(obj_handle);
> +	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	if (!node)
> +		return AE_BAD_PARAMETER;
> +
> +	status = acpi_ut_execute_HID(node, &hid);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	no_match = ACPI_STRCMP(hid->string, HID);
> +	ACPI_FREE(hid);
> +
> +	if (no_match)
> +		return AE_NOT_FOUND;
> +
> +	return AE_OK;
> +}
> +
> +/*******************************************************************************
> + *
>   * FUNCTION:    acpi_ns_get_device_callback
>   *
>   * PARAMETERS:  Callback from acpi_get_device
> Index: linux-2.6/include/acpi/acpixf.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpixf.h
> +++ linux-2.6/include/acpi/acpixf.h
> @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ
>  		    acpi_walk_callback post_order_visit,
>  		    void *context, void **return_value);
>  
> +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID);
> +
>  acpi_status
>  acpi_get_devices(const char *HID,
>  		 acpi_walk_callback user_function,
> 

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

On 09/13/2010 10:28 AM, Bjorn Helgaas wrote:
> On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote:
> 
> I timed the original on a 64-CPU box where the namespace walk cost
> should be relatively high, and the walk you added takes about 0.01
> seconds.  The one already there took about 0.03 seconds, so neither
> costs very much.
> 
> And I don't like the fact that the patch below changes the ACPI CA
> and it only partly implements acpi_get_devices(), since it ignores
> _CID and _STA.

please check this one. it still keep _STA etc.

[PATCH -v3] acpi: Handle ACPI0007 Device in acpi_early_set_pdc

After
| commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
| Author: Alex Chiang <achiang@hp.com>
| Date:   Mon Feb 22 12:11:39 2010 -0700
|
|    ACPI: processor: driver doesn't need to evaluate _PDC
|
|    Now that the early _PDC evaluation path knows how to correctly
|    evaluate _PDC on only physically present processors, there's no
|    need for the processor driver to evaluate it later when it loads.
|
|    To cover the hotplug case, push _PDC evaluation down into the
|    hotplug paths.

only cpu with Processor Statement get processed with _PDC

If bios is using Device object instead of Processor statement.
SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically.

Need to try to scan ACPI0007 in addition to Processor.

That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+
need this patch.

-v2: add 47 lines instead of 1 line to walk namespace one time only.
-v3: Let acpi_match_device_hid() call acpi_ns_get_device_callback()

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/acpica/nsxfeval.c |   41 +++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/processor_core.c  |    8 +++++---
 include/acpi/acpixf.h          |    2 ++
 3 files changed, 48 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_core.c
+++ linux-2.6/drivers/acpi/processor_core.c
@@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i
 EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 #endif
 
-static bool processor_physically_present(acpi_handle handle)
+static bool is_processor_and_physically_present(acpi_handle handle)
 {
 	int cpuid, type;
 	u32 acpi_id;
@@ -211,6 +211,8 @@ static bool processor_physically_present
 		acpi_id = object.processor.proc_id;
 		break;
 	case ACPI_TYPE_DEVICE:
+		if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK)
+			return false;
 		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
 		if (ACPI_FAILURE(status))
 			return false;
@@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc
 static acpi_status
 early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
-	if (processor_physically_present(handle) == false)
+	if (is_processor_and_physically_present(handle) == false)
 		return AE_OK;
 
 	acpi_processor_set_pdc(handle);
@@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc
 	 */
 	dmi_check_system(processor_idle_dmi_table);
 
-	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+	acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT,
 			    ACPI_UINT32_MAX,
 			    early_init_pdc, NULL, NULL, NULL);
 }
Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c
+++ linux-2.6/drivers/acpi/acpica/nsxfeval.c
@@ -642,6 +642,47 @@ acpi_ns_get_device_callback(acpi_handle
 	return (status);
 }
 
+static acpi_status
+acpi_match_device_hid_callback(acpi_handle obj_handle,
+			    u32 nesting_level,
+			    void *context, void **return_value)
+{
+	*return_value = (void *)0xff;
+
+	return AE_OK;
+}
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_match_device_hid
+ *
+ * PARAMETERS:  handle and HID to be compared
+ *
+ * RETURN:      AE_OK mean found match one
+ *
+ * DESCRIPTION: check if device coresponding to handle has HID
+ *
+ ******************************************************************************/
+acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID)
+{
+	struct acpi_get_devices_info info;
+	void *return_value = NULL;
+
+	ACPI_FUNCTION_TRACE(acpi_match_device_hid);
+
+	info.hid = HID;
+	info.context = NULL;
+	info.user_function = acpi_match_device_hid_callback;
+
+	acpi_ns_get_device_callback(obj_handle, ACPI_UINT32_MAX,
+				    &info, &return_value);
+
+	if (!return_value)
+		return AE_NOT_FOUND;
+
+	return AE_OK;
+}
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_get_devices
Index: linux-2.6/include/acpi/acpixf.h
===================================================================
--- linux-2.6.orig/include/acpi/acpixf.h
+++ linux-2.6/include/acpi/acpixf.h
@@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ
 		    acpi_walk_callback post_order_visit,
 		    void *context, void **return_value);
 
+acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID);
+
 acpi_status
 acpi_get_devices(const char *HID,
 		 acpi_walk_callback user_function,

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Bjorn Helgaas]

On Friday, September 17, 2010 01:21:56 am Yinghai Lu wrote:
> On 09/13/2010 10:28 AM, Bjorn Helgaas wrote:
> > On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote:
> > 
> > I timed the original on a 64-CPU box where the namespace walk cost
> > should be relatively high, and the walk you added takes about 0.01
> > seconds.  The one already there took about 0.03 seconds, so neither
> > costs very much.
> > 
> > And I don't like the fact that the patch below changes the ACPI CA
> > and it only partly implements acpi_get_devices(), since it ignores
> > _CID and _STA.
> 
> please check this one. it still keep _STA etc.

I think you missed this line in my previous response:

> > But I do see your point, and having seen the patch
> > below, I prefer the original one-line version.

The patch below is better, because it doesn't reimplement quite
so much stuff, but it still changes the CA, and I think the end
result is still quite confusing to read.

So I still prefer the original one-line version.

Bjorn

> [PATCH -v3] acpi: Handle ACPI0007 Device in acpi_early_set_pdc
> 
> After
> | commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
> | Author: Alex Chiang <achiang@hp.com>
> | Date:   Mon Feb 22 12:11:39 2010 -0700
> |
> |    ACPI: processor: driver doesn't need to evaluate _PDC
> |
> |    Now that the early _PDC evaluation path knows how to correctly
> |    evaluate _PDC on only physically present processors, there's no
> |    need for the processor driver to evaluate it later when it loads.
> |
> |    To cover the hotplug case, push _PDC evaluation down into the
> |    hotplug paths.
> 
> only cpu with Processor Statement get processed with _PDC
> 
> If bios is using Device object instead of Processor statement.
> SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically.
> 
> Need to try to scan ACPI0007 in addition to Processor.
> 
> That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+
> need this patch.
> 
> -v2: add 47 lines instead of 1 line to walk namespace one time only.
> -v3: Let acpi_match_device_hid() call acpi_ns_get_device_callback()
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/acpi/acpica/nsxfeval.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/processor_core.c  |    8 +++++---
>  include/acpi/acpixf.h          |    2 ++
>  3 files changed, 48 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/processor_core.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/processor_core.c
> +++ linux-2.6/drivers/acpi/processor_core.c
> @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  #endif
>  
> -static bool processor_physically_present(acpi_handle handle)
> +static bool is_processor_and_physically_present(acpi_handle handle)
>  {
>  	int cpuid, type;
>  	u32 acpi_id;
> @@ -211,6 +211,8 @@ static bool processor_physically_present
>  		acpi_id = object.processor.proc_id;
>  		break;
>  	case ACPI_TYPE_DEVICE:
> +		if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK)
> +			return false;
>  		status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp);
>  		if (ACPI_FAILURE(status))
>  			return false;
> @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc
>  static acpi_status
>  early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
>  {
> -	if (processor_physically_present(handle) == false)
> +	if (is_processor_and_physically_present(handle) == false)
>  		return AE_OK;
>  
>  	acpi_processor_set_pdc(handle);
> @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc
>  	 */
>  	dmi_check_system(processor_idle_dmi_table);
>  
> -	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
> +	acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT,
>  			    ACPI_UINT32_MAX,
>  			    early_init_pdc, NULL, NULL, NULL);
>  }
> Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c
> +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c
> @@ -642,6 +642,47 @@ acpi_ns_get_device_callback(acpi_handle
>  	return (status);
>  }
>  
> +static acpi_status
> +acpi_match_device_hid_callback(acpi_handle obj_handle,
> +			    u32 nesting_level,
> +			    void *context, void **return_value)
> +{
> +	*return_value = (void *)0xff;
> +
> +	return AE_OK;
> +}
> +
> +/*******************************************************************************
> + *
> + * FUNCTION:    acpi_match_device_hid
> + *
> + * PARAMETERS:  handle and HID to be compared
> + *
> + * RETURN:      AE_OK mean found match one
> + *
> + * DESCRIPTION: check if device coresponding to handle has HID
> + *
> + ******************************************************************************/
> +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID)
> +{
> +	struct acpi_get_devices_info info;
> +	void *return_value = NULL;
> +
> +	ACPI_FUNCTION_TRACE(acpi_match_device_hid);
> +
> +	info.hid = HID;
> +	info.context = NULL;
> +	info.user_function = acpi_match_device_hid_callback;
> +
> +	acpi_ns_get_device_callback(obj_handle, ACPI_UINT32_MAX,
> +				    &info, &return_value);
> +
> +	if (!return_value)
> +		return AE_NOT_FOUND;
> +
> +	return AE_OK;
> +}
> +
>  /*******************************************************************************
>   *
>   * FUNCTION:    acpi_get_devices
> Index: linux-2.6/include/acpi/acpixf.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpixf.h
> +++ linux-2.6/include/acpi/acpixf.h
> @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ
>  		    acpi_walk_callback post_order_visit,
>  		    void *context, void **return_value);
>  
> +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID);
> +
>  acpi_status
>  acpi_get_devices(const char *HID,
>  		 acpi_walk_callback user_function,
> 

Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

On 09/17/2010 06:56 AM, Bjorn Helgaas wrote:
> On Friday, September 17, 2010 01:21:56 am Yinghai Lu wrote:
>> On 09/13/2010 10:28 AM, Bjorn Helgaas wrote:
>>> On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote:
>>>
>>> I timed the original on a 64-CPU box where the namespace walk cost
>>> should be relatively high, and the walk you added takes about 0.01
>>> seconds.  The one already there took about 0.03 seconds, so neither
>>> costs very much.
>>>
>>> And I don't like the fact that the patch below changes the ACPI CA
>>> and it only partly implements acpi_get_devices(), since it ignores
>>> _CID and _STA.
>>
>> please check this one. it still keep _STA etc.
> 
> I think you missed this line in my previous response:
> 
>>> But I do see your point, and having seen the patch
>>> below, I prefer the original one-line version.
> 
> The patch below is better, because it doesn't reimplement quite
> so much stuff, but it still changes the CA,
not sure.
> and I think the end
> result is still quite confusing to read.
yeah, it is confusing to read.... callback with callback.
> 
> So I still prefer the original one-line version.

ok, Will resend out original one with updated changelog.

Yinghai

[PATCH -v4] acpi: Handle ACPI0007 Device in acpi_early_set_pdc

[Yinghai Lu]

After
| commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
| Author: Alex Chiang <achiang@hp.com>
| Date:   Mon Feb 22 12:11:39 2010 -0700
|
|    ACPI: processor: driver doesn't need to evaluate _PDC
|
|    Now that the early _PDC evaluation path knows how to correctly
|    evaluate _PDC on only physically present processors, there's no
|    need for the processor driver to evaluate it later when it loads.
|
|    To cover the hotplug case, push _PDC evaluation down into the
|    hotplug paths.

only cpu with Processor Statement get processed with _PDC

If bios is using Device object instead of Processor statement.
SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically.

Need to try to scan ACPI0007 in addition to Processor.

That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+
need this patch.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/processor_core.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================
--- linux-2.6.orig/drivers/acpi/processor_core.c
+++ linux-2.6/drivers/acpi/processor_core.c
@@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc
 	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
 			    ACPI_UINT32_MAX,
 			    early_init_pdc, NULL, NULL, NULL);
+	acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL);
 }

Re: [PATCH -v4] acpi: Handle ACPI0007 Device in acpi_early_set_pdc

[Len Brown]

applied to acpi-test

thanks,
Len Brown, Intel Open Source Technology Center