Skip to contentSkip to conversation

ACPI: Add _PDC object evaluate for VIA.

PATCH In review louisqi 4 participants 6 messages Sep 27, 2008 – Oct 9, 2008

Subsystems (2)
x86 ACPI
lore
Share & tools
lore

louisqi

author
#0 lore raw

PATCH 1/1 ACPI: Add _PDC object evaluate for VIA.

Dear Brown & Pallipadi：

Attached is a patch to support _PDC object evaluate for VIA CPUs. The routine "arch_acpi_processor_init_pdc" which is at "arch/x86/kernel/acpi/processor.c" currently only supports INTEL CPUs.

The patch has been tested on the "VIA VX800"chipset + "VIA C7-M1.8G" CPU. It should work for other VIA chipset / BIOS that implements the _PDC object.

Thanks
Louis

--------------------------------------------------------------------------------

[PATCH] ACPI: Add _PDC object evaluate for VIA.

Frome: Louis Qi <[email protected]>

The current routine "arch_acpi_processor_init_pdc" only supports INTEL CPUs. This patch add support for VIA CPUs.

Signed-off-by Louis Qi <[email protected]>

---

--- 2.6.26.5/arch/x86/kernel/acpi/processor.c.orig 2008-09-27 15:46:14.000000000
+0800
+++ 2.6.26.5/arch/x86/kernel/acpi/processor.c 2008-09-27 15:46:14.000000000 +0800
@@ -66,6 +66,55 @@ static void init_intel_pdc(struct acpi_p
return;
}

+static void init_via_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;
+ }
+
+ buf[0] = ACPI_PDC_REVISION_ID;
+ buf[1] = 1;
+ buf[2] = 0;
+
+ if (cpu_has(c, X86_FEATURE_EST))
+ buf[2] |= ACPI_PDC_P_FFH;
+
+ if (cpu_has(c, X86_FEATURE_ACPI))
+ buf[2] |= ACPI_PDC_T_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;
+}
+
+
/* Initialize _PDC data based on the CPU vendor */
void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
{
@@ -74,6 +123,8 @@ void arch_acpi_processor_init_pdc(struct
pr->pdc = NULL;
if (c->x86_vendor == X86_VENDOR_INTEL)
init_intel_pdc(pr, c);
+ else if (c->x86_vendor == X86_VENDOR_CENTAUR)
+ init_via_pdc(pr, c);

return;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw

just a few minor comments:

Quoted 17 lines · louisqi
(quoted diff) +17 -0
+static void init_via_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);

Review

Quoted 10 lines
(quoted diff) +10 -0
+		return;
+	}
+
+	buf = kmalloc(12, GFP_KERNEL);
+	if (!buf) {
+		printk(KERN_ERR "Memory allocation error\n");
+		kfree(obj);
+		kfree(obj_list);
+		return;
+	}

error condition cleanliness: it's cleaner to create a single off-line instance of freeing logic, at the end of the function:

...

	[ ... normal path ... ]
	return;

err_obj:

	kfree(obj);
err_obj_list:
	kfree(obj_list);
err:
	printk(KERN_ERR "init_via_pdc: memory allocation error\n");
}

and then after the allocation do:

	if (!obj_list)
		goto err;

etc.

(also note the different style of the printk)

Quoted 4 lines
(quoted diff) +4 -0
+
+	buf[0] = ACPI_PDC_REVISION_ID;
+	buf[1] = 1;
+	buf[2] = 0;

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+	if (cpu_has(c, X86_FEATURE_EST))
+		buf[2] |= ACPI_PDC_P_FFH;
+
+	if (cpu_has(c, X86_FEATURE_ACPI))
+		buf[2] |= ACPI_PDC_T_FFH;
+
+	obj->type = ACPI_TYPE_BUFFER;
+	obj->buffer.length = 12;
+	obj->buffer.pointer = (u8 *) buf;

style: no need to put a space between the '(u8 *)' and 'buf'.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return;
+}

style: no need to put 'return' at the end of a void function.

Ingo

author
#2 lore raw

Thanks a lot, Ingo! Thanks for your kindly help.

I have revise my patch as following:

//////////////////////////////////////////////////////////////////////////////////// [PATCH] ACPI: Add _PDC object evaluate for VIA.

Frome: Louis Qi <[email protected]>

The current routine "arch_acpi_processor_init_pdc" only supports INTEL CPUs. This patch add support for VIA CPUs.

Signed-off-by Louis Qi <[email protected]>

---

arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +49 -0
@@ -66,6 +66,53 @@ static void init_intel_pdc(struct acpi_p
 	return;
 }
 
+static void init_via_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)
+		goto err_obj_list;
+
+	obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
+	if (!obj)
+		goto err_obj;
+
+	buf = kmalloc(12, GFP_KERNEL);
+	if (!buf)
+		goto err_buf;
+
+	buf[0] = ACPI_PDC_REVISION_ID;
+	buf[1] = 1;
+	buf[2] = 0;
+
+	if (cpu_has(c, X86_FEATURE_EST))
+		buf[2] |= ACPI_PDC_P_FFH;
+
+	if (cpu_has(c, X86_FEATURE_ACPI))
+		buf[2] |= ACPI_PDC_T_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;
+
+err_buf:
+	kfree(obj);
+err_obj:
+	kfree(obj_list);
+err_obj_list:
+	printk(KERN_ERR "init_via_pdc: memory allocation failed\n");
+}
+
+
 /* Initialize _PDC data based on the CPU vendor */
 void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 {

@@ -74,6 +121,8 @@ void arch_acpi_processor_init_pdc(struct
 	pr->pdc = NULL;
 	if (c->x86_vendor == X86_VENDOR_INTEL)
 		init_intel_pdc(pr, c);
+	else if (c->x86_vendor == X86_VENDOR_CENTAUR)
+		init_via_pdc(pr, c);
 
 	return;
 }

////////////////////////////////////////////////////////////////////////////////////

Ingo Molnar :

Quoted 84 lines

just a few minor comments:

Quoted 17 lines · louisqi
(quoted diff) +17 -0
+static void init_via_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);

Review

Quoted 10 lines
(quoted diff) +10 -0
+		return;
+	}
+
+	buf = kmalloc(12, GFP_KERNEL);
+	if (!buf) {
+		printk(KERN_ERR "Memory allocation error\n");
+		kfree(obj);
+		kfree(obj_list);
+		return;
+	}

error condition cleanliness: it's cleaner to create a single off-line instance of freeing logic, at the end of the function:

...

	[ ... normal path ... ]
	return;

err_obj:

	kfree(obj);
err_obj_list:
	kfree(obj_list);
err:
	printk(KERN_ERR "init_via_pdc: memory allocation error\n");
}

and then after the allocation do:

	if (!obj_list)
		goto err;

etc.

(also note the different style of the printk)

Quoted 4 lines
(quoted diff) +4 -0
+
+	buf[0] = ACPI_PDC_REVISION_ID;
+	buf[1] = 1;
+	buf[2] = 0;

Review

Quoted 10 lines
(quoted diff) +10 -0
+
+	if (cpu_has(c, X86_FEATURE_EST))
+		buf[2] |= ACPI_PDC_P_FFH;
+
+	if (cpu_has(c, X86_FEATURE_ACPI))
+		buf[2] |= ACPI_PDC_T_FFH;
+
+	obj->type = ACPI_TYPE_BUFFER;
+	obj->buffer.length = 12;
+	obj->buffer.pointer = (u8 *) buf;

style: no need to put a space between the '(u8 *)' and 'buf'.

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return;
+}

style: no need to put 'return' at the end of a void function.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#3 lore raw

Dear Brown & Pallipadi:

I want to know whether there are some problems about this patch.

If it does, please kindly help me to point out, I will revise it.

Waiting for your reply!

Thanks a lot!

Louis

2008.10.08

///////////////////////////////////////////////////////

Quoted 180 lines · louisqi have

Thanks a lot, Ingo! Thanks for your kindly help.

I have revise my patch as following:

////////////////////////////////////////////////////////////////////////////////////

[PATCH] ACPI: Add _PDC object evaluate for VIA.

Frome: Louis Qi <[email protected]>

The current routine "arch_acpi_processor_init_pdc" only supports INTEL CPUs. This patch add support for VIA CPUs.

Signed-off-by Louis Qi <[email protected]>

---

--- 2.6.26.5/arch/x86/kernel/acpi/processor.c.orig 2008-09-28
10:18:11.000000000 +0800
+++ 2.6.26.5/arch/x86/kernel/acpi/processor.c 2008-09-28
10:18:11.000000000 +0800

arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +49 -0
@@ -66,6 +66,53 @@ static void init_intel_pdc(struct acpi_p
     return;
 }
 
