Buggy BIOS on the HP TX2500-series

Buggy BIOS on the HP TX2500-series

[Gandalf Kristensen]

Hi.

The HP tx2500-series has a funny "feature" of
trip-point set to -273C. 
(I.e. see:
http://ubuntu-virginia.ubuntuforums.org/showpost.php?p=5660089&postcount=187
)

In the latest kernel I noticed that we (the
tx2500-users) are not the only ones struggling with
this feature. (Wee! Thank you HP!)

So now I would like to find out of what I can do to
help get some patch in to the kernel for this BIOS?

Any help is very much appriciated.


--Gandalf A. Kristensen


      _________________________________________________________
Alt i ett. Få Yahoo! Mail med adressekartotek, kalender og
notisblokk. http://no.mail.yahoo.com
--
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

Re: Buggy BIOS on the HP TX2500-series

[Matthew Garrett]

A patch went into the kernel earlier this year to ignore critical trip 
points that were below 0. Does that not deal with the issue?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Buggy BIOS on the HP tx2500-series

[Gandalf Kristensen]

--- Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> A patch went into the kernel earlier this year to
> ignore critical trip 
> points that were below 0. Does that not deal with
> the issue?
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> 

Ah. One kernel-compile and test-run later, yeah,
you're right. So it does.
Sorry for asking for help with a problem already dealt
with.
Thanks for looking in to this problem though.


--Gandalf A. Kristensen


      _________________________________________________________
Alt i ett. Få Yahoo! Mail med adressekartotek, kalender og
notisblokk. http://no.mail.yahoo.com
--
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

Re: Buggy BIOS on the HP TX2500-series

[Zhang Rui]

On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> A patch went into the kernel earlier this year to ignore critical trip
> points that were below 0.
well, I think this patch is wrong.
a critical trip point below 0 Celsius doesn't mean it's invalid.
windows can work well on this laptop.
please look at:
http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
IMO, we need to fix the ACPICA code first of all.

Ming, what do you think of the patch in comment #15 and #16?

thanks,
rui

>  Does that not deal with the issue?
> 
> --
> Matthew Garrett | mjg59@srcf.ucam.org
> --
> 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

Re: Buggy BIOS on the HP TX2500-series

[Len Brown]

On Thu, 9 Oct 2008, Zhang Rui wrote:

> On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > A patch went into the kernel earlier this year to ignore critical trip
> > points that were below 0.

> well, I think this patch is wrong.

i agree that it treated the symptom and not the cause,
but the patch is still good.

> a critical trip point below 0 Celsius doesn't mean it's invalid.

using human made technology here on Earth, it is indeed invalid;-)

> windows can work well on this laptop.
> please look at:
> http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13

> IMO, we need to fix the ACPICA code first of all.
> 
> Ming, what do you think of the patch in comment #15 and #16?

the root cause of this issue is that the BIOS AML is broken
and uses an implicit return.  Linux broke our windows bug
compatibility on implicit return, and we need to fix that.

-Len

Re: Buggy BIOS on the HP TX2500-series

[Matthew Garrett]

On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > A patch went into the kernel earlier this year to ignore critical trip
> > points that were below 0.
> well, I think this patch is wrong.
> a critical trip point below 0 Celsius doesn't mean it's invalid.

I think it's pretty clear that a critical trip point below 0 celsius 
means that the critical trip point is invalid, though I agree that 
ignoring the entire thermal zone as a result is somewhat unfortunate.

> windows can work well on this laptop.
> please look at:
> http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> IMO, we need to fix the ACPICA code first of all.
> 
> Ming, what do you think of the patch in comment #15 and #16?

We could quibble over the technical correctness of this approach, but it 
seems to behave in exactly the same way - ie, Linux will ignore the 
thermal zone? The existing code seems fine, other than the fact that a 
bad _CRT will result everything failing. I think we'd be better off just 
losing the return -ENODEV there and try to use as much of the thermal 
information as we can.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Buggy BIOS on the HP TX2500-series

[Len Brown]

On Thu, 9 Oct 2008, Matthew Garrett wrote:

