Received-SPF: pass (google.com: domain of linux-kernel+bounces-129734-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <1737b288-8550-407e-b501-469011b2a8cd@arm.com>
Date: Wed, 3 Apr 2024 13:48:40 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/3] thermal: gov_power_allocator: Allow binding
 without trip points
Content-Language: en-US
To: Nikita Travkin <nikitos.tr@gmail.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
 "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
 Daniel Lezcano <daniel.lezcano@linaro.org>, linux-pm@vger.kernel.org,
 linux-kernel@vger.kernel.org, Zhang Rui <rui.zhang@intel.com>
References: <20240403-gpa-no-cooling-devs-v2-0-79bdd8439449@trvn.ru>
 <20240403-gpa-no-cooling-devs-v2-2-79bdd8439449@trvn.ru>
From: Lukasz Luba <lukasz.luba@arm.com>
In-Reply-To: <20240403-gpa-no-cooling-devs-v2-2-79bdd8439449@trvn.ru>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


On 4/3/24 12:31, Nikita Travkin via B4 Relay wrote:
> From: Nikita Travkin <nikita@trvn.ru>
> 
> 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@trvn.ru>
> ---
>   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@arm.com>