[PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[Alex Chiang]

It was recently discovered that some BIOS implementations load
dynamic SSDTs when a processor's _PDC method is evaluated.

In Linux, we do not evaluate _PDC until after we initialize the EC.

Unfortunately, that same BIOS references objects in the dynamic
table during EC enablement. So we get undefined references to
methods during enablement, and we fail to initialize certain pieces
of hardware.

Windows works just fine on this machine, so to me, this is a hint
that Linux should be evaluating _PDC much earlier.

The first patch in this series does just that and makes Linux
more compatible with Windows' ACPI implementation.

The remaining 10 patches unify the x86/ia64 implementations of
calling _PDC. I noticed the two archs were way more similar than
they were different, so I thought it made sense to combine them
as much as possible.

This patch series was built on both x86 and ia64 (checkpatch and
sparse clean).

It was also boot tested on an HP Envy 15 (x86) and an HP rx6600
(ia64). It resolved the namespace failures on the Envy 15 and did
not cause any regressions on the rx6600.

This does introduce a boot time namespace walk for all the CPUs
in the system, looking for and evaluating _PDC. Hopefully that
will not make life miserable for the giant SGI clusters. If worse
comes to worse, maybe we can quirk them and avoid the namespace
walk.

Thanks,
/ac

---

Alex Chiang (11):
      ACPI: processor: call _PDC early
      ACPI: processor: introduce arch_has_acpi_pdc
      ACPI: processor: unify arch_acpi_processor_init_pdc
      ACPI: processor: factor out common _PDC settings
      ACPI: processor: finish unifying arch_acpi_processor_init_pdc()
      ACPI: processor: unify arch_acpi_processor_cleanup_pdc
      ACPI: processor: introduce acpi_processor_alloc_pdc()
      ACPI: processor: change acpi_processor_eval_pdc interface
      ACPI: processor: open code acpi_processor_cleanup_pdc
      ACPI: processor: change acpi_processor_set_pdc() interface
      ACPI: processor: remove _PDC object list from struct acpi_processor


 arch/ia64/include/asm/acpi.h      |    6 +
 arch/ia64/kernel/Makefile         |    4 -
 arch/ia64/kernel/acpi-processor.c |   85 -------------------
 arch/x86/include/asm/acpi.h       |   26 ++++++
 arch/x86/kernel/acpi/Makefile     |    2 
 arch/x86/kernel/acpi/processor.c  |  101 -----------------------
 drivers/acpi/Makefile             |    1 
 drivers/acpi/bus.c                |    2 
 drivers/acpi/internal.h           |    1 
 drivers/acpi/processor_core.c     |   71 ----------------
 drivers/acpi/processor_pdc.c      |  165 +++++++++++++++++++++++++++++++++++++
 include/acpi/processor.h          |    8 +-
 12 files changed, 206 insertions(+), 266 deletions(-)
 delete mode 100644 arch/ia64/kernel/acpi-processor.c
 delete mode 100644 arch/x86/kernel/acpi/processor.c
 create mode 100644 drivers/acpi/processor_pdc.c

[PATCH 01/11] ACPI: processor: call _PDC early

[Alex Chiang]

We discovered that at least one machine (HP Envy), methods in the DSDT
attempt to call external methods defined in a dynamically loaded SSDT.

Unfortunately, the DSDT methods we are trying to call are part of the
EC initialization, which happens very early, and the the dynamic SSDT
is only loaded when a processor _PDC method runs much later.

This results in namespace lookup errors for the (as of yet) undefined
methods.

Since Windows doesn't have any issues with this machine, we take it
as a hint that they must be evaluating _PDC much earlier than we are.

Thus, the proper thing for Linux to do should be to match the Windows
implementation more closely.

Provide a mechanism to call _PDC before we enable the EC. Doing so loads
the dynamic tables, and allows the EC to be enabled correctly.

The ACPI processor driver will still evaluate _PDC in its .add() method
to cover the hotplug case.

Resolves: http://bugzilla.kernel.org/show_bug.cgi?id=14824

Cc: ming.m.lin@intel.com
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/acpi/Makefile         |    1 
 drivers/acpi/bus.c            |    2 +
 drivers/acpi/internal.h       |    1 
 drivers/acpi/processor_core.c |   69 --------------------------
 drivers/acpi/processor_pdc.c  |  108 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/processor.h      |    3 +
 6 files changed, 115 insertions(+), 69 deletions(-)
 create mode 100644 drivers/acpi/processor_pdc.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index c7b10b4..66cc3f3 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -32,6 +32,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 #
 acpi-y				+= bus.o glue.o
 acpi-y				+= scan.o
+acpi-y				+= processor_pdc.o
 acpi-y				+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-y				+= pci_root.o pci_link.o pci_irq.o pci_bind.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 65f7e33..0bdf24a 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -888,6 +888,8 @@ static int __init acpi_bus_init(void)
 		goto error1;
 	}
 