> On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > A patch went into the kernel earlier this year to ignore critical trip
> > > points that were below 0.
> > well, I think this patch is wrong.
> > a critical trip point below 0 Celsius doesn't mean it's invalid.
> 
> I think it's pretty clear that a critical trip point below 0 celsius 
> means that the critical trip point is invalid, though I agree that 
> ignoring the entire thermal zone as a result is somewhat unfortunate.
> 
> > windows can work well on this laptop.
> > please look at:
> > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > IMO, we need to fix the ACPICA code first of all.
> > 
> > Ming, what do you think of the patch in comment #15 and #16?
> 
> We could quibble over the technical correctness of this approach, but it 
> seems to behave in exactly the same way - ie, Linux will ignore the 
> thermal zone? The existing code seems fine, other than the fact that a 
> bad _CRT will result everything failing. I think we'd be better off just 
> losing the return -ENODEV there and try to use as much of the thermal 
> information as we can.

right, when we put in the workaround we observed that a bad _CRT
would delete an entire thermal zone, and that could be a big
problem on a box with active cooling on that thermal zone.

of course if we are successful on the implicit return front,
this becomes unnecessary.

-Len

RE: Buggy BIOS on the HP TX2500-series

[Moore, Robert]

I believe that our "fix" for implicit return to make it Windows compatible should resolve the issue.

-----Original Message-----
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Len Brown
Sent: Wednesday, October 08, 2008 7:05 PM
To: Matthew Garrett
Cc: Zhang, Rui; Gandalf Kristensen; linux-acpi@vger.kernel.org; Rafael J. Wysocki; Lin, Ming M
Subject: Re: Buggy BIOS on the HP TX2500-series



On Thu, 9 Oct 2008, Matthew Garrett wrote:

> On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > A patch went into the kernel earlier this year to ignore critical trip
> > > points that were below 0.
> > well, I think this patch is wrong.
> > a critical trip point below 0 Celsius doesn't mean it's invalid.
>
> I think it's pretty clear that a critical trip point below 0 celsius
> means that the critical trip point is invalid, though I agree that
> ignoring the entire thermal zone as a result is somewhat unfortunate.
>
> > windows can work well on this laptop.
> > please look at:
> > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > IMO, we need to fix the ACPICA code first of all.
> >
> > Ming, what do you think of the patch in comment #15 and #16?
>
> We could quibble over the technical correctness of this approach, but it
> seems to behave in exactly the same way - ie, Linux will ignore the
> thermal zone? The existing code seems fine, other than the fact that a
> bad _CRT will result everything failing. I think we'd be better off just
> losing the return -ENODEV there and try to use as much of the thermal
> information as we can.

right, when we put in the workaround we observed that a bad _CRT
would delete an entire thermal zone, and that could be a big
problem on a box with active cooling on that thermal zone.

of course if we are successful on the implicit return front,
this becomes unnecessary.

-Len

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

RE: Buggy BIOS on the HP TX2500-series

[Zhang Rui]

On Wed, 2008-10-08 at 21:16 -0700, Moore, Robert wrote:
> I believe that our "fix" for implicit return to make it Windows compatible should resolve the issue.
> 
hmm, what's the fix?
what's the return value of acpi_evaluate_integer("_CRT"), 0 or an error
code?

thanks,
rui

> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Len Brown
> Sent: Wednesday, October 08, 2008 7:05 PM
> To: Matthew Garrett
> Cc: Zhang, Rui; Gandalf Kristensen; linux-acpi@vger.kernel.org; Rafael J. Wysocki; Lin, Ming M
> Subject: Re: Buggy BIOS on the HP TX2500-series
> 
> 
> 
> On Thu, 9 Oct 2008, Matthew Garrett wrote:
> 
> > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > > A patch went into the kernel earlier this year to ignore critical trip
> > > > points that were below 0.
> > > well, I think this patch is wrong.
> > > a critical trip point below 0 Celsius doesn't mean it's invalid.
> >
> > I think it's pretty clear that a critical trip point below 0 celsius
> > means that the critical trip point is invalid, though I agree that
> > ignoring the entire thermal zone as a result is somewhat unfortunate.
> >
> > > windows can work well on this laptop.
> > > please look at:
> > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > > IMO, we need to fix the ACPICA code first of all.
> > >
> > > Ming, what do you think of the patch in comment #15 and #16?
> >
> > We could quibble over the technical correctness of this approach, but it
> > seems to behave in exactly the same way - ie, Linux will ignore the
> > thermal zone? The existing code seems fine, other than the fact that a
> > bad _CRT will result everything failing. I think we'd be better off just
> > losing the return -ENODEV there and try to use as much of the thermal
> > information as we can.
> 
> right, when we put in the workaround we observed that a bad _CRT
> would delete an entire thermal zone, and that could be a big
> problem on a box with active cooling on that thermal zone.
> 
> of course if we are successful on the implicit return front,
> this becomes unnecessary.
> 
> -Len
> 
> --
> 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

