drivers: thermal: Reset previous low and high trip during thermal zone init
PATCH 1/1 drivers: thermal: Reset previous low and high trip during thermal zone init
During the suspend is in process, thermal_zone_device_update bails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.
During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/thermal/thermal_core.c +2 -0
@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ; + tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false; }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Gentle reminder.
Quoted 35 lines · Manaf Meethalavalappu Pallikunhi
During the suspend is in process,
thermal_zone_device_updatebails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
Signed-off-by Manaf Meethalavalappu Pallikunhi <[email protected]>--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/thermal/thermal_core.c +2
-0@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ; + tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false;}
Rafael J. Wysocki
maintainerQuoted 14 lines · Manaf Meethalavalappu Pallikunhi
During the suspend is in process,
thermal_zone_device_updatebails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.
Quoted 2 lines
During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
Makes sense to me.
Daniel?
Quoted 10 lines
Signed-off-by Manaf Meethalavalappu Pallikunhi <[email protected]>--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/thermal/thermal_core.c +0
-0
drivers/thermal/thermal_core.c static void thermal_zone_device_init(struct thermal_zone_device *tz) · line 480
Quoted 4 lines
drivers/thermal/thermal_core.c +1
-0@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ;
Why not use INT_MIN instead?
Quoted 5 lines
drivers/thermal/thermal_core.c +1
-1+ tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false; } --
Quoted 28 lines · Rafael J. Wysocki
Quoted 14 lines · Manaf Meethalavalappu Pallikunhi
During the suspend is in process,
thermal_zone_device_updatebails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.Quoted 2 lines
During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
Makes sense to me.
Daniel?
Quoted 5 lines
Signed-off-by Manaf Meethalavalappu Pallikunhi <[email protected]>--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)
Quoted 13 lines
drivers/thermal/thermal_core.c static void thermal_zone_device_init(struct · line 481
Quoted 10 lines
drivers/thermal/thermal_core.c +1
-0@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ;Why not use
INT_MINinstead?
The thermal_zone_set_trips API uses -INT_MAX as default low trip to start trip aggregation. I used the same default values here as well.
Quoted 5 lines
Quoted 5 lines
drivers/thermal/thermal_core.c +1
-0+ tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false;}
--
Quoted 13 lines · Manaf Meethalavalappu Pallikunhi
During the suspend is in process,
thermal_zone_device_updatebails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.
Quoted 4 lines
During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
Signed-off-by Manaf Meethalavalappu Pallikunhi <[email protected]>
Signature · 2 lines
Warm Regards Thara (She/Her/Hers)
Quoted 18 lines
--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)drivers/thermal/thermal_core.c +2
-0@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ; + tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false; }
Quoted 32 lines · Thara Gopinath
Quoted 13 lines · Manaf Meethalavalappu Pallikunhi
During the suspend is in process,
thermal_zone_device_updatebails out thermal zone re-evaluation for any sensor trip violation without setting next valid trip to that sensor. It assumes during resume it will re-evaluate same thermal zone and update trip. But when it is in suspend temperature goes down and on resume path while updating thermal zone if temperature is less than previously violated trip, thermal zone set trip function evaluates the same previous high and previous low trip as new high and low trip. Since there is no change in high/low trip, it bails out from thermal zone set trip API without setting any trip. It leads to a case where sensor high trip or low trip is disabled forever even though thermal zone has a valid high or low trip.Quoted 4 lines
During thermal zone device init, reset thermal zone previous high and low trip. It resolves above mentioned scenario.
Signed-off-by Manaf Meethalavalappu Pallikunhi <[email protected]>Reviewed-by Thara Gopinath <[email protected]>--
Warm Regards
Thara (She/Her/Hers)Quoted 4 lines
--- drivers/thermal/thermal_core.c | 2 ++ 1 file changed, 2 insertions(+)
Quoted 13 lines
drivers/thermal/thermal_core.c static void thermal_zone_device_init(struct thermal_zone_device *tz) · line 484
Quoted 13 lines
drivers/thermal/thermal_core.c +2
-0@@ -477,6 +477,8 @@ static void thermal_zone_device_init(struct thermal_zone_device *tz) {struct thermal_instance *pos; tz->temperature =THERMAL_TEMP_INVALID ; + tz->prev_low_trip = -INT_MAX ; + tz->prev_high_trip =INT_MAX ;list_for_each_entry (pos, &tz->thermal_instances, tz_node) pos->initialized = false; }
Applied as 5.16-rc2 material, thanks!