Re: [PATCH] thermal/core/power_allocator: avoid cdev->state can not be reset

[Di Shen]

Hi all,
any comments for this patch? Looking forward to your reply. Thank you.

-- 
di



On Thu, Mar 9, 2023 at 10:05 PM Di Shen <di.shen@xxxxxxxxxx> wrote:
>
> Commit 0952177f2a1f (thermal/core/power_allocator: Update once cooling devices when temp is low)
> add a update flag to update cooling device only once when temp is low.
> But when the switch_on_temp is set to be a higher value, the cooling device state
> may not be reset to max, because the last_temp is smaller than the switch_on_temp.
>
> For example:
> First:
> swicth_on_temp=70 control_temp=85;
>
> Then userspace change the trip_temp:
> swicth_on_temp=45 control_temp=55 cur_temp=54
>
> Then userspace reset the trip_temp:
> swicth_on_temp=70 control_temp=85 cur_temp=57 last_temp=54
>
> At this time, the cooling device state should be reset to be max.
> However, because cur_temp(57) < switch_on_temp(70)
> last_temp(54) < swicth_on_temp(70) --> update = false
> When update is false, the cooling device state can not be reset.
>
> So delete the update condition, so that the cooling device state
> could be reset.
>
> Fixes: 0952177f2a1f (thermal/core/power_allocator: Update once cooling devices when temp is low)
> Signed-off-by: Di Shen <di.shen@xxxxxxxxxx>
> ---
>  drivers/thermal/gov_power_allocator.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index 0eaf1527d3e3..153bf528b98c 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -560,7 +560,7 @@ static void reset_pid_controller(struct power_allocator_params *params)
>         params->prev_err = 0;
>  }
>
> -static void allow_maximum_power(struct thermal_zone_device *tz, bool update)
> +static void allow_maximum_power(struct thermal_zone_device *tz)
>  {
>         struct thermal_instance *instance;
>         struct power_allocator_params *params = tz->governor_data;
> @@ -582,8 +582,7 @@ static void allow_maximum_power(struct thermal_zone_device *tz, bool update)
>                  */
>                 cdev->ops->get_requested_power(cdev, &req_power);
>
> -               if (update)
> -                       __thermal_cdev_update(instance->cdev);
> +               __thermal_cdev_update(instance->cdev);
>
>                 mutex_unlock(&instance->cdev->lock);
>         }
> @@ -697,7 +696,6 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id)
>         struct power_allocator_params *params = tz->governor_data;
>         struct thermal_trip trip;
>         int ret;
> -       bool update;
>
>         lockdep_assert_held(&tz->lock);
>
> @@ -710,10 +708,9 @@ static int power_allocator_throttle(struct thermal_zone_device *tz, int trip_id)
>
>         ret = __thermal_zone_get_trip(tz, params->trip_switch_on, &trip);
>         if (!ret && (tz->temperature < trip.temperature)) {
> -               update = (tz->last_temperature >= trip.temperature);
>                 tz->passive = 0;
>                 reset_pid_controller(params);
> -               allow_maximum_power(tz, update);
> +               allow_maximum_power(tz);
>                 return 0;
>         }
>
> --
> 2.17.1
>