RE: Buggy BIOS on the HP TX2500-series

[Lin Ming]

On Wed, 2008-10-08 at 23:39 -0700, Zhang, Rui wrote:
> On Wed, 2008-10-08 at 21:16 -0700, Moore, Robert wrote:
> > I believe that our "fix" for implicit return to make it Windows compatible should resolve the issue.
> >
> hmm, what's the fix?
> what's the return value of acpi_evaluate_integer("_CRT"), 0 or an error
> code?

Take below _CRT as example, it returns 0(predicate value of
LLess) with the latest CA code.

       Method (_CRT, 0, Serialized)
       {
           If (LLess (OSYS, 0x07D6))
           {
               If (LEqual (\_SB.TJ85, Zero))
               {
                   Return (Add (0x0AAC, Multiply (TPC, 0x0A)))
               }
               Else
               {
                   Return (Add (0x0AAC, Multiply (TP85, 0x0A)))
               }
           }
       }

Lin Ming

> 
> thanks,
> rui
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Len Brown
> > Sent: Wednesday, October 08, 2008 7:05 PM
> > To: Matthew Garrett
> > Cc: Zhang, Rui; Gandalf Kristensen; linux-acpi@vger.kernel.org; Rafael J. Wysocki; Lin, Ming M
> > Subject: Re: Buggy BIOS on the HP TX2500-series
> >
> >
> >
> > On Thu, 9 Oct 2008, Matthew Garrett wrote:
> >
> > > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > > > A patch went into the kernel earlier this year to ignore critical trip
> > > > > points that were below 0.
> > > > well, I think this patch is wrong.
> > > > a critical trip point below 0 Celsius doesn't mean it's invalid.
> > >
> > > I think it's pretty clear that a critical trip point below 0 celsius
> > > means that the critical trip point is invalid, though I agree that
> > > ignoring the entire thermal zone as a result is somewhat unfortunate.
> > >
> > > > windows can work well on this laptop.
> > > > please look at:
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > > > IMO, we need to fix the ACPICA code first of all.
> > > >
> > > > Ming, what do you think of the patch in comment #15 and #16?
> > >
> > > We could quibble over the technical correctness of this approach, but it
> > > seems to behave in exactly the same way - ie, Linux will ignore the
> > > thermal zone? The existing code seems fine, other than the fact that a
> > > bad _CRT will result everything failing. I think we'd be better off just
> > > losing the return -ENODEV there and try to use as much of the thermal
> > > information as we can.
> >
> > right, when we put in the workaround we observed that a bad _CRT
> > would delete an entire thermal zone, and that could be a big
> > problem on a box with active cooling on that thermal zone.
> >
> > of course if we are successful on the implicit return front,
> > this becomes unnecessary.
> >
> > -Len
> >
> > --
> > 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
> 

Re: Buggy BIOS on the HP TX2500-series

[Zhang Rui]

On Wed, 2008-10-08 at 19:04 -0700, Len Brown wrote:
> 
> On Thu, 9 Oct 2008, Matthew Garrett wrote:
> 
> > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > > A patch went into the kernel earlier this year to ignore critical trip
> > > > points that were below 0.
> > > well, I think this patch is wrong.
> > > a critical trip point below 0 Celsius doesn't mean it's invalid.
> >
> > I think it's pretty clear that a critical trip point below 0 celsius
> > means that the critical trip point is invalid, 
well, I agree that this workaround can work here.
But I think the proper way to fix this issue is that,
ACPICA returns some error code(AE_BAD_DATA) like it did before,
and the thermal driver knows that it got an invalid value and
should take it carefully.
or else we may still get potential problems, e.g. what if there
is no return value of _PSV method.

> though I agree that
> > ignoring the entire thermal zone as a result is somewhat unfortunate.
> >
> > > windows can work well on this laptop.
> > > please look at:
> > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > > IMO, we need to fix the ACPICA code first of all.
> > >
> > > Ming, what do you think of the patch in comment #15 and #16?
> >
> > We could quibble over the technical correctness of this approach, but it
> > seems to behave in exactly the same way - ie, Linux will ignore the
> > thermal zone? The existing code seems fine, other than the fact that a
> > bad _CRT will result everything failing. I think we'd be better off just
> > losing the return -ENODEV there and try to use as much of the thermal
> > information as we can.
> 
> right, when we put in the workaround we observed that a bad _CRT
> would delete an entire thermal zone, and that could be a big
> problem on a box with active cooling on that thermal zone.
Hmm, I think a patch like this is enough to fix this problem.


