Re: [PATCH] ACPI: thermal fixup for broken BIOS which has invalid trip points

["Németh Márton" <nm127@freemail.hu>]

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