+	acpi_early_processor_set_pdc();
+
 	/*
 	 * Maybe EC region is required at bus_scan/acpi_get_devices. So it
 	 * is necessary to enable it as early as possible.
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 074cf86..cb28e05 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -43,6 +43,7 @@ int acpi_power_transition(struct acpi_device *device, int state);
 extern int acpi_power_nocheck;
 
 int acpi_wakeup_device_init(void);
+void acpi_early_processor_set_pdc(void);
 
 /* --------------------------------------------------------------------------
                                   Embedded Controller
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 4173123..a19a4ff 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -124,29 +124,6 @@ static const struct file_operations acpi_processor_info_fops = {
 
 DEFINE_PER_CPU(struct acpi_processor *, processors);
 struct acpi_processor_errata errata __read_mostly;
-static int set_no_mwait(const struct dmi_system_id *id)
-{
-	printk(KERN_NOTICE PREFIX "%s detected - "
-		"disabling mwait for CPU C-states\n", id->ident);
-	idle_nomwait = 1;
-	return 0;
-}
-
-static struct dmi_system_id __cpuinitdata processor_idle_dmi_table[] = {
-	{
-	set_no_mwait, "IFL91 board", {
-	DMI_MATCH(DMI_BIOS_VENDOR, "COMPAL"),
-	DMI_MATCH(DMI_SYS_VENDOR, "ZEPTO"),
-	DMI_MATCH(DMI_PRODUCT_VERSION, "3215W"),
-	DMI_MATCH(DMI_BOARD_NAME, "IFL91") }, NULL},
-	{
-	set_no_mwait, "Extensa 5220", {
-	DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"),
-	DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
-	DMI_MATCH(DMI_PRODUCT_VERSION, "0100"),
-	DMI_MATCH(DMI_BOARD_NAME, "Columbia") }, NULL},
-	{},
-};
 
 /* --------------------------------------------------------------------------
                                 Errata Handling
@@ -277,45 +254,6 @@ static int acpi_processor_errata(struct acpi_processor *pr)
 }
 
 /* --------------------------------------------------------------------------
-                              Common ACPI processor functions
-   -------------------------------------------------------------------------- */
-
-/*
- * _PDC is required for a BIOS-OS handshake for most of the newer
- * ACPI processor features.
- */
-static int acpi_processor_set_pdc(struct acpi_processor *pr)
-{
-	struct acpi_object_list *pdc_in = pr->pdc;
-	acpi_status status = AE_OK;
-
-
-	if (!pdc_in)
-		return status;
-	if (idle_nomwait) {
-		/*
-		 * If mwait is disabled for CPU C-states, the C2C3_FFH access
-		 * mode will be disabled in the parameter of _PDC object.
-		 * Of course C1_FFH access mode will also be disabled.
-		 */
-		union acpi_object *obj;
-		u32 *buffer = NULL;
-
-		obj = pdc_in->pointer;
-		buffer = (u32 *)(obj->buffer.pointer);
-		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
-
-	}
-	status = acpi_evaluate_object(pr->handle, "_PDC", pdc_in, NULL);
-
-	if (ACPI_FAILURE(status))
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-		    "Could not evaluate _PDC, using legacy perf. control...\n"));
-
-	return status;
-}
-
-/* --------------------------------------------------------------------------
                               FS Interface (/proc)
    -------------------------------------------------------------------------- */
 
@@ -825,9 +763,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
 	}
 
 	/* _PDC call should be done before doing anything else (if reqd.). */
-	arch_acpi_processor_init_pdc(pr);
 	acpi_processor_set_pdc(pr);
-	arch_acpi_processor_cleanup_pdc(pr);
 
 #ifdef CONFIG_CPU_FREQ
 	acpi_processor_ppc_has_changed(pr, 0);
@@ -1145,11 +1081,6 @@ static int __init acpi_processor_init(void)
 	if (!acpi_processor_dir)
 		return -ENOMEM;
 #endif
-	/*
-	 * Check whether the system is DMI table. If yes, OSPM
-	 * should not use mwait for CPU-states.
-	 */
-	dmi_check_system(processor_idle_dmi_table);
 	result = cpuidle_register_driver(&acpi_idle_driver);
 	if (result < 0)
 		goto out_proc;
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
new file mode 100644
index 0000000..b416c32
--- /dev/null
+++ b/drivers/acpi/processor_pdc.c
@@ -0,0 +1,108 @@
+#include <linux/dmi.h>
+
+#include <acpi/acpi_drivers.h>
+#include <acpi/processor.h>
+
+#include "internal.h"
+
+#define PREFIX			"ACPI: "
+#define _COMPONENT		ACPI_PROCESSOR_COMPONENT
+ACPI_MODULE_NAME("processor_pdc");
+
+static int set_no_mwait(const struct dmi_system_id *id)
+{
+	printk(KERN_NOTICE PREFIX "%s detected - "
+		"disabling mwait for CPU C-states\n", id->ident);
+	idle_nomwait = 1;
+	return 0;
+}
+
+static struct dmi_system_id __cpuinitdata processor_idle_dmi_table[] = {
+	{
+	set_no_mwait, "IFL91 board", {
+	DMI_MATCH(DMI_BIOS_VENDOR, "COMPAL"),
+	DMI_MATCH(DMI_SYS_VENDOR, "ZEPTO"),
+	DMI_MATCH(DMI_PRODUCT_VERSION, "3215W"),
+	DMI_MATCH(DMI_BOARD_NAME, "IFL91") }, NULL},
+	{
+	set_no_mwait, "Extensa 5220", {
+	DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"),
+	DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+	DMI_MATCH(DMI_PRODUCT_VERSION, "0100"),
+	DMI_MATCH(DMI_BOARD_NAME, "Columbia") }, NULL},
+	{},
+};
+
+/*
+ * _PDC is required for a BIOS-OS handshake for most of the newer
+ * ACPI processor features.
+ */
+static int acpi_processor_eval_pdc(struct acpi_processor *pr)
+{
+	struct acpi_object_list *pdc_in = pr->pdc;
+	acpi_status status = AE_OK;
+
+	if (!pdc_in)
+		return status;
+	if (idle_nomwait) {
+		/*
+		 * If mwait is disabled for CPU C-states, the C2C3_FFH access
+		 * mode will be disabled in the parameter of _PDC object.
+		 * Of course C1_FFH access mode will also be disabled.
+		 */
+		union acpi_object *obj;
+		u32 *buffer = NULL;
+
+		obj = pdc_in->pointer;
+		buffer = (u32 *)(obj->buffer.pointer);
+		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
+
+	}
+	status = acpi_evaluate_object(pr->handle, "_PDC", pdc_in, NULL);
+
+	if (ACPI_FAILURE(status))
+		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+		    "Could not evaluate _PDC, using legacy perf. control.\n"));
+
+	return status;
+}
+
+void acpi_processor_set_pdc(struct acpi_processor *pr)
+{
+	arch_acpi_processor_init_pdc(pr);
+	acpi_processor_eval_pdc(pr);
+	arch_acpi_processor_cleanup_pdc(pr);
+}
+EXPORT_SYMBOL_GPL(acpi_processor_set_pdc);
+
+static acpi_status
+early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	struct acpi_processor pr;
+
+	pr.handle = handle;
+
+	/* x86 implementation looks at pr.id to determine some
+	 * CPU capabilites. We can just hard code to 0 since we're
+	 * assuming the CPUs in the system are homogenous and all
+	 * have the same capabilities.
+	 */
+	pr.id = 0;
+
+	acpi_processor_set_pdc(&pr);
+
+	return AE_OK;
+}
+
+void acpi_early_processor_set_pdc(void)
+{
+	/*
+	 * Check whether the system is DMI table. If yes, OSPM
+	 * should not use mwait for CPU-states.
+	 */
+	dmi_check_system(processor_idle_dmi_table);
+
+	acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX,
+			    early_init_pdc, NULL, NULL, NULL);
+}
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 29245c6..a1b748a 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -325,6 +325,9 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
 
 #endif				/* CONFIG_CPU_FREQ */
 
