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

[Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com>]

Hello Mario,

On 6/25/2024 8:41 PM, Mario Limonciello wrote:
> 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?

Yup, will do!

Thanks,
Dhananjay

> 
>>
>> 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 {
>