PROBLEM: (regression) thermal zone - fan is turned on at higher temperature than it should

PATCH Vladimir Zajac 4 participants 5 messages May 5, 2009 – May 10, 2009

Subsystems (2)

ACPI Thermal

lore

Share & tools

lore

Vladimir Zajac

author

2009-05-05 19:05 UTC #0 lore raw

[1.] One line summary of the problem:
(regression) thermal zone - fan is turned on at higher temperature than it should

[2.] Full description of the problem/report:
After upgrade to 2.6.30-rc4 I noticed that my laptop's fan sometimes runs at lower speed than it used to. 2.6.29 does not have the bug.

The laptop (HP Compaq 6710b) has 5 ACPI cooling devices which correspond to different fan speeds. They are not turned on when I expect them to be.

Expected behavior:
Each active cooling device should be turned on when current temperature is equal or higher than its trip point. (This is how it worked in 2.6.29 and earlier versions and also how it is defined by ACPI specification.)

Actual behavior in 2.6.30-rc4:
Each cooling device is turned on only when current temperature is strictly greater than the corresponding trip point but not when current temperature is equal to the trip point.

It is caused by commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e:

author Matthew Garrett <[email protected]>

	Wed, 3 Dec 2008 17:55:32 +0000 (17:55 +0000)
committer	Len Brown <[email protected]>
	Fri, 20 Feb 2009 23:41:56 +0000 (18:41 -0500)

commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e
ACPI: move thermal trip handling to generic thermal layer

This commit (probably unintentionally) also changed how kernel determines if a cooling device has to be activated:
before the commit: cooling device is activated if current temperature >= trip point temperature
after the commit: cooling device is activated if current temperature > trip point temperature

I tried to change the trip point test back to "temp >= trip_temp" in drivers/thermal/termal_sys.c (attached patch). It fixes the issue on my laptop.

[3.] Keywords (i.e., modules, networking, kernel):
acpi, thermal zone

[4.] Kernel information
[4.1.] Kernel version (from /proc/version):
Linux version 2.6.30-rc4-64v01kms (root@V2G64) (gcc version 4.3.2 (Gentoo 4.3.2-r3 p1.6, pie-10.1.5) ) #2 SMP PREEMPT Fri May 1 00:11:41 CEST 2009

[5.] Most recent kernel version which did not have the bug:
2.6.29

[X.] Other notes, patches, fixes, workarounds:

patch which should fix the issue(for 2.6.30-rc4):

drivers/thermal/thermal_sys.c +4 -4