+/* in processor_pdc.c */
+void acpi_processor_set_pdc(struct acpi_processor *pr);
+
 /* in processor_throttling.c */
 int acpi_processor_tstate_has_changed(struct acpi_processor *pr);
 int acpi_processor_get_throttling_info(struct acpi_processor *pr);

[PATCH] Fix section mismatch error for acpi_early_processor_set_pdc()

[Luck, Tony]

Alex Chiang introduced acpi_early_processor_set_pdc() in commit:
 ACPI: processor: call _PDC early
 78f1699659963fff97975df44db6d5dbe7218e55

But this results in a section mismatch:

WARNING: drivers/acpi/acpi.o(.text+0xa9c1): Section mismatch in reference from the
function acpi_early_processor_set_pdc() to the variable .cpuinit.data:processor_idle_dmi_table
The function acpi_early_processor_set_pdc() references
the variable __cpuinitdata processor_idle_dmi_table.
This is often because acpi_early_processor_set_pdc lacks a __cpuinitdata
annotation or the annotation of processor_idle_dmi_table is wrong.

The only caller of acpi_early_processor_set_pdc() is acpi_bus_init() which
is an "__init" function. So the correct fix here is to mark
acpi_early_processor_set_pdc() "__init" too.

Signed-off-by: Tony Luck <tony.luck@intel.com>

---

diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 30e4dc0..7d4ee39 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -151,7 +151,7 @@ early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
 	return AE_OK;
 }
 
-void acpi_early_processor_set_pdc(void)
+void __init acpi_early_processor_set_pdc(void)
 {
 	/*
 	 * Check whether the system is DMI table. If yes, OSPM

Re: [PATCH] Fix section mismatch error for acpi_early_processor_set_pdc()

[Alex Chiang]

* Luck, Tony <tony.luck@intel.com>:
> Alex Chiang introduced acpi_early_processor_set_pdc() in commit:
>  ACPI: processor: call _PDC early
>  78f1699659963fff97975df44db6d5dbe7218e55
> 
> But this results in a section mismatch:
> 
> WARNING: drivers/acpi/acpi.o(.text+0xa9c1): Section mismatch in reference from the
> function acpi_early_processor_set_pdc() to the variable .cpuinit.data:processor_idle_dmi_table
> The function acpi_early_processor_set_pdc() references
> the variable __cpuinitdata processor_idle_dmi_table.
> This is often because acpi_early_processor_set_pdc lacks a __cpuinitdata
> annotation or the annotation of processor_idle_dmi_table is wrong.
> 
> The only caller of acpi_early_processor_set_pdc() is acpi_bus_init() which
> is an "__init" function. So the correct fix here is to mark
> acpi_early_processor_set_pdc() "__init" too.
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>

Acked-by: Alex Chiang <achiang@hp.com>

Sorry for not catching this. Not sure why I didn't notice it
myself.

/ac

> 
> ---
> 
> diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
> index 30e4dc0..7d4ee39 100644
> --- a/drivers/acpi/processor_pdc.c
> +++ b/drivers/acpi/processor_pdc.c
> @@ -151,7 +151,7 @@ early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
>  	return AE_OK;
>  }
>  
> -void acpi_early_processor_set_pdc(void)
> +void __init acpi_early_processor_set_pdc(void)
>  {
>  	/*
>  	 * Check whether the system is DMI table. If yes, OSPM
> 

Re: [PATCH] Fix section mismatch error for acpi_early_processor_set_pdc()

[Len Brown]

applied

thanks,
Len Brown, Intel Open Source Technology Center

[PATCH 02/11] ACPI: processor: introduce arch_has_acpi_pdc

[Alex Chiang]

arch dependent helper function that tells us if we should attempt to
evaluate _PDC on this machine or not.

The x86 implementation assumes that the CPUs in the machine must be
homogeneous, and that you cannot mix CPUs of different vendors.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 arch/ia64/include/asm/acpi.h     |    2 ++
 arch/x86/include/asm/acpi.h      |    7 +++++++
 arch/x86/kernel/acpi/processor.c |    4 +---
 drivers/acpi/processor_pdc.c     |    3 +++
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h
index 91df968..3b78882 100644
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -132,6 +132,8 @@ extern int __devinitdata pxm_to_nid_map[MAX_PXM_DOMAINS];
 extern int __initdata nid_to_pxm_map[MAX_NUMNODES];
 #endif
 
+static inline bool arch_has_acpi_pdc(void) { return true; }
+
 #define acpi_unlazy_tlb(x)
 
 #ifdef CONFIG_ACPI_NUMA
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index 60d2b2d..d787e6e 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -142,6 +142,13 @@ static inline unsigned int acpi_processor_cstate_check(unsigned int max_cstate)
 		return max_cstate;
 }
 
+static inline bool arch_has_acpi_pdc(void)
+{
+	struct cpuinfo_x86 *c = &cpu_data(0);
+	return (c->x86_vendor == X86_VENDOR_INTEL ||
+		c->x86_vendor == X86_VENDOR_CENTAUR);
+}
+
 #else /* !CONFIG_ACPI */
 
 #define acpi_lapic 0
diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
index d85d1b2..bcb6efe 100644
--- a/arch/x86/kernel/acpi/processor.c
+++ b/arch/x86/kernel/acpi/processor.c
@@ -79,9 +79,7 @@ void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 	struct cpuinfo_x86 *c = &cpu_data(pr->id);
 
 	pr->pdc = NULL;
-	if (c->x86_vendor == X86_VENDOR_INTEL ||
-	    c->x86_vendor == X86_VENDOR_CENTAUR)
-		init_intel_pdc(pr, c);
+	init_intel_pdc(pr, c);
 
 	return;
 }
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index b416c32..931e735 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -69,6 +69,9 @@ static int acpi_processor_eval_pdc(struct acpi_processor *pr)
 
 void acpi_processor_set_pdc(struct acpi_processor *pr)
 {
+	if (arch_has_acpi_pdc() == false)
+		return;
+
 	arch_acpi_processor_init_pdc(pr);
 	acpi_processor_eval_pdc(pr);
 	arch_acpi_processor_cleanup_pdc(pr);

[PATCH 03/11] ACPI: processor: unify arch_acpi_processor_init_pdc

[Alex Chiang]

The x86 and ia64 implementations of arch_acpi_processor_init_pdc()
are almost exactly the same. The only difference is in what bits
they set in obj_list buffer.

Combine the boilerplate memory management code, and leave the
arch-specific bit twiddling in separate implementations.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 arch/ia64/kernel/acpi-processor.c |   34 +---------------------------
 arch/x86/kernel/acpi/processor.c  |   34 +---------------------------
 drivers/acpi/processor_pdc.c      |   45 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 46 insertions(+), 67 deletions(-)

diff --git a/arch/ia64/kernel/acpi-processor.c b/arch/ia64/kernel/acpi-processor.c
index dbda7bd..ab72d46 100644
--- a/arch/ia64/kernel/acpi-processor.c
+++ b/arch/ia64/kernel/acpi-processor.c
@@ -16,31 +16,7 @@
 
 static void init_intel_pdc(struct acpi_processor *pr)
 {
-	struct acpi_object_list *obj_list;
-	union acpi_object *obj;
-	u32 *buf;
-
-	/* allocate and initialize pdc. It will be used later. */
-	obj_list = kmalloc(sizeof(struct acpi_object_list), GFP_KERNEL);
-	if (!obj_list) {
-		printk(KERN_ERR "Memory allocation error\n");
-		return;
-	}
-
-	obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
-	if (!obj) {
-		printk(KERN_ERR "Memory allocation error\n");
-		kfree(obj_list);
-		return;
-	}
-
-	buf = kmalloc(12, GFP_KERNEL);
-	if (!buf) {
-		printk(KERN_ERR "Memory allocation error\n");
-		kfree(obj);
-		kfree(obj_list);
-		return;
-	}
+	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
 
 	buf[0] = ACPI_PDC_REVISION_ID;
 	buf[1] = 1;
@@ -52,20 +28,12 @@ static void init_intel_pdc(struct acpi_processor *pr)
 	 */
 	buf[2] |= ACPI_PDC_SMP_T_SWCOORD;
 
-	obj->type = ACPI_TYPE_BUFFER;
-	obj->buffer.length = 12;
-	obj->buffer.pointer = (u8 *) buf;
-	obj_list->count = 1;
-	obj_list->pointer = obj;
-	pr->pdc = obj_list;
-
 	return;
 }
 
 /* Initialize _PDC data based on the CPU vendor */
 void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 {
-	pr->pdc = NULL;
 	init_intel_pdc(pr);
 	return;
 }
diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
index bcb6efe..967860b 100644
--- a/arch/x86/kernel/acpi/processor.c
+++ b/arch/x86/kernel/acpi/processor.c
@@ -14,31 +14,7 @@
 
 static void init_intel_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)
 {
-	struct acpi_object_list *obj_list;
-	union acpi_object *obj;
-	u32 *buf;
-
-	/* allocate and initialize pdc. It will be used later. */
-	obj_list = kmalloc(sizeof(struct acpi_object_list), GFP_KERNEL);
-	if (!obj_list) {
-		printk(KERN_ERR "Memory allocation error\n");
-		return;
-	}
-
-	obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
-	if (!obj) {
-		printk(KERN_ERR "Memory allocation error\n");
-		kfree(obj_list);
-		return;
-	}
-
-	buf = kmalloc(12, GFP_KERNEL);
-	if (!buf) {
-		printk(KERN_ERR "Memory allocation error\n");
-		kfree(obj);
-		kfree(obj_list);
-		return;
-	}
+	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
 
 	buf[0] = ACPI_PDC_REVISION_ID;
 	buf[1] = 1;
@@ -62,13 +38,6 @@ static void init_intel_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)
 	if (!cpu_has(c, X86_FEATURE_MWAIT))
 		buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);
 
-	obj->type = ACPI_TYPE_BUFFER;
-	obj->buffer.length = 12;
-	obj->buffer.pointer = (u8 *) buf;
-	obj_list->count = 1;
-	obj_list->pointer = obj;
-	pr->pdc = obj_list;
-
 	return;
 }
 
@@ -78,7 +47,6 @@ void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 {
 	struct cpuinfo_x86 *c = &cpu_data(pr->id);
 
-	pr->pdc = NULL;
 	init_intel_pdc(pr, c);
 
 	return;
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 931e735..87946b6 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -33,6 +33,49 @@ static struct dmi_system_id __cpuinitdata processor_idle_dmi_table[] = {
 	{},
 };
 
+static void acpi_processor_init_pdc(struct acpi_processor *pr)
+{
+	struct acpi_object_list *obj_list;
+	union acpi_object *obj;
+	u32 *buf;
+
+	pr->pdc = NULL;
+
+	/* allocate and initialize pdc. It will be used later. */
+	obj_list = kmalloc(sizeof(struct acpi_object_list), GFP_KERNEL);
+	if (!obj_list) {
+		printk(KERN_ERR "Memory allocation error\n");
+		return;
+	}
+
+	obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
+	if (!obj) {
+		printk(KERN_ERR "Memory allocation error\n");
+		kfree(obj_list);
+		return;
+	}
+
+	buf = kmalloc(12, GFP_KERNEL);
+	if (!buf) {
+		printk(KERN_ERR "Memory allocation error\n");
+		kfree(obj);
+		kfree(obj_list);
+		return;
+	}
+
+	obj->type = ACPI_TYPE_BUFFER;
+	obj->buffer.length = 12;
+	obj->buffer.pointer = (u8 *) buf;
+	obj_list->count = 1;
+	obj_list->pointer = obj;
+	pr->pdc = obj_list;
+
+	/* Now let the arch do the bit-twiddling to buf[] */
+	arch_acpi_processor_init_pdc(pr);
+
+	return;
+}
+
 /*
  * _PDC is required for a BIOS-OS handshake for most of the newer
  * ACPI processor features.
@@ -72,7 +115,7 @@ void acpi_processor_set_pdc(struct acpi_processor *pr)
 	if (arch_has_acpi_pdc() == false)
 		return;
 
-	arch_acpi_processor_init_pdc(pr);
+	acpi_processor_init_pdc(pr);
 	acpi_processor_eval_pdc(pr);
 	arch_acpi_processor_cleanup_pdc(pr);
 }

[PATCH 04/11] ACPI: processor: factor out common _PDC settings

[Alex Chiang]

Both x86 and ia64 initialize _PDC with mostly common bit settings.

Factor out the common settings and leave the arch-specific ones alone.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 arch/ia64/kernel/acpi-processor.c |   10 +---------
 arch/x86/kernel/acpi/processor.c  |   10 +---------
 drivers/acpi/processor_pdc.c      |   11 +++++++++++
 3 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/arch/ia64/kernel/acpi-processor.c b/arch/ia64/kernel/acpi-processor.c
index ab72d46..ebe23f5 100644
--- a/arch/ia64/kernel/acpi-processor.c
+++ b/arch/ia64/kernel/acpi-processor.c
@@ -18,15 +18,7 @@ static void init_intel_pdc(struct acpi_processor *pr)
 {
 	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
 
-	buf[0] = ACPI_PDC_REVISION_ID;
-	buf[1] = 1;
-	buf[2] = ACPI_PDC_EST_CAPABILITY_SMP;
-	/*
-	 * The default of PDC_SMP_T_SWCOORD bit is set for IA64 cpu so
-	 * that OSPM is capable of native ACPI throttling software
-	 * coordination using BIOS supplied _TSD info.
-	 */
-	buf[2] |= ACPI_PDC_SMP_T_SWCOORD;
+	buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP;
 
 	return;
 }
diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
index 967860b..d722ca8 100644
--- a/arch/x86/kernel/acpi/processor.c
+++ b/arch/x86/kernel/acpi/processor.c
@@ -16,16 +16,8 @@ static void init_intel_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)
 {
 	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
 
-	buf[0] = ACPI_PDC_REVISION_ID;
-	buf[1] = 1;
-	buf[2] = ACPI_PDC_C_CAPABILITY_SMP;
+	buf[2] |= ACPI_PDC_C_CAPABILITY_SMP;
 
-	/*
-	 * The default of PDC_SMP_T_SWCOORD bit is set for intel x86 cpu so
-	 * that OSPM is capable of native ACPI throttling software
-	 * coordination using BIOS supplied _TSD info.
-	 */
-	buf[2] |= ACPI_PDC_SMP_T_SWCOORD;
 	if (cpu_has(c, X86_FEATURE_EST))
 		buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP;
 
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 87946b6..ccda7c9 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -33,6 +33,15 @@ static struct dmi_system_id __cpuinitdata processor_idle_dmi_table[] = {
 	{},
 };
 
+static void acpi_set_pdc_bits(u32 *buf)
+{
+	buf[0] = ACPI_PDC_REVISION_ID;
+	buf[1] = 1;
+
+	/* Enable coordination with firmware's _TSD info */
+	buf[2] = ACPI_PDC_SMP_T_SWCOORD;
+}
+
 static void acpi_processor_init_pdc(struct acpi_processor *pr)
 {
 	struct acpi_object_list *obj_list;
@@ -63,6 +72,8 @@ static void acpi_processor_init_pdc(struct acpi_processor *pr)
 		return;
 	}
 
+	acpi_set_pdc_bits(buf);
+
 	obj->type = ACPI_TYPE_BUFFER;
 	obj->buffer.length = 12;
 	obj->buffer.pointer = (u8 *) buf;

[PATCH 05/11] ACPI: processor: finish unifying arch_acpi_processor_init_pdc()

[Alex Chiang]

The only thing arch-specific about calling _PDC is what bits get
set in the input obj_list buffer.

There's no need for several levels of indirection to twiddle those
bits. Additionally, since we're just messing around with a buffer,
we can simplify the interface; no need to pass around the entire
struct acpi_processor * just to get at the buffer.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 arch/ia64/include/asm/acpi.h      |    4 ++++
 arch/ia64/kernel/acpi-processor.c |   18 ------------------
 arch/x86/include/asm/acpi.h       |   19 +++++++++++++++++++
 arch/x86/kernel/acpi/processor.c  |   34 ----------------------------------
 drivers/acpi/processor_pdc.c      |    6 +++---
 include/acpi/processor.h          |    1 -
 6 files changed, 26 insertions(+), 56 deletions(-)

diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h
index 3b78882..7ae5889 100644
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -133,6 +133,10 @@ extern int __initdata nid_to_pxm_map[MAX_NUMNODES];
 #endif
 
 static inline bool arch_has_acpi_pdc(void) { return true; }
+static inline void arch_acpi_set_pdc_bits(u32 *buf)
+{
+	buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP;
+}
 
 #define acpi_unlazy_tlb(x)
 
diff --git a/arch/ia64/kernel/acpi-processor.c b/arch/ia64/kernel/acpi-processor.c
index ebe23f5..7ba5acc 100644
--- a/arch/ia64/kernel/acpi-processor.c
+++ b/arch/ia64/kernel/acpi-processor.c
@@ -14,24 +14,6 @@
 #include <acpi/processor.h>
 #include <asm/acpi.h>
 
-static void init_intel_pdc(struct acpi_processor *pr)
-{
-	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
-
-	buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP;
-
-	return;
-}
-
-/* Initialize _PDC data based on the CPU vendor */
-void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
-{
-	init_intel_pdc(pr);
-	return;
-}
-
-EXPORT_SYMBOL(arch_acpi_processor_init_pdc);
-
 void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr)
 {
 	if (pr->pdc) {
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
index d787e6e..56f462c 100644
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -149,6 +149,25 @@ static inline bool arch_has_acpi_pdc(void)
 		c->x86_vendor == X86_VENDOR_CENTAUR);
 }
 
+static inline void arch_acpi_set_pdc_bits(u32 *buf)
+{
+	struct cpuinfo_x86 *c = &cpu_data(0);
+
+	buf[2] |= ACPI_PDC_C_CAPABILITY_SMP;
+
+	if (cpu_has(c, X86_FEATURE_EST))
+		buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP;
+
+	if (cpu_has(c, X86_FEATURE_ACPI))
+		buf[2] |= ACPI_PDC_T_FFH;
+
+	/*
+	 * If mwait/monitor is unsupported, C2/C3_FFH will be disabled
+	 */
+	if (!cpu_has(c, X86_FEATURE_MWAIT))
+		buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);
+}
+
 #else /* !CONFIG_ACPI */
 
 #define acpi_lapic 0
diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
index d722ca8..0f57307 100644
--- a/arch/x86/kernel/acpi/processor.c
+++ b/arch/x86/kernel/acpi/processor.c
@@ -12,40 +12,6 @@
 #include <acpi/processor.h>
 #include <asm/acpi.h>
 
