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 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 <[email protected]>

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

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================

Quoted 3 lines · Yinghai Lu

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

Quoted 5 lines

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?

Quoted 12 lines

Signed-off-by Yinghai Lu <[email protected]>

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

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================

drivers/acpi/processor_core.c void __init acpi_early_processor_set_pdc · line 355

Quoted 4 lines

drivers/acpi/processor_core.c +1 -0

@@ -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

Quoted 4 lines · Bjorn Helgaas

Quoted 3 lines · Yinghai Lu

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

Quoted 8 lines

Quoted 5 lines

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 <[email protected]>
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 [email protected]

Cc Tony Luck <[email protected]>

Acked-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Alex Chiang <[email protected]>

Signed-off-by Len Brown <[email protected]>

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

Quoted 31 lines

Quoted 13 lines

Signed-off-by Yinghai Lu <[email protected]>

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

Index: linux-2.6/drivers/acpi/processor_core.c
===================================================================

drivers/acpi/processor_core.c void __init acpi_early_processor_set_pdc · line 355

Quoted 4 lines

drivers/acpi/processor_core.c +1 -0

@@ -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

Quoted 35 lines · On Fri, 10 Sep 2010 12:20:36 -0700 Yinghai Lu <[email protected]> wrote:

Quoted 4 lines · Bjorn Helgaas

Quoted 3 lines · Yinghai Lu

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

Quoted 8 lines

Quoted 5 lines

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 <[email protected]>
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 [email protected]

Cc Tony Luck <[email protected]>

Acked-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Alex Chiang <[email protected]>

Signed-off-by Len Brown <[email protected]>

Quoted 4 lines

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?

Quoted 32 lines · Andrew Morton

Quoted 29 lines · On Fri, 10 Sep 2010 12:20:36 -0700 Yinghai Lu <[email protected]> wrote:

Quoted 4 lines · Bjorn Helgaas

Quoted 3 lines · Yinghai Lu

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

Quoted 8 lines

Quoted 5 lines

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 <[email protected]>
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.

Quoted 7 lines

Quoted 7 lines

Cc [email protected]

Cc Tony Luck <[email protected]>

Acked-by Venkatesh Pallipadi <[email protected]>

Signed-off-by Alex Chiang <[email protected]>

Signed-off-by Len Brown <[email protected]>

that is between 2.6.34-rc1 and 2.6.34-rc2.

Quoted 1 line

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 <[email protected]>

---
 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
===================================================================

drivers/acpi/processor_core.c +5 -3

@@ -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
===================================================================

drivers/acpi/acpica/nsxfeval.c +43 -0

@@ -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
===================================================================

include/acpi/acpixf.h +2 -0

@@ -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,

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.

Quoted 7 lines · Bjorn Helgaas

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.

Quoted 3 lines

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.

Quoted 2 lines

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

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

After

| commit d8191fa4a33fdc817277da4f2b7f771ff605a41c
| Author: Alex Chiang <[email protected]>
| 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.

(quoted diff) +0 -2

-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 <[email protected]>

---
 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
===================================================================

drivers/acpi/processor_core.c +5 -3

@@ -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
===================================================================

drivers/acpi/acpica/nsxfeval.c +41 -0

@@ -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
===================================================================

include/acpi/acpixf.h +2 -0

@@ -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,

Quoted 8 lines · Yinghai Lu

Quoted 7 lines · Bjorn Helgaas

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.

Quoted 5 lines

Quoted 3 lines

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:

Quoted 2 lines

Quoted 2 lines

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.