ignore invalid critical trip point.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/thermal.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6/drivers/acpi/thermal.c
===================================================================
--- linux-2.6.orig/drivers/acpi/thermal.c
+++ linux-2.6/drivers/acpi/thermal.c
@@ -373,9 +373,8 @@ static int acpi_thermal_trips_update(str
 		if (ACPI_FAILURE(status) ||
 				tz->trips.critical.temperature <= 2732) {
 			tz->trips.critical.flags.valid = 0;
-			ACPI_EXCEPTION((AE_INFO, status,
+			ACPI_DEBUG_PRINT((ACPI_DB_WARN,
 					"No or invalid critical threshold"));
-			return -ENODEV;
 		} else {
 			tz->trips.critical.flags.valid = 1;
 			ACPI_DEBUG_PRINT((ACPI_DB_INFO,

Re: Buggy BIOS on the HP TX2500-series

[Zhang Rui]

On Wed, 2008-10-08 at 23:36 -0700, Zhang Rui wrote:
> On Wed, 2008-10-08 at 19:04 -0700, Len Brown wrote:
> >
> > On Thu, 9 Oct 2008, Matthew Garrett wrote:
> >
> > > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > > > A patch went into the kernel earlier this year to ignore critical trip
> > > > > points that were below 0.
> > > > well, I think this patch is wrong.
> > > > a critical trip point below 0 Celsius doesn't mean it's invalid.
> > >
> > > I think it's pretty clear that a critical trip point below 0 celsius
> > > means that the critical trip point is invalid,
> well, I agree that this workaround can work here.
> But I think the proper way to fix this issue is that,
> ACPICA returns some error code(AE_BAD_DATA) like it did before,
> and the thermal driver knows that it got an invalid value and
> should take it carefully.
Bob, do you mean Windows return 0 in this case?

> or else we may still get potential problems, e.g. what if there
> is no return value of _PSV method.
If it's true, then we don't need to waste our time on this.
it won't happen because windows doesn't work either in this case, unless
it workarounds this problem again like it has done for _CRT, right?

thanks,
rui
> 
> > though I agree that
> > > ignoring the entire thermal zone as a result is somewhat unfortunate.
> > >
> > > > windows can work well on this laptop.
> > > > please look at:
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > > > IMO, we need to fix the ACPICA code first of all.
> > > >
> > > > Ming, what do you think of the patch in comment #15 and #16?
> > >
> > > We could quibble over the technical correctness of this approach, but it
> > > seems to behave in exactly the same way - ie, Linux will ignore the
> > > thermal zone? The existing code seems fine, other than the fact that a
> > > bad _CRT will result everything failing. I think we'd be better off just
> > > losing the return -ENODEV there and try to use as much of the thermal
> > > information as we can.
> >
> > right, when we put in the workaround we observed that a bad _CRT
> > would delete an entire thermal zone, and that could be a big
> > problem on a box with active cooling on that thermal zone.
> Hmm, I think a patch like this is enough to fix this problem.
> 
> 
> ignore invalid critical trip point.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/thermal.c
> +++ linux-2.6/drivers/acpi/thermal.c
> @@ -373,9 +373,8 @@ static int acpi_thermal_trips_update(str
>                 if (ACPI_FAILURE(status) ||
>                                 tz->trips.critical.temperature <= 2732) {
>                         tz->trips.critical.flags.valid = 0;
> -                       ACPI_EXCEPTION((AE_INFO, status,
> +                       ACPI_DEBUG_PRINT((ACPI_DB_WARN,
>                                         "No or invalid critical threshold"));
> -                       return -ENODEV;
>                 } else {
>                         tz->trips.critical.flags.valid = 1;
>                         ACPI_DEBUG_PRINT((ACPI_DB_INFO,
> 
> 
> --
> 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

RE: Buggy BIOS on the HP TX2500-series

[Moore, Robert]

Yes, windows returns zero in this case, with no error.

Therefore, we have achieved windows compatibility, which is one of the main goals for ACPICA at this time.



>-----Original Message-----
>From: Zhang, Rui
>Sent: Wednesday, October 08, 2008 11:45 PM
>To: Len Brown; Moore, Robert
>Cc: Matthew Garrett; Gandalf Kristensen; linux-acpi@vger.kernel.org; Rafael
>J. Wysocki; Lin, Ming M
>Subject: Re: Buggy BIOS on the HP TX2500-series
>
>On Wed, 2008-10-08 at 23:36 -0700, Zhang Rui wrote:
>> On Wed, 2008-10-08 at 19:04 -0700, Len Brown wrote:
>> >
>> > On Thu, 9 Oct 2008, Matthew Garrett wrote:
>> >
>> > > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
>> > > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
>> > > > > A patch went into the kernel earlier this year to ignore critical
>trip
>> > > > > points that were below 0.
>> > > > well, I think this patch is wrong.
>> > > > a critical trip point below 0 Celsius doesn't mean it's invalid.
>> > >
>> > > I think it's pretty clear that a critical trip point below 0 celsius
>> > > means that the critical trip point is invalid,
>> well, I agree that this workaround can work here.
>> But I think the proper way to fix this issue is that,
>> ACPICA returns some error code(AE_BAD_DATA) like it did before,
>> and the thermal driver knows that it got an invalid value and
>> should take it carefully.
>Bob, do you mean Windows return 0 in this case?
>
>> or else we may still get potential problems, e.g. what if there
>> is no return value of _PSV method.
>If it's true, then we don't need to waste our time on this.
>it won't happen because windows doesn't work either in this case, unless
>it workarounds this problem again like it has done for _CRT, right?
>
>thanks,
>rui
>>
>> > though I agree that
>> > > ignoring the entire thermal zone as a result is somewhat unfortunate.
>> > >
>> > > > windows can work well on this laptop.
>> > > > please look at:
>> > > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
>> > > > IMO, we need to fix the ACPICA code first of all.
>> > > >
>> > > > Ming, what do you think of the patch in comment #15 and #16?
>> > >
>> > > We could quibble over the technical correctness of this approach, but
>it
>> > > seems to behave in exactly the same way - ie, Linux will ignore the
>> > > thermal zone? The existing code seems fine, other than the fact that
>a
>> > > bad _CRT will result everything failing. I think we'd be better off
>just
>> > > losing the return -ENODEV there and try to use as much of the thermal
>> > > information as we can.
>> >
>> > right, when we put in the workaround we observed that a bad _CRT
>> > would delete an entire thermal zone, and that could be a big
>> > problem on a box with active cooling on that thermal zone.
>> Hmm, I think a patch like this is enough to fix this problem.
>>
>>
>> ignore invalid critical trip point.
>>
>> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>> ---
>>  drivers/acpi/thermal.c |    3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> Index: linux-2.6/drivers/acpi/thermal.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/thermal.c
>> +++ linux-2.6/drivers/acpi/thermal.c
>> @@ -373,9 +373,8 @@ static int acpi_thermal_trips_update(str
>>                 if (ACPI_FAILURE(status) ||
>>                                 tz->trips.critical.temperature <= 2732) {
>>                         tz->trips.critical.flags.valid = 0;
>> -                       ACPI_EXCEPTION((AE_INFO, status,
>> +                       ACPI_DEBUG_PRINT((ACPI_DB_WARN,
>>                                         "No or invalid critical
>threshold"));
>> -                       return -ENODEV;
>>                 } else {
>>                         tz->trips.critical.flags.valid = 1;
>>                         ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>>
>>
>> --
>> 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

Re: Buggy BIOS on the HP TX2500-series

[Thomas Renninger]

On Thursday 09 October 2008 08:36:56 Zhang Rui wrote:
> On Wed, 2008-10-08 at 19:04 -0700, Len Brown wrote:
> > On Thu, 9 Oct 2008, Matthew Garrett wrote:
> > > On Thu, Oct 09, 2008 at 08:59:15AM +0800, Zhang Rui wrote:
> > > > On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > > > > A patch went into the kernel earlier this year to ignore critical
> > > > > trip points that were below 0.
> > > >
> > > > well, I think this patch is wrong.
> > > > a critical trip point below 0 Celsius doesn't mean it's invalid.
> > >
> > > I think it's pretty clear that a critical trip point below 0 celsius
> > > means that the critical trip point is invalid,
>
> well, I agree that this workaround can work here.
> But I think the proper way to fix this issue is that,
> ACPICA returns some error code(AE_BAD_DATA) like it did before,
> and the thermal driver knows that it got an invalid value and
> should take it carefully.
> or else we may still get potential problems, e.g. what if there
> is no return value of _PSV method.
>
> > though I agree that
> >
> > > ignoring the entire thermal zone as a result is somewhat unfortunate.
> > >
> > > > windows can work well on this laptop.
> > > > please look at:
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> > > > IMO, we need to fix the ACPICA code first of all.
> > > >
> > > > Ming, what do you think of the patch in comment #15 and #16?
> > >
> > > We could quibble over the technical correctness of this approach, but
> > > it seems to behave in exactly the same way - ie, Linux will ignore the
> > > thermal zone? The existing code seems fine, other than the fact that a
> > > bad _CRT will result everything failing. I think we'd be better off
> > > just losing the return -ENODEV there and try to use as much of the
> > > thermal information as we can.
> >
> > right, when we put in the workaround we observed that a bad _CRT
> > would delete an entire thermal zone, and that could be a big
> > problem on a box with active cooling on that thermal zone.
>
> Hmm, I think a patch like this is enough to fix this problem.
>
>
> ignore invalid critical trip point.
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/thermal.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Index: linux-2.6/drivers/acpi/thermal.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/thermal.c
> +++ linux-2.6/drivers/acpi/thermal.c
> @@ -373,9 +373,8 @@ static int acpi_thermal_trips_update(str
>  		if (ACPI_FAILURE(status) ||
>  				tz->trips.critical.temperature <= 2732) {
>  			tz->trips.critical.flags.valid = 0;
> -			ACPI_EXCEPTION((AE_INFO, status,
> +			ACPI_DEBUG_PRINT((ACPI_DB_WARN,
>  					"No or invalid critical threshold"));
> -			return -ENODEV;
I agree with not returning -ENODEV which I expect to only invalidate the
critical trip point and still let the thermal zone do it's (possibly 
important) job.

I DO NOT AGREE with removing the exception!
These corner cases will always break again and again sooner or later. Totally 
hiding this message will cause a lot grief to people who are going to debug 
thermal issues on their machine two kernel versions later when some bad side 
effect exposes caused by this crappy BIOS and people will lose precious time, 
because this was hidden.

Also this BIOS should never get a Novell or whatever Linux distro certified 
sticker. Our certification people try hard to identify such BIOS bugs and 
reject such BIOSes and force the vendors to fix it. Guessing Windows behavior 
and try to support it is not an option, because history showed that such 
Windows compatibility efforts can take years until things are adopted/guessed 
in the right way.

I am going to resend my [Firmware Bug] interface using this one as a first 
instance to use it. It's the classic example why it's urgently needed.
Once it's in, I promise to adopt other parts, so that it's meaningful to do:
dmesg |grep "[Firmware Bug]"

Have you seen that Len?
Hmm, I forgot to add the acpi list on the latest version, I only added the 
cpufreq list. I am going to resend them. On the cpufreq list it was:
Subject: Resend: Introduce interface to report BIOS bugs (reworked: FW_BUG 
simple solution, large description)

Thanks,

     Thomas

Re: Buggy BIOS on the HP TX2500-series

[Lin Ming]

On Wed, 2008-10-08 at 17:59 -0700, Zhang, Rui wrote:
> On Wed, 2008-10-08 at 12:26 -0700, Matthew Garrett wrote:
> > A patch went into the kernel earlier this year to ignore critical trip
> > points that were below 0.
> well, I think this patch is wrong.
> a critical trip point below 0 Celsius doesn't mean it's invalid.
> windows can work well on this laptop.
> please look at:
> http://bugzilla.kernel.org/show_bug.cgi?id=10686#c13
> IMO, we need to fix the ACPICA code first of all.
> 
> Ming, what do you think of the patch in comment #15 and #16?

We have a ACPICA patch at:
http://git.acpica.org/repos/?p=acpica.git;a=commitdiff;h=ab557ef061978bd54baa05daca33ea16942629fa 

        Method (_CRT, 0, Serialized)
        {
            If (LLess (OSYS, 0x07D6))
            {
                If (LEqual (\_SB.TJ85, Zero))
                {
                    Return (Add (0x0AAC, Multiply (TPC, 0x0A)))
                }
                Else
                {
                    Return (Add (0x0AAC, Multiply (TP85, 0x0A)))
                }
            }
        }
    
Previously _CRT returns 0x07D6, now it returns 0 (predicate value of
LLess) which matches windows behavior.

So I felt it's better for the external interface (the driver) to check
the return value (zero) and decide what to do.

Thanks,
Lin Ming