-static void init_intel_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)
-{
-	u32 *buf = (u32 *)pr->pdc->pointer->buffer.pointer;
-
-	buf[2] |= ACPI_PDC_C_CAPABILITY_SMP;
-
-	if (cpu_has(c, X86_FEATURE_EST))
-		buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP;
-
-	if (cpu_has(c, X86_FEATURE_ACPI))
-		buf[2] |= ACPI_PDC_T_FFH;
-
-	/*
-	 * If mwait/monitor is unsupported, C2/C3_FFH will be disabled
-	 */
-	if (!cpu_has(c, X86_FEATURE_MWAIT))
-		buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);
-
-	return;
-}
-
-
-/* Initialize _PDC data based on the CPU vendor */
-void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
-{
-	struct cpuinfo_x86 *c = &cpu_data(pr->id);
-
-	init_intel_pdc(pr, c);
-
-	return;
-}
-
-EXPORT_SYMBOL(arch_acpi_processor_init_pdc);
-
 void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr)
 {
 	if (pr->pdc) {
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index ccda7c9..48df08e 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -40,6 +40,9 @@ static void acpi_set_pdc_bits(u32 *buf)
 
 	/* Enable coordination with firmware's _TSD info */
 	buf[2] = ACPI_PDC_SMP_T_SWCOORD;
+
+	/* Twiddle arch-specific bits needed for _PDC */
+	arch_acpi_set_pdc_bits(buf);
 }
 
 static void acpi_processor_init_pdc(struct acpi_processor *pr)
@@ -81,9 +84,6 @@ static void acpi_processor_init_pdc(struct acpi_processor *pr)
 	obj_list->pointer = obj;
 	pr->pdc = obj_list;
 
-	/* Now let the arch do the bit-twiddling to buf[] */
-	arch_acpi_processor_init_pdc(pr);
-
 	return;
 }
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index a1b748a..50edd73 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -257,7 +257,6 @@ int acpi_processor_notify_smm(struct module *calling_module);
 DECLARE_PER_CPU(struct acpi_processor *, processors);
 extern struct acpi_processor_errata errata;
 
-void arch_acpi_processor_init_pdc(struct acpi_processor *pr);
 void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr);
 
 #ifdef ARCH_HAS_POWER_INIT

[PATCH 06/11] ACPI: processor: unify arch_acpi_processor_cleanup_pdc

[Alex Chiang]

The x86 and ia64 implementations of the function in $subject are
exactly the same.

Also, since the arch-specific implementations of setting _PDC have
been completely hollowed out, remove the empty shells.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Alex Chiang <achiang@hp.com>
---

 arch/ia64/kernel/Makefile         |    4 ----
 arch/ia64/kernel/acpi-processor.c |   27 ---------------------------
 arch/x86/kernel/acpi/Makefile     |    2 +-
 arch/x86/kernel/acpi/processor.c  |   25 -------------------------
 drivers/acpi/processor_pdc.c      |   21 ++++++++++++++++++++-
 include/acpi/processor.h          |    2 --
 6 files changed, 21 insertions(+), 60 deletions(-)
 delete mode 100644 arch/ia64/kernel/acpi-processor.c
 delete mode 100644 arch/x86/kernel/acpi/processor.c

diff --git a/arch/ia64/kernel/Makefile b/arch/ia64/kernel/Makefile
index 2a75e93..e123634 100644
--- a/arch/ia64/kernel/Makefile
+++ b/arch/ia64/kernel/Makefile
@@ -18,10 +18,6 @@ obj-$(CONFIG_IA64_GENERIC)	+= acpi-ext.o
 obj-$(CONFIG_IA64_HP_ZX1)	+= acpi-ext.o
 obj-$(CONFIG_IA64_HP_ZX1_SWIOTLB) += acpi-ext.o
 
-ifneq ($(CONFIG_ACPI_PROCESSOR),)
-obj-y				+= acpi-processor.o
-endif
-
 obj-$(CONFIG_IA64_PALINFO)	+= palinfo.o
 obj-$(CONFIG_IOSAPIC)		+= iosapic.o
 obj-$(CONFIG_MODULES)		+= module.o
diff --git a/arch/ia64/kernel/acpi-processor.c b/arch/ia64/kernel/acpi-processor.c
deleted file mode 100644
index 7ba5acc..0000000
--- a/arch/ia64/kernel/acpi-processor.c
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * arch/ia64/kernel/acpi-processor.c
- *
- * Copyright (C) 2005 Intel Corporation
- * 	Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
- * 	- Added _PDC for platforms with Intel CPUs
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/acpi.h>
-
-#include <acpi/processor.h>
-#include <asm/acpi.h>
-
-void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr)
-{
-	if (pr->pdc) {
-		kfree(pr->pdc->pointer->buffer.pointer);
-		kfree(pr->pdc->pointer);
-		kfree(pr->pdc);
-		pr->pdc = NULL;
-	}
-}
-
-EXPORT_SYMBOL(arch_acpi_processor_cleanup_pdc);
diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index fd5ca97..6f35260 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -4,7 +4,7 @@ obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_rm.o wakeup_$(BITS).o
 
 ifneq ($(CONFIG_ACPI_PROCESSOR),)
-obj-y				+= cstate.o processor.o
+obj-y				+= cstate.o
 endif
 
 $(obj)/wakeup_rm.o:    $(obj)/realmode/wakeup.bin
diff --git a/arch/x86/kernel/acpi/processor.c b/arch/x86/kernel/acpi/processor.c
deleted file mode 100644
index 0f57307..0000000
--- a/arch/x86/kernel/acpi/processor.c
+++ /dev/null
@@ -1,25 +0,0 @@
-/*
- * Copyright (C) 2005 Intel Corporation
- * 	Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
- * 	- Added _PDC for platforms with Intel CPUs
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/acpi.h>
-
-#include <acpi/processor.h>
-#include <asm/acpi.h>
-
-void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr)
-{
-	if (pr->pdc) {
-		kfree(pr->pdc->pointer->buffer.pointer);
-		kfree(pr->pdc->pointer);
-		kfree(pr->pdc);
-		pr->pdc = NULL;
-	}
-}
-
-EXPORT_SYMBOL(arch_acpi_processor_cleanup_pdc);
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 48df08e..e786e2c 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -1,3 +1,12 @@
+/*
+ * Copyright (C) 2005 Intel Corporation
+ * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ *
+ *	Alex Chiang <achiang@hp.com>
+ *	- Unified x86/ia64 implementations
+ *	Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
+ *	- Added _PDC for platforms with Intel CPUs
+ */
 #include <linux/dmi.h>
 
 #include <acpi/acpi_drivers.h>
@@ -121,6 +130,16 @@ static int acpi_processor_eval_pdc(struct acpi_processor *pr)
 	return status;
 }
 