+static void init_via_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)
+        goto err_obj_list;
+
+    obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
+    if (!obj)
+        goto err_obj;
+
+    buf = kmalloc(12, GFP_KERNEL);
+    if (!buf)
+        goto err_buf;
+
+    buf[0] = ACPI_PDC_REVISION_ID;
+    buf[1] = 1;
+    buf[2] = 0;
+
+    if (cpu_has(c, X86_FEATURE_EST))
+        buf[2] |= ACPI_PDC_P_FFH;
+
+    if (cpu_has(c, X86_FEATURE_ACPI))
+        buf[2] |= ACPI_PDC_T_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;
+
+err_buf:
+    kfree(obj);
+err_obj:
+    kfree(obj_list);
+err_obj_list:
+    printk(KERN_ERR "init_via_pdc: memory allocation failed\n");
+}
+
+
 /* Initialize _PDC data based on the CPU vendor */
 void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 {

@@ -74,6 +121,8 @@ void arch_acpi_processor_init_pdc(struct
     pr->pdc = NULL;
     if (c->x86_vendor == X86_VENDOR_INTEL)
         init_intel_pdc(pr, c);
+    else if (c->x86_vendor == X86_VENDOR_CENTAUR)
+        init_via_pdc(pr, c);
 
     return;
 }

////////////////////////////////////////////////////////////////////////////////////

Ingo Molnar :

Quoted 85 lines

just a few minor comments:

Quoted 18 lines · louisqi

+static void init_via_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)

(quoted diff) +16 -0
+{
+    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);

Review

Quoted 10 lines
(quoted diff) +10 -0
+        return;
+    }
+
+    buf = kmalloc(12, GFP_KERNEL);
+    if (!buf) {
+        printk(KERN_ERR "Memory allocation error\n");
+        kfree(obj);
+        kfree(obj_list);
+        return;
+    }

error condition cleanliness: it's cleaner to create a single off-line instance of freeing logic, at the end of the function:

...

    [ ... normal path ... ]
    return;

err_obj:

    kfree(obj);
err_obj_list:
    kfree(obj_list);
err:
    printk(KERN_ERR "init_via_pdc: memory allocation error\n");
}

and then after the allocation do:

    if (!obj_list)
        goto err;

etc.

(also note the different style of the printk)

Quoted 4 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +4 -0
+
+    buf[0] = ACPI_PDC_REVISION_ID;
+    buf[1] = 1;
+    buf[2] = 0;

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p

Quoted 10 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +10 -0
+
+    if (cpu_has(c, X86_FEATURE_EST))
+        buf[2] |= ACPI_PDC_P_FFH;
+
+    if (cpu_has(c, X86_FEATURE_ACPI))
+        buf[2] |= ACPI_PDC_T_FFH;
+
+    obj->type = ACPI_TYPE_BUFFER;
+    obj->buffer.length = 12;
+    obj->buffer.pointer = (u8 *) buf;

style: no need to put a space between the '(u8 *)' and 'buf'.

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p · line 113

Quoted 2 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +2 -0
+    return;
+}

style: no need to put 'return' at the end of a void function.

Ingo

Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw

Patch looks good.
Question: Where are these PDC bits defined for via CPUs? I mean the bits that we use in intel_pdc are defined in an Intel specific document. Just wondering whether the same bit definitions hold good for via or we should have a different set of defines?

Thanks,
Venki

Quoted 217 lines

-----Original Message-----
From: louisqi [mailto:[email protected]]
Sent: Tuesday, October 07, 2008 7:19 PM
To: Ingo Molnar
Cc: Brown, Len; Pallipadi, Venkatesh; [email protected]
Subject: Re: [PATCH] ACPI: Add _PDC object evaluate for VIA.

Dear Brown & Pallipadi:

I want to know whether there are some problems about this patch.

If it does, please kindly help me to point out, I will revise it.

Waiting for your reply!

Thanks a lot!

Louis

2008.10.08

///////////////////////////////////////////////////////

Quoted 7 lines · louisqi have

Thanks a lot, Ingo! Thanks for your kindly help.

I have revise my patch as following:

/////////////////////////////////////////////////////////////// /////////////////////

Quoted 6 lines

[PATCH] ACPI: Add _PDC object evaluate for VIA.

Frome: Louis Qi <[email protected]>

The current routine "arch_acpi_processor_init_pdc" only

supports INTEL

Quoted 11 lines

CPUs.
This patch add support for VIA CPUs.

Signed-off-by Louis Qi <[email protected]>

---

--- 2.6.26.5/arch/x86/kernel/acpi/processor.c.orig 2008-09-28
10:18:11.000000000 +0800
+++ 2.6.26.5/arch/x86/kernel/acpi/processor.c 2008-09-28
10:18:11.000000000 +0800

