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.
Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
---
drivers/thermal/thermal_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 21db445..2b7a0b4 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -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;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Gentle reminder.
On 2021-11-03 01:30, Manaf Meethalavalappu Pallikunhi wrote:
> 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.
>
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c
> index 21db445..2b7a0b4 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -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;
> }
On Tue, Nov 2, 2021 at 9:01 PM Manaf Meethalavalappu Pallikunhi
<[email protected]> wrote:
>
> 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.
Makes sense to me.
Daniel?
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
> ---
> drivers/thermal/thermal_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 21db445..2b7a0b4 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -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?
> + tz->prev_high_trip = INT_MAX;
> list_for_each_entry(pos, &tz->thermal_instances, tz_node)
> pos->initialized = false;
> }
> --
On 2021-11-05 20:52, Rafael J. Wysocki wrote:
> On Tue, Nov 2, 2021 at 9:01 PM Manaf Meethalavalappu Pallikunhi
> <[email protected]> wrote:
>>
>> 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.
>
> Makes sense to me.
>
> Daniel?
>
>> Signed-off-by: Manaf Meethalavalappu Pallikunhi
>> <[email protected]>
>> ---
>> drivers/thermal/thermal_core.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c
>> index 21db445..2b7a0b4 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -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?
>
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.
>> + tz->prev_high_trip = INT_MAX;
>> list_for_each_entry(pos, &tz->thermal_instances, tz_node)
>> pos->initialized = false;
>> }
>> --
On 11/2/21 4:00 PM, Manaf Meethalavalappu Pallikunhi wrote:
> 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.
>
> Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
Reviewed-by: Thara Gopinath <[email protected]>
--
Warm Regards
Thara (She/Her/Hers)
> ---
> drivers/thermal/thermal_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 21db445..2b7a0b4 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -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;
> }
>
On Fri, Nov 5, 2021 at 7:49 PM Thara Gopinath <[email protected]> wrote:
>
>
>
> On 11/2/21 4:00 PM, Manaf Meethalavalappu Pallikunhi wrote:
> > 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.
> >
> > Signed-off-by: Manaf Meethalavalappu Pallikunhi <[email protected]>
>
> Reviewed-by: Thara Gopinath <[email protected]>
>
> --
> Warm Regards
> Thara (She/Her/Hers)
>
> > ---
> > drivers/thermal/thermal_core.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> > index 21db445..2b7a0b4 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -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!