+static void acpi_processor_cleanup_pdc(struct acpi_processor *pr)
+{
+	if (pr->pdc) {
+		kfree(pr->pdc->pointer->buffer.pointer);
+		kfree(pr->pdc->pointer);
+		kfree(pr->pdc);
+		pr->pdc = NULL;
+	}
+}
+
 void acpi_processor_set_pdc(struct acpi_processor *pr)
 {
 	if (arch_has_acpi_pdc() == false)
@@ -128,7 +147,7 @@ void acpi_processor_set_pdc(struct acpi_processor *pr)
 
 	acpi_processor_init_pdc(pr);
 	acpi_processor_eval_pdc(pr);
-	arch_acpi_processor_cleanup_pdc(pr);
+	acpi_processor_cleanup_pdc(pr);
 }
 EXPORT_SYMBOL_GPL(acpi_processor_set_pdc);
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 50edd73..0873cd5 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -257,8 +257,6 @@ int acpi_processor_notify_smm(struct module *calling_module);
 DECLARE_PER_CPU(struct acpi_processor *, processors);
 extern struct acpi_processor_errata errata;
 
-void arch_acpi_processor_cleanup_pdc(struct acpi_processor *pr);
-
 #ifdef ARCH_HAS_POWER_INIT
 void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
 					unsigned int cpu);

[PATCH 07/11] ACPI: processor: introduce acpi_processor_alloc_pdc()

[Alex Chiang]

acpi_processor_init_pdc() isn't really doing anything interesting
with the struct acpi_processor * parameter. Its real job is to allocate
the buffer for the _PDC bits.

So rename the function to acpi_processor_alloc_pdc(), and just return
the struct acpi_object_list * it's supposed to allocate.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/acpi/processor_pdc.c |   22 +++++++++++++---------
 1 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index e786e2c..23d2828 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -54,26 +54,24 @@ static void acpi_set_pdc_bits(u32 *buf)
 	arch_acpi_set_pdc_bits(buf);
 }
 
-static void acpi_processor_init_pdc(struct acpi_processor *pr)
+static struct acpi_object_list *acpi_processor_alloc_pdc(void)
 {
 	struct acpi_object_list *obj_list;
 	union acpi_object *obj;
 	u32 *buf;
 
-	pr->pdc = NULL;
-
 	/* allocate and initialize pdc. It will be used later. */
 	obj_list = kmalloc(sizeof(struct acpi_object_list), GFP_KERNEL);
 	if (!obj_list) {
 		printk(KERN_ERR "Memory allocation error\n");
-		return;
+		return NULL;
 	}
 
 	obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
 	if (!obj) {
 		printk(KERN_ERR "Memory allocation error\n");
 		kfree(obj_list);
-		return;
+		return NULL;
 	}
 
 	buf = kmalloc(12, GFP_KERNEL);
@@ -81,7 +79,7 @@ static void acpi_processor_init_pdc(struct acpi_processor *pr)
 		printk(KERN_ERR "Memory allocation error\n");
 		kfree(obj);
 		kfree(obj_list);
-		return;
+		return NULL;
 	}
 
 	acpi_set_pdc_bits(buf);
@@ -91,9 +89,8 @@ static void acpi_processor_init_pdc(struct acpi_processor *pr)
 	obj->buffer.pointer = (u8 *) buf;
 	obj_list->count = 1;
 	obj_list->pointer = obj;
-	pr->pdc = obj_list;
 
-	return;
+	return obj_list;
 }
 
 /*
@@ -142,10 +139,17 @@ static void acpi_processor_cleanup_pdc(struct acpi_processor *pr)
 
 void acpi_processor_set_pdc(struct acpi_processor *pr)
 {
+	struct acpi_object_list *obj_list;
+
 	if (arch_has_acpi_pdc() == false)
 		return;
 
-	acpi_processor_init_pdc(pr);
+	obj_list = acpi_processor_alloc_pdc();
+	if (!obj_list)
+		return;
+
+	pr->pdc = obj_list;
+
 	acpi_processor_eval_pdc(pr);
 	acpi_processor_cleanup_pdc(pr);
 }

[PATCH 08/11] ACPI: processor: change acpi_processor_eval_pdc interface

[Alex Chiang]

acpi_processor_eval_pdc() really only needs a handle and an
acpi_object_list * to do its work.

No need to pass in a struct acpi_processor *, so let's be more specific
about what we want.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/acpi/processor_pdc.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 23d2828..974de4e 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -97,13 +97,11 @@ static struct acpi_object_list *acpi_processor_alloc_pdc(void)
  * _PDC is required for a BIOS-OS handshake for most of the newer
  * ACPI processor features.
  */
-static int acpi_processor_eval_pdc(struct acpi_processor *pr)
+static int
+acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 {
-	struct acpi_object_list *pdc_in = pr->pdc;
 	acpi_status status = AE_OK;
 
-	if (!pdc_in)
-		return status;
 	if (idle_nomwait) {
 		/*
 		 * If mwait is disabled for CPU C-states, the C2C3_FFH access
@@ -118,7 +116,7 @@ static int acpi_processor_eval_pdc(struct acpi_processor *pr)
 		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
 
 	}
-	status = acpi_evaluate_object(pr->handle, "_PDC", pdc_in, NULL);
+	status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
 
 	if (ACPI_FAILURE(status))
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
@@ -148,9 +146,7 @@ void acpi_processor_set_pdc(struct acpi_processor *pr)
 	if (!obj_list)
 		return;
 
-	pr->pdc = obj_list;
-
-	acpi_processor_eval_pdc(pr);
+	acpi_processor_eval_pdc(pr->handle, obj_list);
 	acpi_processor_cleanup_pdc(pr);
 }
 EXPORT_SYMBOL_GPL(acpi_processor_set_pdc);

[PATCH 1/3] ACPI: processor: open code acpi_processor_cleanup_pdc

[Alex Chiang]

We have the acpi_object_list * right there in acpi_processor_set_pdc()
so it doesn't seem necessary for an entire helper function just to
free it.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/acpi/processor_pdc.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 974de4e..deeba22 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -125,16 +125,6 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 	return status;
 }
 
-static void acpi_processor_cleanup_pdc(struct acpi_processor *pr)
-{
-	if (pr->pdc) {
-		kfree(pr->pdc->pointer->buffer.pointer);
-		kfree(pr->pdc->pointer);
-		kfree(pr->pdc);
-		pr->pdc = NULL;
-	}
-}
-
 void acpi_processor_set_pdc(struct acpi_processor *pr)
 {
 	struct acpi_object_list *obj_list;
@@ -147,7 +137,10 @@ void acpi_processor_set_pdc(struct acpi_processor *pr)
 		return;
 
 	acpi_processor_eval_pdc(pr->handle, obj_list);
-	acpi_processor_cleanup_pdc(pr);
+
+	kfree(obj_list->pointer->buffer.pointer);
+	kfree(obj_list->pointer);
+	kfree(obj_list);
 }
 EXPORT_SYMBOL_GPL(acpi_processor_set_pdc);
 

[PATCH 2/3] ACPI: processor: change acpi_processor_set_pdc() interface

[Alex Chiang]

When calling _PDC, we really only need the handle to the processor
to call the method; we don't look at any other parts of the
struct acpi_processor * given to us.

In the early path, when we walk the namespace, we are given the
handle directly, so just pass it through to acpi_processor_set_pdc()
without stuffing it into a wasteful struct acpi_processor allocated
on the stack each time

This saves 2834 bytes of stack.

Update the interface accordingly.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 drivers/acpi/processor_core.c |    2 +-
 drivers/acpi/processor_pdc.c  |   18 +++---------------
 include/acpi/processor.h      |    2 +-
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index a19a4ff..9863c98 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -763,7 +763,7 @@ static int __cpuinit acpi_processor_add(struct acpi_device *device)
 	}
 
 	/* _PDC call should be done before doing anything else (if reqd.). */
-	acpi_processor_set_pdc(pr);
+	acpi_processor_set_pdc(pr->handle);
 
 #ifdef CONFIG_CPU_FREQ
 	acpi_processor_ppc_has_changed(pr, 0);
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index deeba22..30e4dc0 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -125,7 +125,7 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 	return status;
 }
 
-void acpi_processor_set_pdc(struct acpi_processor *pr)
+void acpi_processor_set_pdc(acpi_handle handle)
 {
 	struct acpi_object_list *obj_list;
 
@@ -136,7 +136,7 @@ void acpi_processor_set_pdc(struct acpi_processor *pr)
 	if (!obj_list)
 		return;
 
-	acpi_processor_eval_pdc(pr->handle, obj_list);
+	acpi_processor_eval_pdc(handle, obj_list);
 
 	kfree(obj_list->pointer->buffer.pointer);
 	kfree(obj_list->pointer);
@@ -147,19 +147,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc);
 static acpi_status
 early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
-	struct acpi_processor pr;
-
-	pr.handle = handle;
-
-	/* x86 implementation looks at pr.id to determine some
-	 * CPU capabilites. We can just hard code to 0 since we're
-	 * assuming the CPUs in the system are homogenous and all
-	 * have the same capabilities.
-	 */
-	pr.id = 0;
-
-	acpi_processor_set_pdc(&pr);
-
+	acpi_processor_set_pdc(handle);
 	return AE_OK;
 }
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 0873cd5..7cc433d 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -323,7 +323,7 @@ static inline int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
 #endif				/* CONFIG_CPU_FREQ */
 
 /* in processor_pdc.c */
-void acpi_processor_set_pdc(struct acpi_processor *pr);
+void acpi_processor_set_pdc(acpi_handle handle);
 
 /* in processor_throttling.c */
 int acpi_processor_tstate_has_changed(struct acpi_processor *pr);

[PATCH 3/3] ACPI: processor: remove _PDC object list from struct acpi_processor

[Alex Chiang]

When we call _PDC, we get a handle to the processor, allocate the
object list buffer as needed, and free it immediately after calling
_PDC.

There's no need to drag around this object list with us everywhere
else, so let's just get rid of it.

Signed-off-by: Alex Chiang <achiang@hp.com>
---

 include/acpi/processor.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 7cc433d..0ea5ef4 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -224,8 +224,6 @@ struct acpi_processor {
 	struct acpi_processor_throttling throttling;
 	struct acpi_processor_limit limit;
 	struct thermal_cooling_device *cdev;
-	/* the _PDC objects for this processor, if any */
-	struct acpi_object_list *pdc;
 };
 
 struct acpi_processor_errata {

Re: [PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[Alex Chiang]

* Alex Chiang <achiang@hp.com>:
> It was recently discovered that some BIOS implementations load
> dynamic SSDTs when a processor's _PDC method is evaluated.

D'oh, I knew I'd forget something. Here are some bz references:

	http://bugzilla.kernel.org/show_bug.cgi?id=14824
	http://bugzilla.kernel.org/show_bug.cgi?id=14710

/ac

Re: [PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[Dominik Brodowski]

Hey,

On Sun, Dec 20, 2009 at 12:19:04PM -0700, Alex Chiang wrote:
> This does introduce a boot time namespace walk for all the CPUs
> in the system, looking for and evaluating _PDC. Hopefully that
> will not make life miserable for the giant SGI clusters. If worse
> comes to worse, maybe we can quirk them and avoid the namespace
> walk.

Could this be done async, with completion before EC initialization?

Best,
	Dominik

Re: [PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[Alex Chiang]

* Dominik Brodowski <linux@dominikbrodowski.net>:
> 
> On Sun, Dec 20, 2009 at 12:19:04PM -0700, Alex Chiang wrote:
> > This does introduce a boot time namespace walk for all the CPUs
> > in the system, looking for and evaluating _PDC. Hopefully that
> > will not make life miserable for the giant SGI clusters. If worse
> > comes to worse, maybe we can quirk them and avoid the namespace
> > walk.
> 
> Could this be done async, with completion before EC initialization?

I don't think I understand your suggestion.

On some platforms, we need to finish evaluating _PDC before we
initialize the EC. That's what I discovered with the Envy 15. At
that point in boot, we're single-threaded and going through early
ACPI initialization. Each step has to finish before the next, in
order for later initialization to succeed.

There's nothing else going on except for waiting for _PDC to
finish so that we can initialize the EC.

My fear was that on a huge compute cluster, walking the namespace
for every CPU object to evaluate _PDC might take a long time, and
is also probably unnecessary, since their hardware/firmware
probably doesn't load dynamic tables in _PDC.

In that case, my suggestion was to avoid the walk with a quirk.

But maybe it's not necessary to worry about that until it's
actually proven to be a problem.

Thanks,
/ac

Re: [PATCH 00/11] ACPI: early _PDC eval and unify x86/ia64

[Len Brown]

entire series applied to acpi-test

nice cleanup, Alex.
Unless Venki knows of something I don't about upcoming PDC-foo,
I think it will stick.

thanks,
Len Brown, Intel Open Source Technology Center