@@ -961,7 +961,7 @@ void thermal_zone_device_update(struct t
 
 		switch (trip_type) {
 		case THERMAL_TRIP_CRITICAL:
-			if (temp > trip_temp) {
+			if (temp >= trip_temp) {
 				if (tz->ops->notify)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);

@@ -974,7 +974,7 @@ void thermal_zone_device_update(struct t
 			}
 			break;
 		case THERMAL_TRIP_HOT:
-			if (temp > trip_temp)
+			if (temp >= trip_temp)
 				if (tz->ops->notify)
 					tz->ops->notify(tz, count, trip_type);
 			break;

@@ -986,14 +986,14 @@ void thermal_zone_device_update(struct t
 
 				cdev = instance->cdev;
 
-				if (temp > trip_temp)
+				if (temp >= trip_temp)
 					cdev->ops->set_cur_state(cdev, 1);
 				else
 					cdev->ops->set_cur_state(cdev, 0);
 			}
 			break;
 		case THERMAL_TRIP_PASSIVE:
-			if (temp > trip_temp || tz->passive)
+			if (temp >= trip_temp || tz->passive)
 				thermal_zone_device_passive(tz, temp,
 							    trip_temp, count);
 			break;

yakui_zhao

2009-05-06 00:45 UTC #1 lore raw

Quoted 22 lines · Vladimir Zajac

[1.] One line summary of the problem:
(regression) thermal zone - fan is turned on at higher temperature than it should
[2.] Full description of the problem/report:
After upgrade to 2.6.30-rc4 I noticed that my laptop's fan sometimes runs at lower speed than it used to. 2.6.29 does not have the bug.
The laptop (HP Compaq 6710b) has 5 ACPI cooling devices which correspond to different fan speeds. They are not turned on when I expect them to be.
Expected behavior:
Each active cooling device should be turned on when current temperature is equal or higher than its trip point. (This is how it worked in 2.6.29 and earlier versions and also how it is defined by ACPI specification.)
Actual behavior in 2.6.30-rc4:
Each cooling device is turned on only when current temperature is strictly greater than the corresponding trip point but not when current temperature is equal to the trip point.
It is caused by commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e:
author Matthew Garrett <[email protected]>
	Wed, 3 Dec 2008 17:55:32 +0000 (17:55 +0000)
committer	Len Brown <[email protected]>
	Fri, 20 Feb 2009 23:41:56 +0000 (18:41 -0500)

Quoted 7 lines

commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e
ACPI: move thermal trip handling to generic thermal layer
This commit (probably unintentionally) also changed how kernel determines if a cooling device has to be activated:
before the commit: cooling device is activated if current temperature >= trip point temperature
after the commit: cooling device is activated if current temperature > trip point temperature

Do you mean that the issue can be fixed by changing the "temperature > trip point" to "temperature >= trip point"?

If so, what you have done is reasonable.

Thanks.

Quoted 58 lines

I tried to change the trip point test back to "temp >= trip_temp" in drivers/thermal/termal_sys.c (attached patch). It fixes the issue on my laptop.
[3.] Keywords (i.e., modules, networking, kernel):
acpi, thermal zone
[4.] Kernel information
[4.1.] Kernel version (from /proc/version):
Linux version 2.6.30-rc4-64v01kms (root@V2G64) (gcc version 4.3.2 (Gentoo 4.3.2-r3 p1.6, pie-10.1.5) ) #2 SMP PREEMPT Fri May 1 00:11:41 CEST 2009
[5.] Most recent kernel version which did not have the bug:
2.6.29
[X.] Other notes, patches, fixes, workarounds:
patch which should fix the issue(for 2.6.30-rc4):
drivers/thermal/thermal_sys.c +4 -5
@@ -961,7 +961,7 @@ void thermal_zone_device_update(struct t
 
 		switch (trip_type) {
 		case THERMAL_TRIP_CRITICAL:
-			if (temp > trip_temp) {
+			if (temp >= trip_temp) {
 				if (tz->ops->notify)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);
@@ -974,7 +974,7 @@ void thermal_zone_device_update(struct t
 			}
 			break;
 		case THERMAL_TRIP_HOT:
-			if (temp > trip_temp)
+			if (temp >= trip_temp)
 				if (tz->ops->notify)
 					tz->ops->notify(tz, count, trip_type);
 			break;
@@ -986,14 +986,14 @@ void thermal_zone_device_update(struct t
 
 				cdev = instance->cdev;
 
-				if (temp > trip_temp)
+				if (temp >= trip_temp)
 					cdev->ops->set_cur_state(cdev, 1);
 				else
 					cdev->ops->set_cur_state(cdev, 0);
 			}
 			break;
 		case THERMAL_TRIP_PASSIVE:
-			if (temp > trip_temp || tz->passive)
+			if (temp >= trip_temp || tz->passive)
 				thermal_zone_device_passive(tz, temp,
 							    trip_temp, count);
 			break;
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Vladimir Zajac

author

2009-05-06 05:50 UTC #2 lore raw

Quoted 27 lines · On Wednesday 06 May 2009 02:46:52 you wrote:

Quoted 26 lines · Vladimir Zajac
[1.] One line summary of the problem:
(regression) thermal zone - fan is turned on at higher temperature than it should
[2.] Full description of the problem/report:
After upgrade to 2.6.30-rc4 I noticed that my laptop's fan sometimes runs at lower speed than it used to. 2.6.29 does not have the bug.
The laptop (HP Compaq 6710b) has 5 ACPI cooling devices which correspond to different fan speeds. They are not turned on when I expect them to be.
Expected behavior:
Each active cooling device should be turned on when current temperature is equal or higher than its trip point. (This is how it worked in 2.6.29 and earlier versions and also how it is defined by ACPI specification.)
Actual behavior in 2.6.30-rc4:
Each cooling device is turned on only when current temperature is strictly greater than the corresponding trip point but not when current temperature is equal to the trip point.
It is caused by commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e:
author Matthew Garrett <[email protected]>
	Wed, 3 Dec 2008 17:55:32 +0000 (17:55 +0000)
committer	Len Brown <[email protected]>
	Fri, 20 Feb 2009 23:41:56 +0000 (18:41 -0500)
commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e
ACPI: move thermal trip handling to generic thermal layer

Quoted 8 lines

Quoted 3 lines
This commit (probably unintentionally) also changed how kernel determines if a cooling device has to be activated:
before the commit: cooling device is activated if current temperature >= trip point temperature
after the commit: cooling device is activated if current temperature > trip point temperature
Do you mean that the issue can be fixed by changing the "temperature > trip point" to "temperature >= trip point"?
If so, what you have done is reasonable.
Thanks.

Yes, it can be fixed by changing the "temperature > trip point" to "temperature >= trip point".

Quoted 60 lines

Quoted 58 lines
I tried to change the trip point test back to "temp >= trip_temp" in drivers/thermal/termal_sys.c (attached patch). It fixes the issue on my laptop.
[3.] Keywords (i.e., modules, networking, kernel):
acpi, thermal zone
[4.] Kernel information
[4.1.] Kernel version (from /proc/version):
Linux version 2.6.30-rc4-64v01kms (root@V2G64) (gcc version 4.3.2 (Gentoo 4.3.2-r3 p1.6, pie-10.1.5) ) #2 SMP PREEMPT Fri May 1 00:11:41 CEST 2009
[5.] Most recent kernel version which did not have the bug:
2.6.29
[X.] Other notes, patches, fixes, workarounds:
patch which should fix the issue(for 2.6.30-rc4):
drivers/thermal/thermal_sys.c +4 -5
@@ -961,7 +961,7 @@ void thermal_zone_device_update(struct t
 
 		switch (trip_type) {
 		case THERMAL_TRIP_CRITICAL:
-			if (temp > trip_temp) {
+			if (temp >= trip_temp) {
 				if (tz->ops->notify)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);
@@ -974,7 +974,7 @@ void thermal_zone_device_update(struct t
 			}
 			break;
 		case THERMAL_TRIP_HOT:
-			if (temp > trip_temp)
+			if (temp >= trip_temp)
 				if (tz->ops->notify)
 					tz->ops->notify(tz, count, trip_type);
 			break;
@@ -986,14 +986,14 @@ void thermal_zone_device_update(struct t
 
 				cdev = instance->cdev;
 
-				if (temp > trip_temp)
+				if (temp >= trip_temp)
 					cdev->ops->set_cur_state(cdev, 1);
 				else
 					cdev->ops->set_cur_state(cdev, 0);
 			}
 			break;
 		case THERMAL_TRIP_PASSIVE:
-			if (temp > trip_temp || tz->passive)
+			if (temp >= trip_temp || tz->passive)
 				thermal_zone_device_passive(tz, temp,
 							    trip_temp, count);
 			break;
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Zhang Rui

reviewer Acked-by

2009-05-06 00:59 UTC #3 lore raw

In reply to Vladimir Zajac

Hi, Vladimir

Quoted 39 lines · Vladimir Zajac

[1.] One line summary of the problem:
(regression) thermal zone - fan is turned on at higher temperature than it should
[2.] Full description of the problem/report:
After upgrade to 2.6.30-rc4 I noticed that my laptop's fan sometimes runs at lower speed than it used to. 2.6.29 does not have the bug.
The laptop (HP Compaq 6710b) has 5 ACPI cooling devices which correspond to different fan speeds. They are not turned on when I expect them to be.
Expected behavior:
Each active cooling device should be turned on when current temperature is equal or higher than its trip point. (This is how it worked in 2.6.29 and earlier versions and also how it is defined by ACPI specification.)
Actual behavior in 2.6.30-rc4:
Each cooling device is turned on only when current temperature is strictly greater than the corresponding trip point but not when current temperature is equal to the trip point.
It is caused by commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e:
author Matthew Garrett <[email protected]>
	Wed, 3 Dec 2008 17:55:32 +0000 (17:55 +0000)
committer	Len Brown <[email protected]>
	Fri, 20 Feb 2009 23:41:56 +0000 (18:41 -0500)
commit b1569e99c795bf83b4ddf41c4f1c42761ab7f75e
ACPI: move thermal trip handling to generic thermal layer
This commit (probably unintentionally) also changed how kernel determines if a cooling device has to be activated:
before the commit: cooling device is activated if current temperature >= trip point temperature
after the commit: cooling device is activated if current temperature > trip point temperature
I tried to change the trip point test back to "temp >= trip_temp" in drivers/thermal/termal_sys.c (attached patch). It fixes the issue on my laptop.
[3.] Keywords (i.e., modules, networking, kernel):
acpi, thermal zone
[4.] Kernel information
[4.1.] Kernel version (from /proc/version):
Linux version 2.6.30-rc4-64v01kms (root@V2G64) (gcc version 4.3.2 (Gentoo 4.3.2-r3 p1.6, pie-10.1.5) ) #2 SMP PREEMPT Fri May 1 00:11:41 CEST 2009

Quoted 6 lines

[5.] Most recent kernel version which did not have the bug:
2.6.29
[X.] Other notes, patches, fixes, workarounds:
patch which should fix the issue(for 2.6.30-rc4):

Thanks for finding the problem and fixing it.

But I think you must want to resend the patch with correct format so that Len can pick it up.

Acked-by Zhang Rui <[email protected]>

Quoted 38 lines

drivers/thermal/thermal_sys.c +4 -4

@@ -961,7 +961,7 @@ void thermal_zone_device_update(struct t
 
 		switch (trip_type) {
 		case THERMAL_TRIP_CRITICAL:
-			if (temp > trip_temp) {
+			if (temp >= trip_temp) {
 				if (tz->ops->notify)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);

@@ -974,7 +974,7 @@ void thermal_zone_device_update(struct t
 			}
 			break;
 		case THERMAL_TRIP_HOT:
-			if (temp > trip_temp)
+			if (temp >= trip_temp)
 				if (tz->ops->notify)
 					tz->ops->notify(tz, count, trip_type);
 			break;

@@ -986,14 +986,14 @@ void thermal_zone_device_update(struct t
 
 				cdev = instance->cdev;
 
-				if (temp > trip_temp)
+				if (temp >= trip_temp)
 					cdev->ops->set_cur_state(cdev, 1);
 				else
 					cdev->ops->set_cur_state(cdev, 0);
 			}
 			break;
 		case THERMAL_TRIP_PASSIVE:
-			if (temp > trip_temp || tz->passive)
+			if (temp >= trip_temp || tz->passive)
 				thermal_zone_device_passive(tz, temp,
 							    trip_temp, count);
 			break;

Matthew Garrett

Acked-by

2009-05-10 02:26 UTC #4 lore raw

In reply to Vladimir Zajac

Quoted 27 lines · Vladimir Zajac

drivers/thermal/thermal_sys.c +3 -3

@@ -961,7 +961,7 @@ void thermal_zone_device_update(struct t
 
 		switch (trip_type) {
 		case THERMAL_TRIP_CRITICAL:
-			if (temp > trip_temp) {
+			if (temp >= trip_temp) {
 				if (tz->ops->notify)
 					ret = tz->ops->notify(tz, count,
 							      trip_type);

@@ -974,7 +974,7 @@ void thermal_zone_device_update(struct t
 			}
 			break;
 		case THERMAL_TRIP_HOT:
-			if (temp > trip_temp)
+			if (temp >= trip_temp)
 				if (tz->ops->notify)
 					tz->ops->notify(tz, count, trip_type);
 			break;

@@ -986,14 +986,14 @@ void thermal_zone_device_update(struct t
 
 				cdev = instance->cdev;
 
-				if (temp > trip_temp)
+				if (temp >= trip_temp)
 					cdev->ops->set_cur_state(cdev, 1);

drivers/thermal/thermal_sys.c void thermal_zone_device_update(struct t · line 999

Quoted 10 lines

drivers/thermal/thermal_sys.c +1 -1

 				else
 					cdev->ops->set_cur_state(cdev, 0);
 			}
 			break;
 		case THERMAL_TRIP_PASSIVE:
-			if (temp > trip_temp || tz->passive)
+			if (temp >= trip_temp || tz->passive)
 				thermal_zone_device_passive(tz, temp,
 							    trip_temp, count);
 			break;

Looks good. You probably need to resend it with a Signed-off-by: line, but feel free to add

Acked-by Matthew Garrett <[email protected]>

Signature · 1 line

Matthew Garrett | [email protected]