* [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
[parent not found: <CAEe-dwg5pG2aDHopeHA2yiACUjS3cba5Vm1GyOaM4y=gpvpmMQ@mail.gmail.com>]
* [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.