Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency

[Mario Limonciello <mario.limonciello@amd.com>]

On 6/25/2024 08:51, Dhananjay Ugwekar wrote:
> Minor modification, the commit subject is supposed to be
> "cpufreq/amd-pstate-ut: Handle the inconsistency between nominal_freq and other *_freq units"
> 
> The second half disappeared due to the word wrapping I guess.

I had some other feedback on the series, so when you submit a v2 can you 
try to fix the title on the first patch?

> 
> Regards,
> Dhananjay
> 
> On 6/25/2024 7:11 PM, Dhananjay Ugwekar wrote:
>> cpudata->nominal_freq being in MHz whereas other frequencies being in
>> KHz breaks the amd-pstate-ut frequency sanity check. This fixes it.
>>
>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>
>> ---
>>   drivers/cpufreq/amd-pstate-ut.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>> index fc275d41d51e..66b73c308ce6 100644
>> --- a/drivers/cpufreq/amd-pstate-ut.c
>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>> @@ -202,6 +202,7 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   	int cpu = 0;
>>   	struct cpufreq_policy *policy = NULL;
>>   	struct amd_cpudata *cpudata = NULL;
>> +	u32 nominal_freq_khz;
>>   
>>   	for_each_possible_cpu(cpu) {
>>   		policy = cpufreq_cpu_get(cpu);
>> @@ -209,13 +210,14 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   			break;
>>   		cpudata = policy->driver_data;
>>   
>> -		if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
>> -			(cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
>> +		nominal_freq_khz = cpudata->nominal_freq*1000;
>> +		if (!((cpudata->max_freq >= nominal_freq_khz) &&
>> +			(nominal_freq_khz > cpudata->lowest_nonlinear_freq) &&
>>   			(cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
>>   			(cpudata->min_freq > 0))) {
>>   			amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>>   			pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
>> -				__func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
>> +				__func__, cpu, cpudata->max_freq, nominal_freq_khz,
>>   				cpudata->lowest_nonlinear_freq, cpudata->min_freq);
>>   			goto skip_test;
>>   		}
>> @@ -229,13 +231,13 @@ static void amd_pstate_ut_check_freq(u32 index)
>>   
>>   		if (cpudata->boost_supported) {
>>   			if ((policy->max == cpudata->max_freq) ||
>> -					(policy->max == cpudata->nominal_freq))
>> +					(policy->max == nominal_freq_khz))
>>   				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
>>   			else {
>>   				amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>>   				pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
>>   					__func__, cpu, policy->max, cpudata->max_freq,
>> -					cpudata->nominal_freq);
>> +					nominal_freq_khz);
>>   				goto skip_test;
>>   			}
>>   		} else {