* [PATCH] ACPI: Add _PDC object evaluate for VIA.
@ 2008-09-27 8:52 louisqi
2008-09-27 16:05 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: louisqi @ 2008-09-27 8:52 UTC (permalink / raw)
To: len.brown, venkatesh.pallipadi, linux-acpi
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 <louisqi@viatech.com.cn>
The current routine "arch_acpi_processor_init_pdc" only supports INTEL CPUs.
This patch add support for VIA CPUs.
Signed-off-by: Louis Qi <louisqi@viatech.com.cn>
---
--- 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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI: Add _PDC object evaluate for VIA.
2008-09-27 8:52 [PATCH] ACPI: Add _PDC object evaluate for VIA louisqi
@ 2008-09-27 16:05 ` Ingo Molnar
2008-09-28 2:40 ` louisqi
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2008-09-27 16:05 UTC (permalink / raw)
To: louisqi; +Cc: len.brown, venkatesh.pallipadi, linux-acpi
just a few minor comments:
* louisqi <louisqi@viatech.com.cn> wrote:
> +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;
> + }
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)
> +
> + 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;
style: no need to put a space between the '(u8 *)' and 'buf'.
> + return;
> +}
style: no need to put 'return' at the end of a void function.
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI: Add _PDC object evaluate for VIA.
2008-09-27 16:05 ` Ingo Molnar
@ 2008-09-28 2:40 ` louisqi
2008-10-08 2:19 ` louisqi
0 siblings, 1 reply; 6+ messages in thread
From: louisqi @ 2008-09-28 2:40 UTC (permalink / raw)
To: Ingo Molnar; +Cc: len.brown, venkatesh.pallipadi, linux-acpi
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 <louisqi@viatech.com.cn>
The current routine "arch_acpi_processor_init_pdc" only supports INTEL CPUs.
This patch add support for VIA CPUs.
Signed-off-by: Louis Qi <louisqi@viatech.com.cn>
---
--- 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
@@ -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 :
> just a few minor comments:
>
> * louisqi <louisqi@viatech.com.cn> wrote:
>
>> +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;
>> + }
>
> 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)
>
>> +
>> + 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;
>
> style: no need to put a space between the '(u8 *)' and 'buf'.
>
>> + 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI: Add _PDC object evaluate for VIA.
2008-09-28 2:40 ` louisqi
@ 2008-10-08 2:19 ` louisqi
2008-10-08 23:47 ` Pallipadi, Venkatesh
0 siblings, 1 reply; 6+ messages in thread
From: louisqi @ 2008-10-08 2:19 UTC (permalink / raw)
To: Ingo Molnar; +Cc: len.brown, venkatesh.pallipadi, linux-acpi
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
///////////////////////////////////////////////////////
louisqi have wrote:
>
> 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 <louisqi@viatech.com.cn>
>
> The current routine "arch_acpi_processor_init_pdc" only supports INTEL
> CPUs.
> This patch add support for VIA CPUs.
>
> Signed-off-by: Louis Qi <louisqi@viatech.com.cn>
> ---
>
> --- 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
> @@ -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 :
>> just a few minor comments:
>>
>> * louisqi <louisqi@viatech.com.cn> wrote:
>>
>>> +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;
>>> + }
>>
>> 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)
>>
>>> +
>>> + 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;
>>
>> style: no need to put a space between the '(u8 *)' and 'buf'.
>>
>>> + 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 majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] ACPI: Add _PDC object evaluate for VIA.
2008-10-08 2:19 ` louisqi
@ 2008-10-08 23:47 ` Pallipadi, Venkatesh
2008-10-09 0:46 ` Len Brown
0 siblings, 1 reply; 6+ messages in thread
From: Pallipadi, Venkatesh @ 2008-10-08 23:47 UTC (permalink / raw)
To: louisqi, Ingo Molnar; +Cc: Brown, Len, linux-acpi@vger.kernel.org
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
>-----Original Message-----
>From: louisqi [mailto:louisqi@viatech.com.cn]
>Sent: Tuesday, October 07, 2008 7:19 PM
>To: Ingo Molnar
>Cc: Brown, Len; Pallipadi, Venkatesh; linux-acpi@vger.kernel.org
>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
>
>///////////////////////////////////////////////////////
>louisqi have wrote:
>>
>> 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 <louisqi@viatech.com.cn>
>>
>> The current routine "arch_acpi_processor_init_pdc" only
>supports INTEL
>> CPUs.
>> This patch add support for VIA CPUs.
>>
>> Signed-off-by: Louis Qi <louisqi@viatech.com.cn>
>> ---
>>
>> --- 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
>> @@ -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 :
>>> just a few minor comments:
>>>
>>> * louisqi <louisqi@viatech.com.cn> wrote:
>>>
>>>> +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;
>>>> + }
>>>
>>> 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)
>>>
>>>> +
>>>> + 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;
>>>
>>> style: no need to put a space between the '(u8 *)' and 'buf'.
>>>
>>>> + 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 majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] ACPI: Add _PDC object evaluate for VIA.
2008-10-08 23:47 ` Pallipadi, Venkatesh
@ 2008-10-09 0:46 ` Len Brown
0 siblings, 0 replies; 6+ messages in thread
From: Len Brown @ 2008-10-09 0:46 UTC (permalink / raw)
To: Pallipadi, Venkatesh; +Cc: louisqi, Ingo Molnar, linux-acpi@vger.kernel.org
> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-09 0:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-27 8:52 [PATCH] ACPI: Add _PDC object evaluate for VIA louisqi
2008-09-27 16:05 ` Ingo Molnar
2008-09-28 2:40 ` louisqi
2008-10-08 2:19 ` louisqi
2008-10-08 23:47 ` Pallipadi, Venkatesh
2008-10-09 0:46 ` Len Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).