From: Mario Limonciello <mario.limonciello@amd.com>

Don't take and release the mutex when prefcore isn't present and
avoid initialization of variables that will be initially set
in the function.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 75568d0f84623..ed05d7a0add10 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu)
 	int ret;
 	bool highest_perf_changed = false;
 
-	mutex_lock(&amd_pstate_driver_lock);
-	if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore))
-		goto free_cpufreq_put;
+	if (!amd_pstate_prefcore)
+		return;
 
+	mutex_lock(&amd_pstate_driver_lock);
 	ret = amd_get_highest_perf(cpu, &cur_high);
 	if (ret)
 		goto free_cpufreq_put;
 
 	prev_high = READ_ONCE(cpudata->prefcore_ranking);
-	if (prev_high != cur_high) {
-		highest_perf_changed = true;
+	highest_perf_changed = (prev_high != cur_high);
+	if (highest_perf_changed) {
 		WRITE_ONCE(cpudata->prefcore_ranking, cur_high);
 
 		if (cur_high < CPPC_MAX_PERF)
-- 
2.43.0

On Mon, Aug 26, 2024 at 04:13:57PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> Don't take and release the mutex when prefcore isn't present and
> avoid initialization of variables that will be initially set
> in the function.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

> ---
>  drivers/cpufreq/amd-pstate.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 75568d0f84623..ed05d7a0add10 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu)
>  	int ret;
>  	bool highest_perf_changed = false;
>  
> -	mutex_lock(&amd_pstate_driver_lock);
> -	if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore))
> -		goto free_cpufreq_put;
> +	if (!amd_pstate_prefcore)
> +		return;

Looks good to me.

Wondering if it is worth maintaining a static key for
amd_pstate_prefcore. Anyway it doesn't change after boot.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>


--
Thanks and Regards
gautham.

On 8/27/2024 11:57, Gautham R. Shenoy wrote:
> On Mon, Aug 26, 2024 at 04:13:57PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> Don't take and release the mutex when prefcore isn't present and
>> avoid initialization of variables that will be initially set
>> in the function.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
>> ---
>>   drivers/cpufreq/amd-pstate.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index 75568d0f84623..ed05d7a0add10 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -798,17 +798,17 @@ static void amd_pstate_update_limits(unsigned int cpu)
>>   	int ret;
>>   	bool highest_perf_changed = false;
>>   
>> -	mutex_lock(&amd_pstate_driver_lock);
>> -	if ((!amd_pstate_prefcore) || (!cpudata->hw_prefcore))
>> -		goto free_cpufreq_put;
>> +	if (!amd_pstate_prefcore)
>> +		return;
> 
> Looks good to me.
> 
> Wondering if it is worth maintaining a static key for
> amd_pstate_prefcore. Anyway it doesn't change after boot.

As there is a kernel command line option how would you pass the early 
param parsing result over without a static variable?

> 
> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> 
Thanks!

> 
> --
> Thanks and Regards
> gautham.