* [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points
@ 2008-03-10 1:53 Zhang, Rui
2008-03-11 6:35 ` Len Brown
0 siblings, 1 reply; 4+ messages in thread
From: Zhang, Rui @ 2008-03-10 1:53 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi
thermal fixup for broken BIOS which has invalid trip points.
http://bugzilla.kernel.org/show_bug.cgi?id=8544
http://marc.info/?l=linux-kernel&m=120496222629983&w=2
Signed-off-by: Zhang Rui<rui.zhang@intel.com>
---
drivers/acpi/thermal.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/acpi/thermal.c
===================================================================
--- linux-2.6.orig/drivers/acpi/thermal.c
+++ linux-2.6/drivers/acpi/thermal.c
@@ -326,7 +326,9 @@ static int acpi_thermal_set_cooling_mode
#define ACPI_TRIPS_ACTIVE 0x08
#define ACPI_TRIPS_DEVICES 0x10
-#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
+#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | \
+ ACPI_TRIPS_ACTIVE | \
+ ACPI_TRIPS_DEVICES)
#define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points
2008-03-10 1:53 [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points Zhang, Rui
@ 2008-03-11 6:35 ` Len Brown
2008-03-11 7:52 ` Zhang, Rui
0 siblings, 1 reply; 4+ messages in thread
From: Len Brown @ 2008-03-11 6:35 UTC (permalink / raw)
To: Zhang, Rui; +Cc: linux-acpi
On Sunday 09 March 2008, Zhang, Rui wrote:
> thermal fixup for broken BIOS which has invalid trip points.
> http://bugzilla.kernel.org/show_bug.cgi?id=8544
> http://marc.info/?l=linux-kernel&m=120496222629983&w=2
>
> Signed-off-by: Zhang Rui<rui.zhang@intel.com>
> ---
> drivers/acpi/thermal.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-2.6/drivers/acpi/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/thermal.c
> +++ linux-2.6/drivers/acpi/thermal.c
> @@ -326,7 +326,9 @@ static int acpi_thermal_set_cooling_mode
> #define ACPI_TRIPS_ACTIVE 0x08
> #define ACPI_TRIPS_DEVICES 0x10
>
> -#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
> +#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | \
> + ACPI_TRIPS_ACTIVE | \
> + ACPI_TRIPS_DEVICES)
> #define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
>
> #define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
>
>
I don't like re-evaluating _AL0 on notify x81 as a workaround
to notice that there _is_ no _AL0. We should re-evaluate _AL0
on notify x82 -- per the spec.
We should print out a single exeception at boot time when
we realize that the BIOS has a bug of no _AL0 for the _AC0.
At run time, we should have no concept of a valid _AC0
and thus a notify x81 should not try to use it.
Can we do that?
Otherwise, I think we're obfuscating the code to workaround
a broken BIOS.
I think we need a test in linuxfirmware kit to verify
that there is an ALx for every ACx.
thanks,
-Len
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points
2008-03-11 6:35 ` Len Brown
@ 2008-03-11 7:52 ` Zhang, Rui
2008-03-11 21:23 ` Németh Márton
0 siblings, 1 reply; 4+ messages in thread
From: Zhang, Rui @ 2008-03-11 7:52 UTC (permalink / raw)
To: Len Brown; +Cc: linux-acpi, Németh Márton
[-- Attachment #1: Type: text/plain, Size: 4216 bytes --]
On Tue, 2008-03-11 at 14:35 +0800, Len Brown wrote:
> On Sunday 09 March 2008, Zhang, Rui wrote:
> > thermal fixup for broken BIOS which has invalid trip points.
> > http://bugzilla.kernel.org/show_bug.cgi?id=8544
> > http://marc.info/?l=linux-kernel&m=120496222629983&w=2
> >
> > Signed-off-by: Zhang Rui<rui.zhang@intel.com>
> > ---
> > drivers/acpi/thermal.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6/drivers/acpi/thermal.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/thermal.c
> > +++ linux-2.6/drivers/acpi/thermal.c
> > @@ -326,7 +326,9 @@ static int acpi_thermal_set_cooling_mode
> > #define ACPI_TRIPS_ACTIVE 0x08
> > #define ACPI_TRIPS_DEVICES 0x10
> >
> > -#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE |
> ACPI_TRIPS_ACTIVE)
> > +#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE |
> \
> > + ACPI_TRIPS_ACTIVE | \
> > + ACPI_TRIPS_DEVICES)
> > #define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
> >
> > #define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT
> | \
> >
> >
>
> I don't like re-evaluating _AL0 on notify 0x81 as a workaround
> to notice that there _is_ no _AL0. We should re-evaluate _AL0
> on notify x82 -- per the spec.
I agree.
>
> We should print out a single exeception at boot time when
> we realize that the BIOS has a bug of no _AL0 for the _AC0.
> At run time, we should have no concept of a valid _AC0
> and thus a notify x81 should not try to use it.
OK.
With the refreshed patch applied, ACPI thermal driver will not try to
evaluate any invalid trip points for notification 0X81. And a warn
message is printed out at boot time.
Marton, can you try this patch and attach the dmesg output please? :)
thermal fixup for broken BIOS which has invalid trip points.
ACPI thermal driver only re-evaluate valid trip points.
For the broken BIOS show in
http://bugzilla.kernel.org/show_bug.cgi?id=8544
the active[0] is set to invalid at boot time
and it will not be re-evaluated again.
We can still get a warning message at boot time.
http://marc.info/?l=linux-kernel&m=120496222629983&w=2
Signed-off-by: Zhang Rui<rui.zhang@intel.com>
---
drivers/acpi/thermal.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/acpi/thermal.c
===================================================================
--- linux-2.6.orig/drivers/acpi/thermal.c
+++ linux-2.6/drivers/acpi/thermal.c
@@ -401,7 +401,8 @@ static int acpi_thermal_trips_update(str
}
/* Passive (optional) */
- if (flag & ACPI_TRIPS_PASSIVE) {
+ if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
+ (flag == ACPI_TRIPS_INIT)) {
valid = tz->trips.passive.flags.valid;
if (psv == -1) {
status = AE_SUPPORT;
@@ -440,8 +441,11 @@ static int acpi_thermal_trips_update(str
memset(&devices, 0, sizeof(struct acpi_handle_list));
status = acpi_evaluate_reference(tz->device->handle, "_PSL",
NULL, &devices);
- if (ACPI_FAILURE(status))
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_WARNING PREFIX
+ "Invalid passive threshold\n");
tz->trips.passive.flags.valid = 0;
+ }
else
tz->trips.passive.flags.valid = 1;
@@ -465,7 +469,8 @@ static int acpi_thermal_trips_update(str
if (act == -1)
break; /* disable all active trip points */
- if (flag & ACPI_TRIPS_ACTIVE) {
+ if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
+ tz->trips.active[i].flags.valid)) {
status = acpi_evaluate_integer(tz->device->handle,
name, NULL, &tz->trips.active[i].temperature);
if (ACPI_FAILURE(status)) {
@@ -497,8 +502,11 @@ static int acpi_thermal_trips_update(str
memset(&devices, 0, sizeof(struct acpi_handle_list));
status = acpi_evaluate_reference(tz->device->handle,
name, NULL, &devices);
- if (ACPI_FAILURE(status))
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_WARNING PREFIX
+ "Invalid active%d threshold\n", i);
tz->trips.active[i].flags.valid = 0;
+ }
else
tz->trips.active[i].flags.valid = 1;
[-- Attachment #2: thermal-fix --]
[-- Type: text/plain, Size: 2323 bytes --]
thermal fixup for broken BIOS which has invalid trip points.
ACPI thermal driver only re-evaluate valid trip points.
For the broken BIOS show in
http://bugzilla.kernel.org/show_bug.cgi?id=8544
the active[0] is set to invalid at boot time
and it will not be re-evaluated again.
We can still get a warning message at boot time.
http://marc.info/?l=linux-kernel&m=120496222629983&w=2
Signed-off-by: Zhang Rui<rui.zhang@intel.com>
---
drivers/acpi/thermal.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/acpi/thermal.c
===================================================================
--- linux-2.6.orig/drivers/acpi/thermal.c
+++ linux-2.6/drivers/acpi/thermal.c
@@ -401,7 +401,8 @@ static int acpi_thermal_trips_update(str
}
/* Passive (optional) */
- if (flag & ACPI_TRIPS_PASSIVE) {
+ if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
+ (flag == ACPI_TRIPS_INIT)) {
valid = tz->trips.passive.flags.valid;
if (psv == -1) {
status = AE_SUPPORT;
@@ -440,8 +441,11 @@ static int acpi_thermal_trips_update(str
memset(&devices, 0, sizeof(struct acpi_handle_list));
status = acpi_evaluate_reference(tz->device->handle, "_PSL",
NULL, &devices);
- if (ACPI_FAILURE(status))
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_WARNING PREFIX
+ "Invalid passive threshold\n");
tz->trips.passive.flags.valid = 0;
+ }
else
tz->trips.passive.flags.valid = 1;
@@ -465,7 +469,8 @@ static int acpi_thermal_trips_update(str
if (act == -1)
break; /* disable all active trip points */
- if (flag & ACPI_TRIPS_ACTIVE) {
+ if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
+ tz->trips.active[i].flags.valid)) {
status = acpi_evaluate_integer(tz->device->handle,
name, NULL, &tz->trips.active[i].temperature);
if (ACPI_FAILURE(status)) {
@@ -497,8 +502,11 @@ static int acpi_thermal_trips_update(str
memset(&devices, 0, sizeof(struct acpi_handle_list));
status = acpi_evaluate_reference(tz->device->handle,
name, NULL, &devices);
- if (ACPI_FAILURE(status))
+ if (ACPI_FAILURE(status)) {
+ printk(KERN_WARNING PREFIX
+ "Invalid active%d threshold\n", i);
tz->trips.active[i].flags.valid = 0;
+ }
else
tz->trips.active[i].flags.valid = 1;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points
2008-03-11 7:52 ` Zhang, Rui
@ 2008-03-11 21:23 ` Németh Márton
0 siblings, 0 replies; 4+ messages in thread
From: Németh Márton @ 2008-03-11 21:23 UTC (permalink / raw)
To: Zhang, Rui; +Cc: Len Brown, linux-acpi
Zhang, Rui wrote:
> On Tue, 2008-03-11 at 14:35 +0800, Len Brown wrote:
>> On Sunday 09 March 2008, Zhang, Rui wrote:
>>> thermal fixup for broken BIOS which has invalid trip points.
>>> http://bugzilla.kernel.org/show_bug.cgi?id=8544
>>> http://marc.info/?l=linux-kernel&m=120496222629983&w=2
>>>
>>> Signed-off-by: Zhang Rui<rui.zhang@intel.com>
>>> ---
>>> drivers/acpi/thermal.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> Index: linux-2.6/drivers/acpi/thermal.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/acpi/thermal.c
>>> +++ linux-2.6/drivers/acpi/thermal.c
>>> @@ -326,7 +326,9 @@ static int acpi_thermal_set_cooling_mode
>>> #define ACPI_TRIPS_ACTIVE 0x08
>>> #define ACPI_TRIPS_DEVICES 0x10
>>>
>>> -#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE |
>> ACPI_TRIPS_ACTIVE)
>>> +#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE |
>> \
>>> + ACPI_TRIPS_ACTIVE | \
>>> + ACPI_TRIPS_DEVICES)
>>> #define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
>>>
>>> #define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT
>> | \
>>>
>> I don't like re-evaluating _AL0 on notify 0x81 as a workaround
>> to notice that there _is_ no _AL0. We should re-evaluate _AL0
>> on notify x82 -- per the spec.
> I agree.
>> We should print out a single exeception at boot time when
>> we realize that the BIOS has a bug of no _AL0 for the _AC0.
>> At run time, we should have no concept of a valid _AC0
>> and thus a notify x81 should not try to use it.
> OK.
> With the refreshed patch applied, ACPI thermal driver will not try to
> evaluate any invalid trip points for notification 0X81. And a warn
> message is printed out at boot time.
>
> Marton, can you try this patch and attach the dmesg output please? :)
>
> thermal fixup for broken BIOS which has invalid trip points.
>
> ACPI thermal driver only re-evaluate valid trip points.
> For the broken BIOS show in
> http://bugzilla.kernel.org/show_bug.cgi?id=8544
> the active[0] is set to invalid at boot time
> and it will not be re-evaluated again.
> We can still get a warning message at boot time.
>
> http://marc.info/?l=linux-kernel&m=120496222629983&w=2
>
> Signed-off-by: Zhang Rui<rui.zhang@intel.com>
> ---
> drivers/acpi/thermal.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> Index: linux-2.6/drivers/acpi/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/thermal.c
> +++ linux-2.6/drivers/acpi/thermal.c
> @@ -401,7 +401,8 @@ static int acpi_thermal_trips_update(str
> }
>
> /* Passive (optional) */
> - if (flag & ACPI_TRIPS_PASSIVE) {
> + if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
> + (flag == ACPI_TRIPS_INIT)) {
> valid = tz->trips.passive.flags.valid;
> if (psv == -1) {
> status = AE_SUPPORT;
> @@ -440,8 +441,11 @@ static int acpi_thermal_trips_update(str
> memset(&devices, 0, sizeof(struct acpi_handle_list));
> status = acpi_evaluate_reference(tz->device->handle, "_PSL",
> NULL, &devices);
> - if (ACPI_FAILURE(status))
> + if (ACPI_FAILURE(status)) {
> + printk(KERN_WARNING PREFIX
> + "Invalid passive threshold\n");
> tz->trips.passive.flags.valid = 0;
> + }
> else
> tz->trips.passive.flags.valid = 1;
>
> @@ -465,7 +469,8 @@ static int acpi_thermal_trips_update(str
> if (act == -1)
> break; /* disable all active trip points */
>
> - if (flag & ACPI_TRIPS_ACTIVE) {
> + if ((flag == ACPI_TRIPS_INIT) || ((flag & ACPI_TRIPS_ACTIVE) &&
> + tz->trips.active[i].flags.valid)) {
> status = acpi_evaluate_integer(tz->device->handle,
> name, NULL, &tz->trips.active[i].temperature);
> if (ACPI_FAILURE(status)) {
> @@ -497,8 +502,11 @@ static int acpi_thermal_trips_update(str
> memset(&devices, 0, sizeof(struct acpi_handle_list));
> status = acpi_evaluate_reference(tz->device->handle,
> name, NULL, &devices);
> - if (ACPI_FAILURE(status))
> + if (ACPI_FAILURE(status)) {
> + printk(KERN_WARNING PREFIX
> + "Invalid active%d threshold\n", i);
> tz->trips.active[i].flags.valid = 0;
> + }
> else
> tz->trips.active[i].flags.valid = 1;
I tried Linux 2.6.25-rc5 with Clevo D410J. The following message appears:
| ACPI Exception (thermal-0514): AE_ERROR, ACPI thermal trip point state changed
| Please send acpidump to linux-acpi@vger.kernel.org
| [20070126]
I applied this patch to 2.6.25-rc5 on Clevo D410J. I found that the previous
message does not appear anymore.
The third trial I did was to also apply the previous debug patch, so I can
see if there was any events. The result is:
[ 8.955518] ACPI thermal trips update, flag is 1f
[ 8.955686] Checking invalid passive trip point...
[ 8.956254] Checking invalid active[0] trip point...
[ 8.956367] ACPI: Invalid active0 threshold
[ 8.956921] ACPI: LNXTHERM:01 is registered as thermal_zone0
[ 8.957833] ACPI: Thermal Zone [THRM] (57 C)
...
[ 236.669619] ACPI thermal trips update, flag is c
[ 236.669638] Checking valid passive trip point...
[ 236.672311] Checking invalid active[0] trip point...
I think that this patch works good with my hardware.
Márton Németh
--
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] 4+ messages in thread
end of thread, other threads:[~2008-03-11 21:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-10 1:53 [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points Zhang, Rui
2008-03-11 6:35 ` Len Brown
2008-03-11 7:52 ` Zhang, Rui
2008-03-11 21:23 ` Németh Márton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox