Fix thermal problems during suspend/bootup
Subsystems (2)
v3 cover Fix thermal problems during suspend/bootup
This patch set fixes two problems when system is trying to suspend and boot up:
1.After system is woken up from suspend, the thermal framework uses
the dirty 'cached' thermal variables before suspend, which might cause expected behavior.
2.If a cooling device is registered after the thermal zone's registration,
current thermal framework forgets to update the thermal_zone's status, which might bring expected behavior under special cases.
Chen Yu (3):
Thermal: initialize thermal zone device correctly
Thermal: handle thermal zone device properly during system sleep
Thermal: do thermal zone update after a cooling device registered
drivers/thermal/step_wise.c | 17 ++++++++-- drivers/thermal/thermal_core.c | 75 ++++++++++++++++++++++++++++++++++++++++-- drivers/thermal/thermal_core.h | 1 + include/linux/thermal.h | 5 +++ 4 files changed, 93 insertions(+), 5 deletions(-)
Signature · 1 line
1.8.4.2
PATCH v3 1/3 Thermal: initialize thermal zone device correctly
From: Zhang Rui <[email protected]>
After thermal zone device registered, as we have not read any temperature before, thus tz->temperature should not be 0, which actually means 0C, and thermal trend is not available. In this case, we need specially handling for the first thermal_zone_device_update().
Both thermal core framework and step_wise governor is enhanced to handle this. And since the step_wise governor is the only one that uses trends, so it's the only thermal governor that needs to be updated.
--- drivers/thermal/step_wise.c | 17 +++++++++++++++-- drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- drivers/thermal/thermal_core.h | 1 + include/linux/thermal.h | 3 +++ 4 files changed, 36 insertions(+), 4 deletions(-)
drivers/thermal/step_wise.c +15 -2
@@ -63,6 +63,19 @@ static unsigned long get_target_state(struct thermal_instance *instance, next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state); +if (!instance->initialized) { +if (throttle) { + next_target = (cur_state +1 ) >= instance->upper ? + instance->upper : + ((cur_state +1 ) < instance->lower ? + instance->lower : (cur_state +1 )); + }else { + next_target =THERMAL_NO_TARGET ; + } + +return next_target; + } +switch (trend) {case THERMAL_TREND_RAISING :if (throttle) {
@@ -149,7 +162,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)dev_dbg (&instance->cdev->device,"old_target=%d, target=%d\n" , old_target, (int )instance->target); -if (old_target == instance->target) +if (instance->initialized && old_target == instance->target)continue ;/* Activate a passive thermal instance */
@@ -161,7 +174,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) instance->target == THERMAL_NO_TARGET)update_passive_instance (tz, trip_type,-1 ); - + instance->initialized = true; instance->cdev->updated = false;/* cdev needs update */ }
drivers/thermal/thermal_core.c +17 -2
@@ -532,8 +532,22 @@ static void update_temperature(struct thermal_zone_device *tz)mutex_unlock (&tz->lock);trace_thermal_temperature (tz); -dev_dbg (&tz->device,"last_temperature=%d, current_temperature=%d\n" , - tz->last_temperature, tz->temperature); +if (tz->last_temperature ==THERMAL_TEMP_INVALID ) +dev_dbg (&tz->device,"last_temperature N/A, current_temperature=%d\n" , + tz->temperature); +else +dev_dbg (&tz->device,"last_temperature=%d, current_temperature=%d\n" , + tz->last_temperature, tz->temperature); +} + +static void thermal_zone_device_reset (struct thermal_zone_device *tz) +{ +struct thermal_instance *pos; + + tz->temperature =THERMAL_TEMP_INVALID ; + tz->passive =0 ; +list_for_each_entry (pos, &tz->thermal_instances, tz_node) + pos->initialized = false; }void thermal_zone_device_update (struct thermal_zone_device *tz)
@@ -1900,6 +1914,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,INIT_DELAYED_WORK (&(tz->poll_queue), thermal_zone_device_check); +thermal_zone_device_reset (tz);thermal_zone_device_update (tz);return tz;
drivers/thermal/thermal_core.h +1 -0
@@ -41,6 +41,7 @@ struct thermal_instance {struct thermal_zone_device *tz;struct thermal_cooling_device *cdev;int trip; +bool initialized;unsigned long upper;/* Highest cooling state for this trip point */ unsigned long lower;/* Lowest cooling state for this trip point */ unsigned long target;/* expected cooling state */
include/linux/thermal.h +3 -0
@@ -43,6 +43,9 @@/* Default weight of a bound cooling device */ #define THERMAL_WEIGHT_DEFAULT 0 +/* use value, which < 0K, to indicate an invalid/uninitialized temperature */ +#define THERMAL_TEMP_INVALID -274000 +/* Unit conversion macros */ #define KELVIN_TO_CELSIUS (t) (long)(((long)t-2732 >= 0) ? \ ((long)t-2732+5)/10 : ((long)t-2732-5)/10)
Signature · 1 line
1.8.4.2
Re: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctly
For some reason, I thought Rui had picked this already.
Anyways, here are a couple of comments:
Quoted 22 lines · Chen Yu
From: Zhang Rui <[email protected]>
After thermal zone device registered, as we have not read any temperature before, thus
tz->temperatureshould not be 0, which actually means 0C, and thermal trend is not available. In this case, we need specially handling for the firstthermal_zone_device_update().Both thermal core framework and
step_wisegovernor is enhanced to handle this. And since thestep_wisegovernor is the only one that uses trends, so it's the only thermal governor that needs to be updated.CC <[email protected]> #3.18+Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Tested-by Matthias <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>
Quoted 4 lines
--- drivers/thermal/step_wise.c | 17 +++++++++++++++-- drivers/thermal/thermal_core.c | 19 +++++++++++++++++-- drivers/thermal/thermal_core.h | 1 +
I would prefer if you could split this patch in two. One for thermal core another one for step wise.
Quoted 37 lines
include/linux/thermal.h | 3 +++ 4 files changed, 36 insertions(+), 4 deletions(-)drivers/thermal/step_wise.c +14
-1@@ -63,6 +63,19 @@ static unsigned long get_target_state(struct thermal_instance *instance, next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state); +if (!instance->initialized) { +if (throttle) { + next_target = (cur_state +1 ) >= instance->upper ? + instance->upper : + ((cur_state +1 ) < instance->lower ? + instance->lower : (cur_state +1 )); + }else { + next_target =THERMAL_NO_TARGET ; + } + +return next_target; + } +switch (trend) {case THERMAL_TREND_RAISING :if (throttle) {@@ -149,7 +162,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)dev_dbg (&instance->cdev->device,"old_target=%d, target=%d\n" , old_target, (int )instance->target); -if (old_target == instance->target) +if (instance->initialized && old_target == instance->target)continue ;/* Activate a passive thermal instance */
drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) · line 180
Quoted 8 lines
drivers/thermal/step_wise.c +1
-1@@ -161,7 +174,7 @@ static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) instance->target == THERMAL_NO_TARGET)update_passive_instance (tz, trip_type,-1 ); - + instance->initialized = true; instance->cdev->updated = false;/* cdev needs update */ }
Considering that I understood the problem and your proposal well, I would say these changes on step wise are the perfect case for setting up a step_wise.bind_to_tz(). bind_to_tz() is already designed as an opportunity for governor to check the thermal zone status at the time of binding. Remember that moving to bind_to_tz() covers not only registration time, but governor switching too (say, user chooses user_space, then step_wise).
The above code seams to be correct, but after reviewing the code of step_wise.throttle(), I would say it is already complicated and deserves simplification, when possible.
Quoted 66 lines
drivers/thermal/thermal_core.c +17
-2@@ -532,8 +532,22 @@ static void update_temperature(struct thermal_zone_device *tz)mutex_unlock (&tz->lock);trace_thermal_temperature (tz); -dev_dbg (&tz->device,"last_temperature=%d, current_temperature=%d\n" , - tz->last_temperature, tz->temperature); +if (tz->last_temperature ==THERMAL_TEMP_INVALID ) +dev_dbg (&tz->device,"last_temperature N/A, current_temperature=%d\n" , + tz->temperature); +else +dev_dbg (&tz->device,"last_temperature=%d, current_temperature=%d\n" , + tz->last_temperature, tz->temperature); +} + +static void thermal_zone_device_reset (struct thermal_zone_device *tz) +{ +struct thermal_instance *pos; + + tz->temperature =THERMAL_TEMP_INVALID ; + tz->passive =0 ; +list_for_each_entry (pos, &tz->thermal_instances, tz_node) + pos->initialized = false; }void thermal_zone_device_update (struct thermal_zone_device *tz)@@ -1900,6 +1914,7 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,INIT_DELAYED_WORK (&(tz->poll_queue), thermal_zone_device_check); +thermal_zone_device_reset (tz);thermal_zone_device_update (tz);return tz;drivers/thermal/thermal_core.h +1
-0@@ -41,6 +41,7 @@ struct thermal_instance {struct thermal_zone_device *tz;struct thermal_cooling_device *cdev;int trip; +bool initialized;unsigned long upper;/* Highest cooling state for this trip point */ unsigned long lower;/* Lowest cooling state for this trip point */ unsigned long target;/* expected cooling state */ include/linux/thermal.h +3
-0@@ -43,6 +43,9 @@/* Default weight of a bound cooling device */ #define THERMAL_WEIGHT_DEFAULT 0 +/* use value, which < 0K, to indicate an invalid/uninitialized temperature */ +#define THERMAL_TEMP_INVALID -274000 +/* Unit conversion macros */ #define KELVIN_TO_CELSIUS (t) (long)(((long)t-2732 >= 0) ? \ ((long)t-2732+5)/10 : ((long)t-2732-5)/10)Signature · 1 line
1.8.4.2
RE: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctly
Hi Eduardo,
Thanks for your review and sorry for that I missed your email.
Quoted 41 lines
-----Original Message-----
From: [email protected] [mailto:linux-pm- [email protected]] On Behalf Of Eduardo Valentin
Sent: Friday, January 01, 2016 2:44 AM
To: Chen, Yu C
Cc: Zhang, Rui; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctlyFor some reason, I thought Rui had picked this already.
Anyways, here are a couple of comments:
Quoted 22 lines · Chen Yu
From: Zhang Rui <[email protected]>
After thermal zone device registered, as we have not read any temperature before, thus
tz->temperatureshould not be 0, which actually means 0C, and thermal trend is not available. In this case, we need specially handling for the firstthermal_zone_device_update().Both thermal core framework and
step_wisegovernor is enhanced to handle this. And since thestep_wisegovernor is the only one that uses trends, so it's the only thermal governor that needs to be updated.CC <[email protected]> #3.18+Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Tested-by Matthias <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>Quoted 4 lines
--- drivers/thermal/step_wise.c | 17 +++++++++++++++-- drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
drivers/thermal/thermal_core.h| 1 +
Quoted 2 lines
I would prefer if you could split this patch in two. One for thermal core another one for step wise.
[Yu] It would be better if we can split this patch into two, for stable material. What do you think, Rui?
Quoted 46 lines
drivers/thermal/step_wise.c static unsigned long get_target_state(struct
Quoted 9 lines
include/linux/thermal.h | 3 +++ 4 files changed, 36 insertions(+), 4 deletions(-)drivers/thermal/step_wise.c +0
-0@@ -63,6 +63,19 @@ static unsigned long get_target_state(struct
thermal_instance*instance,drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct
Quoted 20 lines
drivers/thermal/step_wise.c +13
-0next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state); +if (!instance->initialized) { +if (throttle) { + next_target = (cur_state +1 ) >= instance->upper ? + instance->upper : + ((cur_state +1 ) < instance->lower ? + instance->lower : (cur_state +1 )); + }else { + next_target =THERMAL_NO_TARGET ; + } + +return next_target; + } +switch (trend) {case THERMAL_TREND_RAISING :if (throttle) {@@ -149,7 +162,7 @@ static void thermal_zone_trip_update(struct
thermal_zone_device*tz, int trip)drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct · line 162
Quoted 1 line
drivers/thermal/step_wise.c +0
-0dev_dbg(&instance->cdev->device, "old_target=%d,target=%d\n",
drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
Quoted 7 lines
drivers/thermal/step_wise.c +1
-1old_target, (int)instance->target); -if (old_target == instance->target) +if (instance->initialized && old_target == instance->target)continue ; /* Activate a passive thermal instance */ @@ -161,7 +174,7@@
Quoted 1 line
static void thermal_zone_trip_update(struct
thermal_zone_device*tz, inttrip)
Quoted 3 lines
drivers/thermal/step_wise.c +0
-0instance->target == THERMAL_NO_TARGET)update_passive_instance (tz, trip_type,-1 );
Quoted 12 lines
drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) · line 179
Quoted 5 lines
drivers/thermal/step_wise.c +1
-1- + instance->initialized = true; instance->cdev->updated = false;/* cdev needs update */ }Considering that I understood the problem and your proposal well, I would say these changes on step wise are the perfect case for setting up a
step_wise.bind_to_tz().bind_to_tz()is already designed as an opportunity for governor to check the thermal zone status at the time of binding. Remember that moving tobind_to_tz()covers not only registration time, but governor switching too (say, user choosesuser_space, thenstep_wise).
[Yu] The code change in step_wise. get_target_state is mainly for suspend/resume scenario, which is not involved with thermal zone/governor bindings IMO.
Quoted 78 lines
The above code seams to be correct, but after reviewing the code of
step_wise.throttle(), I would say it is already complicated and deserves simplification, when possible.drivers/thermal/thermal_core.c static void update_temperature(struct
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/thermal/thermal_core.cindex d9e525c..682bc1e 100644drivers/thermal/thermal_core.c +0
-0@@ -532,8 +532,22 @@ static void update_temperature(struct
thermal_zone_device*tz)drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 4 lines
drivers/thermal/thermal_core.c +0
-1mutex_unlock (&tz->lock);trace_thermal_temperature (tz); - dev_dbg(&tz->device, "last_temperature=%d,current_temperature=%d\n",
drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 3 lines
drivers/thermal/thermal_core.c +2
-1- tz->last_temperature, tz->temperature); +if (tz->last_temperature ==THERMAL_TEMP_INVALID ) + dev_dbg(&tz->device, "last_temperature N/A,current_temperature=%d\n",
drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 3 lines
drivers/thermal/thermal_core.c +3
-0+ tz->temperature); + else + dev_dbg(&tz->device, "last_temperature=%d,current_temperature=%d\n",
Quoted 16 lines
drivers/thermal/thermal_core.c +10
-1+ tz->last_temperature, tz->temperature); } + +static void thermal_zone_device_reset (struct thermal_zone_device *tz) +{ +struct thermal_instance *pos; + + tz->temperature =THERMAL_TEMP_INVALID ; + tz->passive =0 ; +list_for_each_entry (pos, &tz->thermal_instances, tz_node) + pos->initialized = false; } void thermal_zone_device_update(struct thermal_zone_device *tz) @@ -1900,6 +1914,7 @@ struct thermal_zone_device*thermal_zone_device_register(const char *type,
Quoted 1 line
INIT_DELAYED_WORK(&(tz->poll_queue),
thermal_zone_device_check);Quoted 25 lines
drivers/thermal/thermal_core.c +1
-0+thermal_zone_device_reset (tz);thermal_zone_device_update (tz);return tz;(quoted diff) +0
-0
b/drivers/thermal/thermal_core.hindex d7ac1fc..749d41a 100644drivers/thermal/thermal_core.h +1
-0@@ -41,6 +41,7 @@ struct thermal_instance {struct thermal_zone_device *tz;struct thermal_cooling_device *cdev;int trip; +bool initialized;unsigned long upper;/* Highest cooling state for this trip point */ unsigned long lower;/* Lowest cooling state for this trip point */ unsigned long target;/* expected cooling state */ include/linux/thermal.h index renamed from include/linux/thermal.h +0
-0157d366..5bcabc7 100644
include/linux/thermal.h +0
-0@@ -43,6 +43,9 @@ /* Default weight of a bound cooling device */ #define
THERMAL_WEIGHT_DEFAULT0Quoted 1 line
+/* use value, which < 0K, to indicate an invalid/uninitialized temperature
*/
Quoted 7 lines
include/linux/thermal.h +2
-1+#define THERMAL_TEMP_INVALID -274000 +/* Unit conversion macros */ #define KELVIN_TO_CELSIUS (t) (long)(((long)t-2732 >= 0) ? \ ((long)t-2732+5)/10 : ((long)t-2732-5)/10) --1.8.4.2
Zhang, Rui
reviewerRE: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctly
Quoted 57 lines
-----Original Message-----
From: Chen, Yu C
Sent: Tuesday, January 12, 2016 2:42 PM
To: Eduardo Valentin <[email protected]>; Zhang, Rui <[email protected]>
Cc: [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: RE: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctly
Importance: HighHi Eduardo,
Thanks for your review and sorry for that I missed your email.Quoted 42 lines
-----Original Message-----
From: [email protected] [mailto:linux-pm- [email protected]] On Behalf Of Eduardo Valentin
Sent: Friday, January 01, 2016 2:44 AM
To: Chen, Yu C
Cc: Zhang, Rui; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 1/3][v3] Thermal: initialize thermal zone device correctlyFor some reason, I thought Rui had picked this already.
Anyways, here are a couple of comments:
Quoted 26 lines · Chen Yu
From: Zhang Rui <[email protected]>
After thermal zone device registered, as we have not read any temperature before, thus
tz->temperatureshould not be 0, which actually means 0C, and thermal trend is not available. In this case, we need specially handling for the firstthermal_zone_device_update().Both thermal core framework and
step_wisegovernor is enhanced to handle this. And since thestep_wisegovernor is the only one that uses trends, so it's the only thermal governor that needs to be updated.CC <[email protected]> #3.18+Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Tested-by Matthias <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>--- drivers/thermal/step_wise.c | 17 +++++++++++++++-- drivers/thermal/thermal_core.c | 19 +++++++++++++++++--
drivers/thermal/thermal_core.h| 1 +
Quoted 4 lines
Quoted 2 lines
I would prefer if you could split this patch in two. One for thermal core another one for step wise.
[Yu] It would be better if we can split this patch into two, for stable material. What do you think, Rui?
[Zhang, Rui] No, first of all, we cannot take one patch for upstream and then split it into two for stable, second, I'd say I'm okay if the code is organized in two patches as Eduardo described, but at the same time, I don't think it's a problem if it's sent within one patch.
Quoted 57 lines
Quoted 47 lines
drivers/thermal/step_wise.c static unsigned long get_target_state(struct
Quoted 9 lines
include/linux/thermal.h | 3 +++ 4 files changed, 36 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/drivers/thermal/step_wise.cindex 2f9f708..ea9366a 100644drivers/thermal/step_wise.c +0
-0@@ -63,6 +63,19 @@ static unsigned long get_target_state(struct
thermal_instance*instance,drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct
Quoted 20 lines
drivers/thermal/step_wise.c +13
-0next_target = instance->target;dev_dbg (&cdev->device,"cur_state=%ld\n" , cur_state); +if (!instance->initialized) { +if (throttle) { + next_target = (cur_state +1 ) >= instance->upper ? + instance->upper : + ((cur_state +1 ) < instance->lower ? + instance->lower : (cur_state +1 )); + }else { + next_target =THERMAL_NO_TARGET ; + } + +return next_target; + } +switch (trend) {case THERMAL_TREND_RAISING :if (throttle) {@@ -149,7 +162,7 @@ static void thermal_zone_trip_update(struct
thermal_zone_device*tz, int trip)drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct · line 162
Quoted 1 line
drivers/thermal/step_wise.c +0
-0dev_dbg(&instance->cdev->device, "old_target=%d,target=%d\n",
drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip)
Quoted 7 lines
drivers/thermal/step_wise.c +1
-1old_target, (int)instance->target); -if (old_target == instance->target) +if (instance->initialized && old_target == instance->target)continue ; /* Activate a passive thermal instance */ @@ -161,7 +174,7@@
Quoted 2 lines
static void thermal_zone_trip_update(struct
thermal_zone_device*tz, inttrip)
Quoted 3 lines
drivers/thermal/step_wise.c +0
-0instance->target == THERMAL_NO_TARGET)update_passive_instance (tz, trip_type,-1 );Quoted 10 lines
drivers/thermal/step_wise.c static void thermal_zone_trip_update(struct thermal_zone_device *tz, int trip) · line 179
Quoted 5 lines
drivers/thermal/step_wise.c +1
-1- + instance->initialized = true; instance->cdev->updated = false;/* cdev needs update */ }Considering that I understood the problem and your proposal well, I would say these changes on step wise are the perfect case for setting up a
step_wise.bind_to_tz().bind_to_tz()is already designed as an opportunity for governor to check the thermal zone status at the time of
Quoted 8 lines
binding.
Quoted 2 lines
Remember that moving to
bind_to_tz()covers not only registration time, but governor switching too (say, user choosesuser_space, then
step_wise).[Yu] The code change in
step_wise.get_target_stateis mainly for suspend/resume scenario, which is not involved with thermal zone/governor bindings IMO.
Agreed.
Thanks,
rui
Quoted 80 lines
Quoted 38 lines
The above code seams to be correct, but after reviewing the code of
step_wise.throttle(), I would say it is already complicated and deserves simplification, when possible.drivers/thermal/thermal_core.c static void update_temperature(struct
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/thermal/thermal_core.cindex d9e525c..682bc1e 100644drivers/thermal/thermal_core.c +0
-0@@ -532,8 +532,22 @@ static void update_temperature(struct
thermal_zone_device*tz)drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 4 lines
drivers/thermal/thermal_core.c +0
-1mutex_unlock (&tz->lock);trace_thermal_temperature (tz); - dev_dbg(&tz->device, "last_temperature=%d,current_temperature=%d\n",
drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 3 lines
drivers/thermal/thermal_core.c +2
-1- tz->last_temperature, tz->temperature); +if (tz->last_temperature ==THERMAL_TEMP_INVALID ) + dev_dbg(&tz->device, "last_temperature N/A,current_temperature=%d\n",
drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz)
Quoted 3 lines
drivers/thermal/thermal_core.c +3
-0+ tz->temperature); + else + dev_dbg(&tz->device, "last_temperature=%d,current_temperature=%d\n",
Quoted 12 lines
drivers/thermal/thermal_core.c +10
-0+ tz->last_temperature, tz->temperature); } + +static void thermal_zone_device_reset (struct thermal_zone_device +*tz) { +struct thermal_instance *pos; + + tz->temperature =THERMAL_TEMP_INVALID ; + tz->passive =0 ; +list_for_each_entry (pos, &tz->thermal_instances, tz_node) + pos->initialized = false; }Quoted 1 line
drivers/thermal/thermal_core.c static void update_temperature(struct thermal_zone_device *tz) · line 553
Quoted 1 line
drivers/thermal/thermal_core.c +0
-0void thermal_zone_device_update (struct thermal_zone_device *tz)@@
Quoted 40 lines
Quoted 4 lines
-1900,6 +1914,7 @@ struct
thermal_zone_device
*thermal_zone_device_register(const char *type,INIT_DELAYED_WORK(&(tz->poll_queue),
thermal_zone_device_check);Quoted 25 lines
drivers/thermal/thermal_core.c +1
-0+thermal_zone_device_reset (tz);thermal_zone_device_update (tz);return tz;(quoted diff) +0
-0
b/drivers/thermal/thermal_core.hindex d7ac1fc..749d41a 100644drivers/thermal/thermal_core.h +1
-0@@ -41,6 +41,7 @@ struct thermal_instance {struct thermal_zone_device *tz;struct thermal_cooling_device *cdev;int trip; +bool initialized;unsigned long upper;/* Highest cooling state for this trip point */ unsigned long lower;/* Lowest cooling state for this trip point */ unsigned long target;/* expected cooling state */ include/linux/thermal.h index renamed from include/linux/thermal.h +0
-0157d366..5bcabc7 100644
include/linux/thermal.h +0
-0@@ -43,6 +43,9 @@ /* Default weight of a bound cooling device */ #define
THERMAL_WEIGHT_DEFAULT0include/linux/thermal.h
Quoted 2 lines
include/linux/thermal.h +2
-0+/* use value, which < 0K, to indicate an invalid/uninitialized +temperature */
Quoted 7 lines
include/linux/thermal.h +2
-1+#define THERMAL_TEMP_INVALID -274000 +/* Unit conversion macros */ #define KELVIN_TO_CELSIUS (t) (long)(((long)t-2732 >= 0) ? \ ((long)t-2732+5)/10 : ((long)t-2732-5)/10) --1.8.4.2
PATCH v3 2/3 Thermal: handle thermal zone device properly during system sleep
In reply to Chen YuFrom: Zhang Rui <[email protected]>
Current thermal code does not handle system sleep well because
1. the cooling device cooling state may be changed during suspend
2. the previous temperature reading becomes invalid after resumed because it is got before system sleep
3. updating thermal zone device during suspending/resuming
is wrong because some devices may have already been suspended or may have not been resumed.
Thus, the proper way to do this is to cancel all thermal zone device update requirements during suspend/resume, and after all the devices have been resumed, reset and update every registered thermal zone devices.
This also fixes a regression introduced by:
Commit 19593a1fb1f6 ("ACPI / fan: convert to platform driver") Because, with above commit applied, all the fan devices are attached to the acpi_general_pm_domain, and they are turned on by the pm_domain automatically after resume, without the awareness of thermal core.
CC: <[email protected]> #3.18+
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=78201
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=91411
--- drivers/thermal/thermal_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
drivers/thermal/thermal_core.c +42 -0
@@ -37,6 +37,7 @@#include <linux/of.h> #include <net/netlink.h> #include <net/genetlink.h> +#include <linux/suspend.h> #define CREATE_TRACE_POINTS #include <trace/events/thermal.h>
@@ -59,6 +60,8 @@ static LIST_HEAD(thermal_governor_list);static DEFINE_MUTEX (thermal_list_lock);static DEFINE_MUTEX (thermal_governor_lock); +static atomic_t in_suspend; +static struct thermal_governor *def_governor;static struct thermal_governor *__find_governor (const char *name)
@@ -554,6 +557,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) {int count; +if (atomic_read (&in_suspend)) +return ; +if (!tz->ops->get_temp)return ;
@@ -2155,6 +2161,36 @@ static void thermal_unregister_governors(void)thermal_gov_power_allocator_unregister (); } +static int thermal_pm_notify (struct notifier_block *nb, +unsigned long mode,void *_unused) +{ +struct thermal_zone_device *tz; + +switch (mode) { +case PM_HIBERNATION_PREPARE : +case PM_RESTORE_PREPARE : +case PM_SUSPEND_PREPARE : +atomic_set (&in_suspend,1 ); +break ; +case PM_POST_HIBERNATION : +case PM_POST_RESTORE : +case PM_POST_SUSPEND : +atomic_set (&in_suspend,0 ); +list_for_each_entry (tz, &thermal_tz_list, node) { +thermal_zone_device_reset (tz); +thermal_zone_device_update (tz); + } +break ; +default : +break ; + } +return 0 ; +} + +static struct notifier_block thermal_pm_nb = { + .notifier_call = thermal_pm_notify, +}; +static int __init thermal_init (void ) {int result;
@@ -2175,6 +2211,11 @@ static int __init thermal_init(void)if (result) goto exit_netlink; + result =register_pm_notifier (&thermal_pm_nb); +if (result) +pr_warn ("Thermal: Can not register suspend notifier, return %d\n" , + result); +return 0 ; exit_netlink:
@@ -2194,6 +2235,7 @@ error: static void __exit thermal_exit(void) { +unregister_pm_notifier (&thermal_pm_nb);of_thermal_destroy_zones ();genetlink_exit ();class_unregister (&thermal_class);
Signature · 1 line
1.8.4.2
Re: [PATCH 2/3][v3] Thermal: handle thermal zone device properly during system sleep
Quoted 24 lines · Chen Yu
From: Zhang Rui <[email protected]>
Current thermal code does not handle system sleep well because
1. the cooling device cooling state may be changed during suspend
2. the previous temperature reading becomes invalid after resumed because it is got before system sleep
3. updating thermal zone device during suspending/resuming
is wrong because some devices may have already been suspended or may have not been resumed.Thus, the proper way to do this is to cancel all thermal zone device update requirements during suspend/resume, and after all the devices have been resumed, reset and update every registered thermal zone devices.
This also fixes a regression introduced by:
Commit19593a1fb1f6("ACPI / fan: convert to platform driver") Because, with above commit applied, all the fan devices are attached to theacpi_general_pm_domain, and they are turned on by thepm_domainautomatically after resume, without the awareness of thermal core.CC: <[email protected]> #3.18+
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=78201
Reference: https://bugzilla.kernel.org/show_bug.cgi?id=91411
Quoted 8 lines
Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Tested-by Matthias <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>
Quoted 95 lines
--- drivers/thermal/thermal_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)drivers/thermal/thermal_core.c +42
-0@@ -37,6 +37,7 @@#include <linux/of.h> #include <net/netlink.h> #include <net/genetlink.h> +#include <linux/suspend.h> #define CREATE_TRACE_POINTS #include <trace/events/thermal.h> @@ -59,6 +60,8 @@ static LIST_HEAD(thermal_governor_list);static DEFINE_MUTEX (thermal_list_lock);static DEFINE_MUTEX (thermal_governor_lock); +static atomic_t in_suspend; +static struct thermal_governor *def_governor;static struct thermal_governor *__find_governor (const char *name)@@ -554,6 +557,9 @@ void thermal_zone_device_update(struct thermal_zone_device *tz) {int count; +if (atomic_read (&in_suspend)) +return ; +if (!tz->ops->get_temp)return ;@@ -2155,6 +2161,36 @@ static void thermal_unregister_governors(void)thermal_gov_power_allocator_unregister (); } +static int thermal_pm_notify (struct notifier_block *nb, +unsigned long mode,void *_unused) +{ +struct thermal_zone_device *tz; + +switch (mode) { +case PM_HIBERNATION_PREPARE : +case PM_RESTORE_PREPARE : +case PM_SUSPEND_PREPARE : +atomic_set (&in_suspend,1 ); +break ; +case PM_POST_HIBERNATION : +case PM_POST_RESTORE : +case PM_POST_SUSPEND : +atomic_set (&in_suspend,0 ); +list_for_each_entry (tz, &thermal_tz_list, node) { +thermal_zone_device_reset (tz); +thermal_zone_device_update (tz); + } +break ; +default : +break ; + } +return 0 ; +} + +static struct notifier_block thermal_pm_nb = { + .notifier_call = thermal_pm_notify, +}; +static int __init thermal_init (void ) {int result;@@ -2175,6 +2211,11 @@ static int __init thermal_init(void)if (result) goto exit_netlink; + result =register_pm_notifier (&thermal_pm_nb); +if (result) +pr_warn ("Thermal: Can not register suspend notifier, return %d\n" , + result); +return 0 ; exit_netlink:@@ -2194,6 +2235,7 @@ error: static void __exit thermal_exit(void) { +unregister_pm_notifier (&thermal_pm_nb);of_thermal_destroy_zones ();genetlink_exit ();class_unregister (&thermal_class);Signature · 1 line
1.8.4.2
PATCH v3 3/3 Thermal: do thermal zone update after a cooling device registered
In reply to Chen YuWhen a new cooling device is registered, we need to update the thermal zone to set the new registered cooling device to a proper state.
This fixes a problem that the system is cool, while the fan devices are left running on full speed after boot, if fan device is registered after thermal zone device.
Here is the history of why current patch looks like this:
https://patchwork.kernel.org/patch/7273041/
CC: <[email protected]> #3.18+ Reference:https://bugzilla.kernel.org/show_bug.cgi?id=92431
--- drivers/thermal/thermal_core.c | 14 +++++++++++++- include/linux/thermal.h | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-)
drivers/thermal/thermal_core.c +13 -1
@@ -1341,6 +1341,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,if (!result) {list_add_tail (&dev->tz_node, &tz->thermal_instances);list_add_tail (&dev->cdev_node, &cdev->thermal_instances); +atomic_set (&tz->need_update,1 ); }mutex_unlock (&cdev->lock);mutex_unlock (&tz->lock);
@@ -1450,6 +1451,7 @@ __thermal_cooling_device_register(struct device_node *np, const struct thermal_cooling_device_ops *ops) {struct thermal_cooling_device *cdev; +struct thermal_zone_device *pos =NULL ;int result;if (type &&strlen (type) >=THERMAL_NAME_LENGTH )
@@ -1494,6 +1496,12 @@ __thermal_cooling_device_register(struct device_node *np,/* Update binding information for 'this' new cdev */ bind_cdev (cdev); +mutex_lock (&thermal_list_lock); +list_for_each_entry (pos, &thermal_tz_list, node) +if (atomic_cmpxchg (&pos->need_update,1 ,0 )) +thermal_zone_device_update (pos); +mutex_unlock (&thermal_list_lock); +return cdev; }
@@ -1826,6 +1834,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, tz->trips = trips; tz->passive_delay = passive_delay; tz->polling_delay = polling_delay; +/* A new thermal zone needs to be updated anyway. */ +atomic_set (&tz->need_update,1 );dev_set_name (&tz->device,"thermal_zone%d" , tz->id); result =device_register (&tz->device);
@@ -1921,7 +1931,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,INIT_DELAYED_WORK (&(tz->poll_queue), thermal_zone_device_check);thermal_zone_device_reset (tz); -thermal_zone_device_update (tz); +/* Update the new thermal zone and mark it as already updated. */ +if (atomic_cmpxchg (&tz->need_update,1 ,0 )) +thermal_zone_device_update (tz);return tz;
include/linux/thermal.h +2 -0
@@ -168,6 +168,7 @@ struct thermal_attr {* @forced_passive: If > 0, temperature at which to switch on all ACPI * processor cooling devices. Currently only used by the * step-wise governor. +* @need_update: if equals 1, thermal_zone_device_update needs to be invoked. * @ops: operations this &thermal_zone_device supports * @tzp: thermal zone parameters * @governor: pointer to the governor for this thermal zone
@@ -195,6 +196,7 @@ struct thermal_zone_device {int emul_temperature;int passive;unsigned int forced_passive; +atomic_t need_update;struct thermal_zone_device_ops *ops;struct thermal_zone_params *tzp;struct thermal_governor *governor;
Signature · 1 line
1.8.4.2
Re: [PATCH 3/3][v3] Thermal: do thermal zone update after a cooling device registered
Hello Chen,
Quoted 91 lines · Chen Yu
When a new cooling device is registered, we need to update the thermal zone to set the new registered cooling device to a proper state.
This fixes a problem that the system is cool, while the fan devices are left running on full speed after boot, if fan device is registered after thermal zone device.
Here is the history of why current patch looks like this:
https://patchwork.kernel.org/patch/7273041/CC: <[email protected]> #3.18+ Reference:https://bugzilla.kernel.org/show_bug.cgi?id=92431
Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>--- drivers/thermal/thermal_core.c | 14 +++++++++++++- include/linux/thermal.h | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-)drivers/thermal/thermal_core.c +13
-1@@ -1341,6 +1341,7 @@ int thermal_zone_bind_cooling_device(struct thermal_zone_device *tz,if (!result) {list_add_tail (&dev->tz_node, &tz->thermal_instances);list_add_tail (&dev->cdev_node, &cdev->thermal_instances); +atomic_set (&tz->need_update,1 ); }mutex_unlock (&cdev->lock);mutex_unlock (&tz->lock);@@ -1450,6 +1451,7 @@ __thermal_cooling_device_register(struct device_node *np, const struct thermal_cooling_device_ops *ops) {struct thermal_cooling_device *cdev; +struct thermal_zone_device *pos =NULL ;int result;if (type &&strlen (type) >=THERMAL_NAME_LENGTH )@@ -1494,6 +1496,12 @@ __thermal_cooling_device_register(struct device_node *np,/* Update binding information for 'this' new cdev */ bind_cdev (cdev); +mutex_lock (&thermal_list_lock); +list_for_each_entry (pos, &thermal_tz_list, node) +if (atomic_cmpxchg (&pos->need_update,1 ,0 )) +thermal_zone_device_update (pos); +mutex_unlock (&thermal_list_lock); +return cdev; }@@ -1826,6 +1834,8 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type, tz->trips = trips; tz->passive_delay = passive_delay; tz->polling_delay = polling_delay; +/* A new thermal zone needs to be updated anyway. */ +atomic_set (&tz->need_update,1 );dev_set_name (&tz->device,"thermal_zone%d" , tz->id); result =device_register (&tz->device);@@ -1921,7 +1931,9 @@ struct thermal_zone_device *thermal_zone_device_register(const char *type,INIT_DELAYED_WORK (&(tz->poll_queue), thermal_zone_device_check);thermal_zone_device_reset (tz); -thermal_zone_device_update (tz); +/* Update the new thermal zone and mark it as already updated. */ +if (atomic_cmpxchg (&tz->need_update,1 ,0 )) +thermal_zone_device_update (tz);return tz;include/linux/thermal.h +1
-0@@ -168,6 +168,7 @@ struct thermal_attr {* @forced_passive: If > 0, temperature at which to switch on all ACPI * processor cooling devices. Currently only used by the * step-wise governor. +* @need_update: if equals 1, thermal_zone_device_update needs to be invoked. * @ops: operations this &thermal_zone_device supports * @tzp: thermal zone parameters * @governor: pointer to the governor for this thermal zone
include/linux/thermal.h struct thermal_zone_device { · line 199
Quoted 4 lines
include/linux/thermal.h +1
-0@@ -195,6 +196,7 @@ struct thermal_zone_device {int emul_temperature;int passive;unsigned int forced_passive; +atomic_t need_update;
The only problem I have with the above change is the fact that it does not touch thermal_zone_device_update() in any place. Please, remember that thermal_zone_device_update() is an exported function. That means that anyone can actually call it. And that is what happens today. If you git grep for it you will see that there are occurrences inside thermal core and inside thermal drivers. And this change do not take care of them. Are you sure you don't need to revisit all occurrences?
Shouldn't thermal_zone_device_update() unset the need_update bit, given it has just updated the zone?
Also, what happens to the logic when external changes happens? Say, we have writable trip points enabled, and user changes a trip point value?
BR,
Quoted 6 lines
include/linux/thermal.h +0
-0struct thermal_zone_device_ops *ops;struct thermal_zone_params *tzp;struct thermal_governor *governor;Signature · 1 line
1.8.4.2
RE: [PATCH 3/3][v3] Thermal: do thermal zone update after a cooling device registered
Hi Eduardo,
thanks for your review,
Quoted 105 lines
-----Original Message-----
From: Eduardo Valentin [mailto:[email protected]]
Sent: Friday, January 01, 2016 3:04 AM
To: Chen, Yu C
Cc: Zhang, Rui; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH 3/3][v3] Thermal: do thermal zone update after a cooling device registeredHello Chen,
drivers/thermal/thermal_core.c int thermal_zone_bind_cooling_device(struct
Quoted 29 lines · Chen Yu
When a new cooling device is registered, we need to update the thermal zone to set the new registered cooling device to a proper state.
This fixes a problem that the system is cool, while the fan devices are left running on full speed after boot, if fan device is registered after thermal zone device.
Here is the history of why current patch looks like this:
https://patchwork.kernel.org/patch/7273041/CC: <[email protected]> #3.18+ Reference:https://bugzilla.kernel.org/show_bug.cgi?id=92431
Tested-by Manuel Krause <[email protected]>Tested-by szegad <[email protected]>Tested-by prash <[email protected]>Tested-by amish <[email protected]>Reviewed-by Javi Merino <[email protected]>Signed-off-by Zhang Rui <[email protected]>Signed-off-by Chen Yu <[email protected]>--- drivers/thermal/thermal_core.c | 14 +++++++++++++- include/linux/thermal.h | 2 ++ 2 files changed, 15 insertions(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/thermal/thermal_core.cindex 9aae767..ba08b55 100644drivers/thermal/thermal_core.c +0
-0@@ -1341,6 +1341,7 @@ int thermal_zone_bind_cooling_device(struct
thermal_zone_device*tz,drivers/thermal/thermal_core.c __thermal_cooling_device_register(struct
Quoted 8 lines
drivers/thermal/thermal_core.c +1
-0if (!result) {list_add_tail (&dev->tz_node, &tz->thermal_instances);list_add_tail (&dev->cdev_node, &cdev->thermal_instances); +atomic_set (&tz->need_update,1 ); }mutex_unlock (&cdev->lock);mutex_unlock (&tz->lock);@@ -1450,6 +1451,7 @@ __thermal_cooling_device_register(struct
device_node*np,drivers/thermal/thermal_core.c __thermal_cooling_device_register(struct · line 1451
Quoted 1 line
drivers/thermal/thermal_core.c +0
-0const struct thermal_cooling_device_ops*ops) {
drivers/thermal/thermal_core.c __thermal_cooling_device_register(struct device_node *np,
Quoted 5 lines
drivers/thermal/thermal_core.c +1
-0struct thermal_cooling_device *cdev; +struct thermal_zone_device *pos =NULL ;int result; if (type && strlen(type) >= THERMAL_NAME_LENGTH) @@ -1494,6+1496,12
drivers/thermal/thermal_core.c struct thermal_zone_device
Quoted 14 lines
drivers/thermal/thermal_core.c +0
-0@@ __thermal_cooling_device_register(struct device_node *np,drivers/thermal/thermal_core.c +6
-0/* Update binding information for 'this' new cdev */ bind_cdev (cdev); +mutex_lock (&thermal_list_lock); +list_for_each_entry (pos, &thermal_tz_list, node) +if (atomic_cmpxchg (&pos->need_update,1 ,0 )) +thermal_zone_device_update (pos); +mutex_unlock (&thermal_list_lock); +return cdev; }@@ -1826,6 +1834,8 @@ struct thermal_zone_device*thermal_zone_device_register(const char *type,
drivers/thermal/thermal_core.c struct thermal_zone_device *thermal_zone_device_register(const char *type,
Quoted 8 lines
drivers/thermal/thermal_core.c +2
-0tz->trips = trips; tz->passive_delay = passive_delay; tz->polling_delay = polling_delay; +/* A new thermal zone needs to be updated anyway. */ +atomic_set (&tz->need_update,1 );dev_set_name (&tz->device,"thermal_zone%d" , tz->id); result = device_register(&tz->device); @@ -1921,7 +1931,9 @@struct
Quoted 2 lines
thermal_zone_device*thermal_zone_device_register(const char *type,
INIT_DELAYED_WORK(&(tz->poll_queue),
thermal_zone_device_check);Quoted 14 lines
drivers/thermal/thermal_core.c +3
-1thermal_zone_device_reset (tz); -thermal_zone_device_update (tz); +/* Update the new thermal zone and mark it as already updated. */ +if (atomic_cmpxchg (&tz->need_update,1 ,0 )) +thermal_zone_device_update (tz);return tz;include/linux/thermal.h index renamed from include/linux/thermal.h +0
-05bcabc7..385d411 100644
include/linux/thermal.h struct thermal_attr { · line 171
Quoted 4 lines
include/linux/thermal.h +1
-0@@ -168,6 +168,7 @@ struct thermal_attr {* @forced_passive: If > 0, temperature at which to switch on all ACPI * processor cooling devices. Currently only used by the * step-wise governor. +* @need_update: if equals 1, thermal_zone_device_update needs to be invoked.
Quoted 16 lines
include/linux/thermal.h struct thermal_zone_device { · line 199
Quoted 8 lines
include/linux/thermal.h +1
-0* @ops: operations this &thermal_zone_device supports * @tzp: thermal zone parameters * @governor: pointer to the governor for this thermal zone @@ -195,6 +196,7 @@ struct thermal_zone_device {int emul_temperature;int passive;unsigned int forced_passive; +atomic_t need_update;The only problem I have with the above change is the fact that it does not touch
thermal_zone_device_update()in any place. Please, remember thatthermal_zone_device_update()is an exported function. That means that anyone can actually call it. And that is what happens today. If you git grep for it you will see that there are occurrences inside thermal core and inside thermal drivers. And this change do not take care of them. Are you sure you don't need to revisit all occurrences?
[Yu] Do you mean this patch should be aware of other places who have already 'updated' the thermal zone for the new device? so we don't need to update them for the second time? Yes, the current implementation seems to be redundant when others have updated the zone for us, but it is to make sure the updating for this thermal zone will be invoked at least once, if a new cooling device is added into it.
Quoted 3 lines
Shouldn't
thermal_zone_device_update()unset theneed_updatebit, given it has just updated the zone?
[Yu] If thermal_zone_device_update takes care of this flag, we might need to makethermal_zone_device_update a atomic function first. Maybe the name of need_update should be renamed to new_device_added.
Quoted 3 lines
Also, what happens to the logic when external changes happens? Say, we have writable trip points enabled, and user changes a trip point value?
[Yu] If the user changes a trip point, since currently thermal_zone_device_update is not atomic, this might bring problems when thermal_zone_device_update is executing parallelly,
and we might need to make thermal_zone_device_update into a atomic operation in the future.
Quoted 10 lines
BR,
Quoted 6 lines
include/linux/thermal.h +0
-1struct thermal_zone_device_ops *ops;struct thermal_zone_params *tzp;struct thermal_governor *governor; --1.8.4.2
thanks and happy new year,
Yu