Re: [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding without trip points

[Lukasz Luba]

On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@xxxxxxx>
>
> IPA probe function was recently refactored to perform extra error checks
> and make sure the thermal zone has trip points necessary for the IPA
> operation. With this change, if a thermal zone is probed such that it
> has no trip points that IPA can use, IPA will fail and the TZ won't be
> created. This is the case if a platform defines a TZ without cooling
> devices and only with "hot"/"critical" trip points, often found on some
> Qualcomm devices [1].
>
> Documentation across IPA code (notably get_governor_trips() kerneldoc)
> suggests that IPA is supposed to handle such TZ even if it won't
> actually do anything.
>
> This commit partially reverts the previous change to allow IPA to bind
> to such "empty" thermal zones.
>
> [1] arch/arm64/boot/dts/qcom/sc7180.dtsi#n4776
>
> Fixes: e83747c2f8e3 ("thermal: gov_power_allocator: Set up trip points earlier")
> Signed-off-by: Nikita Travkin <nikita@xxxxxxx>
> ---
>   drivers/thermal/gov_power_allocator.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c
> index ec637071ef1f..e25e48d76aa7 100644
> --- a/drivers/thermal/gov_power_allocator.c
> +++ b/drivers/thermal/gov_power_allocator.c
> @@ -679,11 +679,6 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>   		return -ENOMEM;
>   
>   	get_governor_trips(tz, params);
> -	if (!params->trip_max) {
> -		dev_warn(&tz->device, "power_allocator: missing trip_max\n");
> -		kfree(params);
> -		return -EINVAL;
> -	}
>   
>   	ret = check_power_actors(tz, params);
>   	if (ret < 0) {
> @@ -714,9 +709,10 @@ static int power_allocator_bind(struct thermal_zone_device *tz)
>   	else
>   		params->sustainable_power = tz->tzp->sustainable_power;
>   
> -	estimate_pid_constants(tz, tz->tzp->sustainable_power,
> -			       params->trip_switch_on,
> -			       params->trip_max->temperature);
> +	if (params->trip_max)
> +		estimate_pid_constants(tz, tz->tzp->sustainable_power,
> +				       params->trip_switch_on,
> +				       params->trip_max->temperature);
>   
>   	reset_pid_controller(params);
>   

LGTM

Reviewed-by: Lukasz Luba <lukasz.luba@xxxxxxx>