* [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