Review static void init_intel_pdc(struct acpi_p · line 69

Quoted 4 lines
(quoted diff) +1 -0
@@ -66,6 +66,53 @@ static void init_intel_pdc(struct acpi_p
     return;
 }
 
+static void init_via_pdc(struct acpi_processor *pr, struct

cpuinfo_x86 *c)

Quoted 51 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +46 -0
+{
+    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)
+        goto err_obj_list;
+
+    obj = kmalloc(sizeof(union acpi_object), GFP_KERNEL);
+    if (!obj)
+        goto err_obj;
+
+    buf = kmalloc(12, GFP_KERNEL);
+    if (!buf)
+        goto err_buf;
+
+    buf[0] = ACPI_PDC_REVISION_ID;
+    buf[1] = 1;
+    buf[2] = 0;
+
+    if (cpu_has(c, X86_FEATURE_EST))
+        buf[2] |= ACPI_PDC_P_FFH;
+
+    if (cpu_has(c, X86_FEATURE_ACPI))
+        buf[2] |= ACPI_PDC_T_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;
+
+err_buf:
+    kfree(obj);
+err_obj:
+    kfree(obj_list);
+err_obj_list:
+    printk(KERN_ERR "init_via_pdc: memory allocation failed\n");
+}
+
+
 /* Initialize _PDC data based on the CPU vendor */
 void arch_acpi_processor_init_pdc(struct acpi_processor *pr)
 {

arch/x86/kernel/acpi/processor.c void arch_acpi_processor_init_pdc(struct · line 128

Quoted 8 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +2 -0
@@ -74,6 +121,8 @@ void arch_acpi_processor_init_pdc(struct
     pr->pdc = NULL;
     if (c->x86_vendor == X86_VENDOR_INTEL)
         init_intel_pdc(pr, c);
+    else if (c->x86_vendor == X86_VENDOR_CENTAUR)
+        init_via_pdc(pr, c);
 
     return;
 }

/////////////////////////////////////////////////////////////// /////////////////////

Quoted 7 lines

Ingo Molnar :

Quoted 4 lines

just a few minor comments:

* louisqi <[email protected]> wrote:

Quoted 9 lines
Quoted 9 lines

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p

Quoted 9 lines

+static void init_via_pdc(struct acpi_processor *pr, struct cpuinfo_x86 *c)

arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +7 -0
+{
+    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);

Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
(quoted diff) +13 -0
+    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);
Quoted 8 lines
Quoted 8 lines

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p · line 113

Quoted 6 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +6 -0
+    if (!buf) {
+        printk(KERN_ERR "Memory allocation error\n");
+        kfree(obj);
+        kfree(obj_list);
+        return;
+    }

error condition cleanliness: it's cleaner to create a

single off-line

Quoted 40 lines
Quoted 40 lines

instance of freeing logic, at the end of the function:

...

    [ ... normal path ... ]
    return;

err_obj:

    kfree(obj);
err_obj_list:
    kfree(obj_list);
err:
    printk(KERN_ERR "init_via_pdc: memory allocation error\n");
}

and then after the allocation do:

    if (!obj_list)
        goto err;

etc.

(also note the different style of the printk)

Quoted 4 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +4 -0
+
+    buf[0] = ACPI_PDC_REVISION_ID;
+    buf[1] = 1;
+    buf[2] = 0;

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p

Quoted 10 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +10 -0
+
+    if (cpu_has(c, X86_FEATURE_EST))
+        buf[2] |= ACPI_PDC_P_FFH;
+
+    if (cpu_has(c, X86_FEATURE_ACPI))
+        buf[2] |= ACPI_PDC_T_FFH;
+
+    obj->type = ACPI_TYPE_BUFFER;
+    obj->buffer.length = 12;
+    obj->buffer.pointer = (u8 *) buf;

style: no need to put a space between the '(u8 *)' and 'buf'.

Quoted 8 lines
Quoted 8 lines

arch/x86/kernel/acpi/processor.c static void init_intel_pdc(struct acpi_p · line 113

Quoted 2 lines
arch/x86/kernel/acpi/processor.c renamed from arch/x86/kernel/acpi/processor.c.orig +2 -0
+    return;
+}

style: no need to put 'return' at the end of a void function.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe

linux-acpi" in

Quoted 6 lines
Quoted 4 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe

linux-acpi" in

Quoted 4 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

reviewer
#5 lore raw
Quoted 5 lines

Question: Where are these PDC bits defined for via CPUs? I mean the bits that we use in intel_pdc are defined in an Intel specific document. Just wondering whether the same bit definitions hold good for via or we should have a different set of defines?

ditto.
init_intel_pdc() uses Intel vendor specific bits documented in ftp://download.intel.com/technology/IAPC/acpi/downloads/30222305.pdf and coded into include/acpi/pdc_intel.h

If VIA is going to have any different bit definitions, then we should really create a pdc_via.h. But if VIA is going to re-use the bits defined by Intel in a compatible way for the forseeable future, then probably all we need to do is add a comment saying that the bits VIA is using have the exact same meaning as they do for Intel.

thanks,
-Len

ps. I'm delighted to receive a patch from VIA. Sometimes we run into failures in undocumented VIA chipsets, perhaps you can help us when we run into those?