* [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
@ 2012-05-04 13:46 Marco Aurelio da Costa
2012-05-04 13:52 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 13:46 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, linux-kernel
From: Marco Aurelio da Costa <costa@gamic.com>
Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
The EliteBook 8560W has non-initialized entries in its _PSS ACPI
table. Instead of bailing out when the first non-initialized entry is
found, ignore it and use only the valid entries. Only bail out if there
is no valid entry at all.
---
--- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
22:18:23.288041268 +0200
+++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
22:19:25.912042603 +0200
@@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
struct acpi_buffer state = { 0, NULL };
union acpi_object *pss = NULL;
int i;
+ int last_invalid = -1;
status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
@@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
printk(KERN_ERR FW_BUG PREFIX
"Invalid BIOS _PSS frequency: 0x%llx MHz\n",
px->core_frequency);
- result = -EFAULT;
- kfree(pr->performance->states);
- goto end;
+ if (-1 == last_invalid)
+ last_invalid = i;
+ } else {
+ if (last_invalid != -1) {
+ /*
+ * Copy this valid entry over last_invalid entry
+ */
+ memcpy(&(pr->performance->states[last_invalid]),
+ px, sizeof(struct acpi_processor_px));
+ ++last_invalid;
+ }
}
}
+ if (0 == last_invalid) {
+ printk(KERN_ERR FW_BUG PREFIX
+ "No valid BIOS _PSS frequency found\n");
+ result = -EFAULT;
+ kfree(pr->performance->states);
+ }
+
+ if (last_invalid > 0)
+ pr->performance->state_count = last_invalid;
+
end:
kfree(buffer.pointer);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
2012-05-04 13:46 [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones Marco Aurelio da Costa
@ 2012-05-04 13:52 ` Konrad Rzeszutek Wilk
2012-05-04 14:13 ` Marco Aurelio da Costa
0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-05-04 13:52 UTC (permalink / raw)
To: Marco Aurelio da Costa; +Cc: Len Brown, linux-acpi, linux-kernel
On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
> From: Marco Aurelio da Costa <costa@gamic.com>
> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
>
> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
> table. Instead of bailing out when the first non-initialized entry is
> found, ignore it and use only the valid entries. Only bail out if there
> is no valid entry at all.
Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
warning at the end should say: "Trying to compensate by using the
other CPU's PSS state).
>
> ---
> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
> 22:18:23.288041268 +0200
> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
> 22:19:25.912042603 +0200
> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
> struct acpi_buffer state = { 0, NULL };
> union acpi_object *pss = NULL;
> int i;
> + int last_invalid = -1;
>
>
> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
> printk(KERN_ERR FW_BUG PREFIX
> "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
> px->core_frequency);
> - result = -EFAULT;
> - kfree(pr->performance->states);
> - goto end;
> + if (-1 == last_invalid)
Swap it around or just do it this way:
if (last_invalid < 0)
> + last_invalid = i;
> + } else {
> + if (last_invalid != -1) {
if (last_invalid >= 0)
> + /*
> + * Copy this valid entry over last_invalid entry
> + */
> + memcpy(&(pr->performance->states[last_invalid]),
> + px, sizeof(struct acpi_processor_px));
> + ++last_invalid;
> + }
> }
> }
>
> + if (0 == last_invalid) {
So if _PSS that is missing is at CPU2, this own't print it.
I think you want 'if (last_invalid >= 0)'
> + printk(KERN_ERR FW_BUG PREFIX
> + "No valid BIOS _PSS frequency found\n");
And you should mention which CPU has it busted - as there are
some that are working.
> + result = -EFAULT;
> + kfree(pr->performance->states);
> + }
> +
> + if (last_invalid > 0)
Don't you want 'last_invalid >= 0' ?
> + pr->performance->state_count = last_invalid;
> +
> end:
> kfree(buffer.pointer);
> --
> 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] 10+ messages in thread
* Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
2012-05-04 13:52 ` Konrad Rzeszutek Wilk
@ 2012-05-04 14:13 ` Marco Aurelio da Costa
0 siblings, 0 replies; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 14:13 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Len Brown, linux-acpi, linux-kernel
Hi, Konrad.
On Fri, May 4, 2012 at 10:52 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
>> From: Marco Aurelio da Costa <costa@gamic.com>
>> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
>>
>> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
>> table. Instead of bailing out when the first non-initialized entry is
>> found, ignore it and use only the valid entries. Only bail out if there
>> is no valid entry at all.
>
> Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
> warning at the end should say: "Trying to compensate by using the
> other CPU's PSS state).
This case in question was created by HP removing the overclock options
and leaving the entries in a invalid/empty situation. In this specific
case, it is safe.
I am not changing the table in any way, I just ignore the
non-initialized entries. The code only use listed states. If they are
CPU bound, the code doesn't assume anything.
>
>>
>> ---
>> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
>> 22:18:23.288041268 +0200
>> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
>> 22:19:25.912042603 +0200
>> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
>> struct acpi_buffer state = { 0, NULL };
>> union acpi_object *pss = NULL;
>> int i;
>> + int last_invalid = -1;
>>
>>
>> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
>> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
>> printk(KERN_ERR FW_BUG PREFIX
>> "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
>> px->core_frequency);
>> - result = -EFAULT;
>> - kfree(pr->performance->states);
>> - goto end;
>> + if (-1 == last_invalid)
>
> Swap it around or just do it this way:
Ok.
>
> if (last_invalid < 0)
>
>> + last_invalid = i;
>> + } else {
>> + if (last_invalid != -1) {
>
> if (last_invalid >= 0)
>
>> + /*
>> + * Copy this valid entry over last_invalid entry
>> + */
>> + memcpy(&(pr->performance->states[last_invalid]),
>> + px, sizeof(struct acpi_processor_px));
>> + ++last_invalid;
>> + }
>> }
>> }
>>
>> + if (0 == last_invalid) {
>
> So if _PSS that is missing is at CPU2, this own't print it.
I don't get what do you mean by CPU. last_invalid is just the last
invalid _PSS entry item. Nothing to do with the CPU.
>
> I think you want 'if (last_invalid >= 0)'
No, it is correct. If the last invalid found item is the item 0, than
it means that no valid item was found.
>
>> + printk(KERN_ERR FW_BUG PREFIX
>> + "No valid BIOS _PSS frequency found\n");
>
> And you should mention which CPU has it busted - as there are
> some that are working.
No CPU here, just the _PSS item.
>
>
>> + result = -EFAULT;
>> + kfree(pr->performance->states);
>> + }
>> +
>> + if (last_invalid > 0)
>
> Don't you want 'last_invalid >= 0' ?
No. It is correct. If the last invalid item is greater than 0, then
there was at least 1 valid _PSS entry. And the count of valid entries
is the same as the last_invalid variable.
>
>> + pr->performance->state_count = last_invalid;
>> +
>> end:
>> kfree(buffer.pointer);
>> --
>> 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
I will send the corrected patch next.
--
Marco Costa
Customer Support
--
GAMIC mbH
Roermonder Strasse, 151
52072 Aachen
Germany
--
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] 10+ messages in thread
* Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
@ 2012-05-04 14:13 ` Marco Aurelio da Costa
0 siblings, 0 replies; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 14:13 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Len Brown, linux-acpi, linux-kernel
Hi, Konrad.
On Fri, May 4, 2012 at 10:52 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
>> From: Marco Aurelio da Costa <costa@gamic.com>
>> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
>>
>> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
>> table. Instead of bailing out when the first non-initialized entry is
>> found, ignore it and use only the valid entries. Only bail out if there
>> is no valid entry at all.
>
> Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
> warning at the end should say: "Trying to compensate by using the
> other CPU's PSS state).
This case in question was created by HP removing the overclock options
and leaving the entries in a invalid/empty situation. In this specific
case, it is safe.
I am not changing the table in any way, I just ignore the
non-initialized entries. The code only use listed states. If they are
CPU bound, the code doesn't assume anything.
>
>>
>> ---
>> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
>> 22:18:23.288041268 +0200
>> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
>> 22:19:25.912042603 +0200
>> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
>> struct acpi_buffer state = { 0, NULL };
>> union acpi_object *pss = NULL;
>> int i;
>> + int last_invalid = -1;
>>
>>
>> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
>> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
>> printk(KERN_ERR FW_BUG PREFIX
>> "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
>> px->core_frequency);
>> - result = -EFAULT;
>> - kfree(pr->performance->states);
>> - goto end;
>> + if (-1 == last_invalid)
>
> Swap it around or just do it this way:
Ok.
>
> if (last_invalid < 0)
>
>> + last_invalid = i;
>> + } else {
>> + if (last_invalid != -1) {
>
> if (last_invalid >= 0)
>
>> + /*
>> + * Copy this valid entry over last_invalid entry
>> + */
>> + memcpy(&(pr->performance->states[last_invalid]),
>> + px, sizeof(struct acpi_processor_px));
>> + ++last_invalid;
>> + }
>> }
>> }
>>
>> + if (0 == last_invalid) {
>
> So if _PSS that is missing is at CPU2, this own't print it.
I don't get what do you mean by CPU. last_invalid is just the last
invalid _PSS entry item. Nothing to do with the CPU.
>
> I think you want 'if (last_invalid >= 0)'
No, it is correct. If the last invalid found item is the item 0, than
it means that no valid item was found.
>
>> + printk(KERN_ERR FW_BUG PREFIX
>> + "No valid BIOS _PSS frequency found\n");
>
> And you should mention which CPU has it busted - as there are
> some that are working.
No CPU here, just the _PSS item.
>
>
>> + result = -EFAULT;
>> + kfree(pr->performance->states);
>> + }
>> +
>> + if (last_invalid > 0)
>
> Don't you want 'last_invalid >= 0' ?
No. It is correct. If the last invalid item is greater than 0, then
there was at least 1 valid _PSS entry. And the count of valid entries
is the same as the last_invalid variable.
>
>> + pr->performance->state_count = last_invalid;
>> +
>> end:
>> kfree(buffer.pointer);
>> --
>> 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
I will send the corrected patch next.
--
Marco Costa
Customer Support
--
GAMIC mbH
Roermonder Strasse, 151
52072 Aachen
Germany
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
2012-05-04 14:13 ` Marco Aurelio da Costa
@ 2012-05-04 14:39 ` Konrad Rzeszutek Wilk
-1 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-05-04 14:39 UTC (permalink / raw)
To: Marco Aurelio da Costa; +Cc: Len Brown, linux-acpi, linux-kernel
On Fri, May 04, 2012 at 11:13:22AM -0300, Marco Aurelio da Costa wrote:
> Hi, Konrad.
>
> On Fri, May 4, 2012 at 10:52 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
> >> From: Marco Aurelio da Costa <costa@gamic.com>
> >> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
> >>
> >> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
> >> table. Instead of bailing out when the first non-initialized entry is
> >> found, ignore it and use only the valid entries. Only bail out if there
> >> is no valid entry at all.
> >
> > Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
> > warning at the end should say: "Trying to compensate by using the
> > other CPU's PSS state).
>
> This case in question was created by HP removing the overclock options
> and leaving the entries in a invalid/empty situation. In this specific
> case, it is safe.
> I am not changing the table in any way, I just ignore the
> non-initialized entries. The code only use listed states. If they are
> CPU bound, the code doesn't assume anything.
>
> >
> >>
> >> ---
> >> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
> >> 22:18:23.288041268 +0200
> >> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
> >> 22:19:25.912042603 +0200
> >> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
> >> struct acpi_buffer state = { 0, NULL };
> >> union acpi_object *pss = NULL;
> >> int i;
> >> + int last_invalid = -1;
> >>
> >>
> >> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
> >> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
> >> printk(KERN_ERR FW_BUG PREFIX
> >> "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
> >> px->core_frequency);
> >> - result = -EFAULT;
> >> - kfree(pr->performance->states);
> >> - goto end;
> >> + if (-1 == last_invalid)
> >
> > Swap it around or just do it this way:
>
> Ok.
>
> >
> > if (last_invalid < 0)
> >
> >> + last_invalid = i;
> >> + } else {
> >> + if (last_invalid != -1) {
> >
> > if (last_invalid >= 0)
> >
> >> + /*
> >> + * Copy this valid entry over last_invalid entry
> >> + */
> >> + memcpy(&(pr->performance->states[last_invalid]),
> >> + px, sizeof(struct acpi_processor_px));
> >> + ++last_invalid;
> >> + }
> >> }
> >> }
> >>
> >> + if (0 == last_invalid) {
> >
> > So if _PSS that is missing is at CPU2, this own't print it.
>
> I don't get what do you mean by CPU. last_invalid is just the last
> invalid _PSS entry item. Nothing to do with the CPU.
The loop is based on CPU, oh wait. Not this loop. You are right - ignore
that comment please.
>
> >
> > I think you want 'if (last_invalid >= 0)'
>
> No, it is correct. If the last invalid found item is the item 0, than
> it means that no valid item was found.
I somehow thought that the 'i' was for the for_each_possible(cpu), but
that is another funtion.
>
> >
> >> + printk(KERN_ERR FW_BUG PREFIX
> >> + "No valid BIOS _PSS frequency found\n");
> >
> > And you should mention which CPU has it busted - as there are
> > some that are working.
>
> No CPU here, just the _PSS item.
Add pr->id - that will tell us which of the _PSS entries is defective.
>
> >
> >
> >> + result = -EFAULT;
> >> + kfree(pr->performance->states);
> >> + }
> >> +
> >> + if (last_invalid > 0)
> >
> > Don't you want 'last_invalid >= 0' ?
>
> No. It is correct. If the last invalid item is greater than 0, then
> there was at least 1 valid _PSS entry. And the count of valid entries
> is the same as the last_invalid variable.
>
> >
> >> + pr->performance->state_count = last_invalid;
> >> +
> >> end:
> >> kfree(buffer.pointer);
> >> --
> >> 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
>
> I will send the corrected patch next.
>
>
> --
> Marco Costa
> Customer Support
> --
> GAMIC mbH
> Roermonder Strasse, 151
> 52072 Aachen
> Germany
--
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] 10+ messages in thread
* Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones
@ 2012-05-04 14:39 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-05-04 14:39 UTC (permalink / raw)
To: Marco Aurelio da Costa; +Cc: Len Brown, linux-acpi, linux-kernel
On Fri, May 04, 2012 at 11:13:22AM -0300, Marco Aurelio da Costa wrote:
> Hi, Konrad.
>
> On Fri, May 4, 2012 at 10:52 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> > On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
> >> From: Marco Aurelio da Costa <costa@gamic.com>
> >> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
> >>
> >> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
> >> table. Instead of bailing out when the first non-initialized entry is
> >> found, ignore it and use only the valid entries. Only bail out if there
> >> is no valid entry at all.
> >
> > Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
> > warning at the end should say: "Trying to compensate by using the
> > other CPU's PSS state).
>
> This case in question was created by HP removing the overclock options
> and leaving the entries in a invalid/empty situation. In this specific
> case, it is safe.
> I am not changing the table in any way, I just ignore the
> non-initialized entries. The code only use listed states. If they are
> CPU bound, the code doesn't assume anything.
>
> >
> >>
> >> ---
> >> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
> >> 22:18:23.288041268 +0200
> >> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-04-24
> >> 22:19:25.912042603 +0200
> >> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
> >> struct acpi_buffer state = { 0, NULL };
> >> union acpi_object *pss = NULL;
> >> int i;
> >> + int last_invalid = -1;
> >>
> >>
> >> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
> >> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
> >> printk(KERN_ERR FW_BUG PREFIX
> >> "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
> >> px->core_frequency);
> >> - result = -EFAULT;
> >> - kfree(pr->performance->states);
> >> - goto end;
> >> + if (-1 == last_invalid)
> >
> > Swap it around or just do it this way:
>
> Ok.
>
> >
> > if (last_invalid < 0)
> >
> >> + last_invalid = i;
> >> + } else {
> >> + if (last_invalid != -1) {
> >
> > if (last_invalid >= 0)
> >
> >> + /*
> >> + * Copy this valid entry over last_invalid entry
> >> + */
> >> + memcpy(&(pr->performance->states[last_invalid]),
> >> + px, sizeof(struct acpi_processor_px));
> >> + ++last_invalid;
> >> + }
> >> }
> >> }
> >>
> >> + if (0 == last_invalid) {
> >
> > So if _PSS that is missing is at CPU2, this own't print it.
>
> I don't get what do you mean by CPU. last_invalid is just the last
> invalid _PSS entry item. Nothing to do with the CPU.
The loop is based on CPU, oh wait. Not this loop. You are right - ignore
that comment please.
>
> >
> > I think you want 'if (last_invalid >= 0)'
>
> No, it is correct. If the last invalid found item is the item 0, than
> it means that no valid item was found.
I somehow thought that the 'i' was for the for_each_possible(cpu), but
that is another funtion.
>
> >
> >> + printk(KERN_ERR FW_BUG PREFIX
> >> + "No valid BIOS _PSS frequency found\n");
> >
> > And you should mention which CPU has it busted - as there are
> > some that are working.
>
> No CPU here, just the _PSS item.
Add pr->id - that will tell us which of the _PSS entries is defective.
>
> >
> >
> >> + result = -EFAULT;
> >> + kfree(pr->performance->states);
> >> + }
> >> +
> >> + if (last_invalid > 0)
> >
> > Don't you want 'last_invalid >= 0' ?
>
> No. It is correct. If the last invalid item is greater than 0, then
> there was at least 1 valid _PSS entry. And the count of valid entries
> is the same as the last_invalid variable.
>
> >
> >> + pr->performance->state_count = last_invalid;
> >> +
> >> end:
> >> kfree(buffer.pointer);
> >> --
> >> 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
>
> I will send the corrected patch next.
>
>
> --
> Marco Costa
> Customer Support
> --
> GAMIC mbH
> Roermonder Strasse, 151
> 52072 Aachen
> Germany
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC][PATCH v2] ACPI: Ignore invalid _PSS entries, but use valid ones
[not found] ` <CAEe-dwg5pG2aDHopeHA2yiACUjS3cba5Vm1GyOaM4y=gpvpmMQ@mail.gmail.com>
@ 2012-05-04 15:25 ` Marco Aurelio da Costa
2012-05-04 16:11 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 15:25 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: linux-acpi, linux-kernel, Len Brown
From: Marco Aurelio da Costa <costa@gamic.com>
[v2: Fixes suggested by Konrad]
Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
The EliteBook 8560W has non-initialized entries in its _PSS ACPI
table. Instead of bailing out when the first non-initialized entry is
found, ignore it and use only the valid entries. Only bail out if there
is no valid entry at all.
---
--- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
22:18:23.288041268 +0200
+++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-05-04
17:22:57.400034613 +0200
@@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
struct acpi_buffer state = { 0, NULL };
union acpi_object *pss = NULL;
int i;
+ int last_invalid = -1;
status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
@@ -372,14 +373,32 @@ static int acpi_processor_get_performanc
((u32)(px->core_frequency * 1000) !=
(px->core_frequency * 1000))) {
printk(KERN_ERR FW_BUG PREFIX
- "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
- px->core_frequency);
- result = -EFAULT;
- kfree(pr->performance->states);
- goto end;
+ "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
+ pr->id, px->core_frequency);
+ if (last_invalidi == -1)
+ last_invalid = i;
+ } else {
+ if (last_invalid != -1) {
+ /*
+ * Copy this valid entry over last_invalid entry
+ */
+ memcpy(&(pr->performance->states[last_invalid]),
+ px, sizeof(struct acpi_processor_px));
+ ++last_invalid;
+ }
}
}
+ if (last_invalid == 0) {
+ printk(KERN_ERR FW_BUG PREFIX
+ "No valid BIOS _PSS frequency found for processor %d\n", pr->id);
+ result = -EFAULT;
+ kfree(pr->performance->states);
+ }
+
+ if (last_invalid > 0)
+ pr->performance->state_count = last_invalid;
+
end:
kfree(buffer.pointer);
--
Marco Costa
Customer Support
--
GAMIC mbH
Roermonder Strasse, 151
52072 Aachen
Germany
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC][PATCH v2] ACPI: Ignore invalid _PSS entries, but use valid ones
2012-05-04 15:25 ` [RFC][PATCH v2] " Marco Aurelio da Costa
@ 2012-05-04 16:11 ` Konrad Rzeszutek Wilk
2012-05-04 16:21 ` Marco Aurelio da Costa
0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-05-04 16:11 UTC (permalink / raw)
To: Marco Aurelio da Costa; +Cc: linux-acpi, linux-kernel, Len Brown
On Fri, May 04, 2012 at 05:25:13PM +0200, Marco Aurelio da Costa wrote:
> From: Marco Aurelio da Costa <costa@gamic.com>
> [v2: Fixes suggested by Konrad]
> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
Heh. You didn't compile test this version did you?
>
> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
> table. Instead of bailing out when the first non-initialized entry is
> found, ignore it and use only the valid entries. Only bail out if there
> is no valid entry at all.
>
> ---
> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
> 22:18:23.288041268 +0200
> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-05-04
> 17:22:57.400034613 +0200
> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
> struct acpi_buffer state = { 0, NULL };
> union acpi_object *pss = NULL;
> int i;
> + int last_invalid = -1;
>
>
> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
> @@ -372,14 +373,32 @@ static int acpi_processor_get_performanc
> ((u32)(px->core_frequency * 1000) !=
> (px->core_frequency * 1000))) {
> printk(KERN_ERR FW_BUG PREFIX
> - "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
> - px->core_frequency);
> - result = -EFAULT;
> - kfree(pr->performance->states);
> - goto end;
> + "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
> + pr->id, px->core_frequency);
> + if (last_invalidi == -1)
Hrmm. invalidi?
> + last_invalid = i;
> + } else {
> + if (last_invalid != -1) {
> + /*
> + * Copy this valid entry over last_invalid entry
> + */
> + memcpy(&(pr->performance->states[last_invalid]),
> + px, sizeof(struct acpi_processor_px));
> + ++last_invalid;
> + }
> }
> }
>
> + if (last_invalid == 0) {
> + printk(KERN_ERR FW_BUG PREFIX
> + "No valid BIOS _PSS frequency found for processor %d\n", pr->id);
> + result = -EFAULT;
> + kfree(pr->performance->states);
Just as a precaution - also do this pls:
pr->performance->states = NULL?
> + }
> +
> + if (last_invalid > 0)
> + pr->performance->state_count = last_invalid;
> +
> end:
> kfree(buffer.pointer);
>
> --
> Marco Costa
> Customer Support
> --
> GAMIC mbH
> Roermonder Strasse, 151
> 52072 Aachen
> Germany
> --
> 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] 10+ messages in thread
* Re: [RFC][PATCH v2] ACPI: Ignore invalid _PSS entries, but use valid ones
2012-05-04 16:11 ` Konrad Rzeszutek Wilk
@ 2012-05-04 16:21 ` Marco Aurelio da Costa
0 siblings, 0 replies; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 16:21 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: linux-acpi, linux-kernel, Len Brown
On Fri, May 4, 2012 at 6:11 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, May 04, 2012 at 05:25:13PM +0200, Marco Aurelio da Costa wrote:
>> From: Marco Aurelio da Costa <costa@gamic.com>
>> [v2: Fixes suggested by Konrad]
>> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
>
> Heh. You didn't compile test this version did you?
>>
>> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
>> table. Instead of bailing out when the first non-initialized entry is
>> found, ignore it and use only the valid entries. Only bail out if there
>> is no valid entry at all.
>>
>> ---
>> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
>> 22:18:23.288041268 +0200
>> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-05-04
>> 17:22:57.400034613 +0200
>> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
>> struct acpi_buffer state = { 0, NULL };
>> union acpi_object *pss = NULL;
>> int i;
>> + int last_invalid = -1;
>>
>>
>> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
>> @@ -372,14 +373,32 @@ static int acpi_processor_get_performanc
>> ((u32)(px->core_frequency * 1000) !=
>> (px->core_frequency * 1000))) {
>> printk(KERN_ERR FW_BUG PREFIX
>> - "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
>> - px->core_frequency);
>> - result = -EFAULT;
>> - kfree(pr->performance->states);
>> - goto end;
>> + "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
>> + pr->id, px->core_frequency);
>> + if (last_invalidi == -1)
>
> Hrmm. invalidi?
Sorry, vi...
>
>> + last_invalid = i;
>> + } else {
>> + if (last_invalid != -1) {
>> + /*
>> + * Copy this valid entry over last_invalid entry
>> + */
>> + memcpy(&(pr->performance->states[last_invalid]),
>> + px, sizeof(struct acpi_processor_px));
>> + ++last_invalid;
>> + }
>> }
>> }
>>
>> + if (last_invalid == 0) {
>> + printk(KERN_ERR FW_BUG PREFIX
>> + "No valid BIOS _PSS frequency found for processor %d\n", pr->id);
>> + result = -EFAULT;
>> + kfree(pr->performance->states);
>
> Just as a precaution - also do this pls:
>
> pr->performance->states = NULL?
It wasn't on the original code, but I agree with you.
Doing and compiling, this time... :)
>
>> + }
>> +
>> + if (last_invalid > 0)
>> + pr->performance->state_count = last_invalid;
>> +
>> end:
>> kfree(buffer.pointer);
>>
>> --
>> Marco Costa
>> Customer Support
>> --
>> GAMIC mbH
>> Roermonder Strasse, 151
>> 52072 Aachen
>> Germany
>> --
>> 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
--
Marco Costa
Customer Support
--
GAMIC mbH
Roermonder Strasse, 151
52072 Aachen
Germany
--
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] 10+ messages in thread
* Re: [RFC][PATCH v2] ACPI: Ignore invalid _PSS entries, but use valid ones
@ 2012-05-04 16:21 ` Marco Aurelio da Costa
0 siblings, 0 replies; 10+ messages in thread
From: Marco Aurelio da Costa @ 2012-05-04 16:21 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: linux-acpi, linux-kernel, Len Brown
On Fri, May 4, 2012 at 6:11 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Fri, May 04, 2012 at 05:25:13PM +0200, Marco Aurelio da Costa wrote:
>> From: Marco Aurelio da Costa <costa@gamic.com>
>> [v2: Fixes suggested by Konrad]
>> Signed-off-by: Marco Aurelio da Costa <costa@gamic.com>
>
> Heh. You didn't compile test this version did you?
>>
>> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
>> table. Instead of bailing out when the first non-initialized entry is
>> found, ignore it and use only the valid entries. Only bail out if there
>> is no valid entry at all.
>>
>> ---
>> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig 2012-04-24
>> 22:18:23.288041268 +0200
>> +++ linux-3.3.3/drivers/acpi/processor_perflib.c 2012-05-04
>> 17:22:57.400034613 +0200
>> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
>> struct acpi_buffer state = { 0, NULL };
>> union acpi_object *pss = NULL;
>> int i;
>> + int last_invalid = -1;
>>
>>
>> status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
>> @@ -372,14 +373,32 @@ static int acpi_processor_get_performanc
>> ((u32)(px->core_frequency * 1000) !=
>> (px->core_frequency * 1000))) {
>> printk(KERN_ERR FW_BUG PREFIX
>> - "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
>> - px->core_frequency);
>> - result = -EFAULT;
>> - kfree(pr->performance->states);
>> - goto end;
>> + "Invalid BIOS _PSS frequency found for processor %d: 0x%llx MHz\n",
>> + pr->id, px->core_frequency);
>> + if (last_invalidi == -1)
>
> Hrmm. invalidi?
Sorry, vi...
>
>> + last_invalid = i;
>> + } else {
>> + if (last_invalid != -1) {
>> + /*
>> + * Copy this valid entry over last_invalid entry
>> + */
>> + memcpy(&(pr->performance->states[last_invalid]),
>> + px, sizeof(struct acpi_processor_px));
>> + ++last_invalid;
>> + }
>> }
>> }
>>
>> + if (last_invalid == 0) {
>> + printk(KERN_ERR FW_BUG PREFIX
>> + "No valid BIOS _PSS frequency found for processor %d\n", pr->id);
>> + result = -EFAULT;
>> + kfree(pr->performance->states);
>
> Just as a precaution - also do this pls:
>
> pr->performance->states = NULL?
It wasn't on the original code, but I agree with you.
Doing and compiling, this time... :)
>
>> + }
>> +
>> + if (last_invalid > 0)
>> + pr->performance->state_count = last_invalid;
>> +
>> end:
>> kfree(buffer.pointer);
>>
>> --
>> Marco Costa
>> Customer Support
>> --
>> GAMIC mbH
>> Roermonder Strasse, 151
>> 52072 Aachen
>> Germany
>> --
>> 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
--
Marco Costa
Customer Support
--
GAMIC mbH
Roermonder Strasse, 151
52072 Aachen
Germany
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-05-04 16:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-04 13:46 [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid ones Marco Aurelio da Costa
2012-05-04 13:52 ` Konrad Rzeszutek Wilk
2012-05-04 14:13 ` Marco Aurelio da Costa
2012-05-04 14:13 ` Marco Aurelio da Costa
2012-05-04 14:39 ` Konrad Rzeszutek Wilk
2012-05-04 14:39 ` Konrad Rzeszutek Wilk
[not found] ` <CAEe-dwg5pG2aDHopeHA2yiACUjS3cba5Vm1GyOaM4y=gpvpmMQ@mail.gmail.com>
2012-05-04 15:25 ` [RFC][PATCH v2] " Marco Aurelio da Costa
2012-05-04 16:11 ` Konrad Rzeszutek Wilk
2012-05-04 16:21 ` Marco Aurelio da Costa
2012-05-04 16:21 ` Marco Aurelio da Costa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.