| Message ID | 20250219210302.442954-10-superm1@kernel.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | amd-pstate cleanups | expand |
On 2/20/2025 2:32 AM, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > Enums are effectively used as a boolean and don't show > the return value of the failing call. > > Instead of using enums switch to returning the actual return > code from the unit test. > One query below, apart from that LGTM, Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com> > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate-ut.c | 143 ++++++++++++-------------------- > 1 file changed, 55 insertions(+), 88 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c > index 0f0b867e271cc..028527a0019ca 100644 > --- a/drivers/cpufreq/amd-pstate-ut.c > +++ b/drivers/cpufreq/amd-pstate-ut.c > @@ -32,30 +32,20 @@ > > #include "amd-pstate.h" > > -/* > - * Abbreviations: > - * amd_pstate_ut: used as a shortform for AMD P-State unit test. > - * It helps to keep variable names smaller, simpler > - */ > -enum amd_pstate_ut_result { > - AMD_PSTATE_UT_RESULT_PASS, > - AMD_PSTATE_UT_RESULT_FAIL, > -}; > > struct amd_pstate_ut_struct { > const char *name; > - void (*func)(u32 index); > - enum amd_pstate_ut_result result; > + int (*func)(u32 index); > }; > > /* > * Kernel module for testing the AMD P-State unit test > */ > -static void amd_pstate_ut_acpi_cpc_valid(u32 index); > -static void amd_pstate_ut_check_enabled(u32 index); > -static void amd_pstate_ut_check_perf(u32 index); > -static void amd_pstate_ut_check_freq(u32 index); > -static void amd_pstate_ut_check_driver(u32 index); > +static int amd_pstate_ut_acpi_cpc_valid(u32 index); > +static int amd_pstate_ut_check_enabled(u32 index); > +static int amd_pstate_ut_check_perf(u32 index); > +static int amd_pstate_ut_check_freq(u32 index); > +static int amd_pstate_ut_check_driver(u32 index); > > static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { > {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, > @@ -78,51 +68,46 @@ static bool get_shared_mem(void) > /* > * check the _CPC object is present in SBIOS. > */ > -static void amd_pstate_ut_acpi_cpc_valid(u32 index) > +static int amd_pstate_ut_acpi_cpc_valid(u32 index) > { > - if (acpi_cpc_valid()) > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; > - else { > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; > + if (!acpi_cpc_valid()) { > pr_err("%s the _CPC object is not present in SBIOS!\n", __func__); > + return -EINVAL; > } > + > + return 0; > } > > -static void amd_pstate_ut_pstate_enable(u32 index) > +/* > + * check if amd pstate is enabled > + */ > +static int amd_pstate_ut_check_enabled(u32 index) > { > - int ret = 0; > u64 cppc_enable = 0; > + int ret; > + > + if (get_shared_mem()) > + return 0; What do you think about adding a "cppc_get_enable()" function in acpi_cppc.c so that we can run this check for shared mem systems as well ? Thanks, Dhananjay > > ret = rdmsrl_safe(MSR_AMD_CPPC_ENABLE, &cppc_enable); > if (ret) { > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; > pr_err("%s rdmsrl_safe MSR_AMD_CPPC_ENABLE ret=%d error!\n", __func__, ret); > - return; > + return ret; > } > - if (cppc_enable) > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; > - else { > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; > + > + if (!cppc_enable) { > pr_err("%s amd pstate must be enabled!\n", __func__); > + return -EINVAL; > } > -} > > -/* > - * check if amd pstate is enabled > - */ > -static void amd_pstate_ut_check_enabled(u32 index) > -{ > - if (get_shared_mem()) > - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; > - else > - amd_pstate_ut_pstate_enable(index); > + return 0; > } > > /* > * check if performance values are reasonable. > * highest_perf >= nominal_perf > lowest_nonlinear_perf > lowest_perf > 0 > */ > -static void amd_pstate_ut_check_perf(u32 index) > +static int amd_pstate_ut_check_perf(u32 index) > { > int cpu = 0, ret = 0; > u32 highest_perf = 0, nominal_perf = 0, lowest_nonlinear_perf = 0, lowest_perf = 0; [Snip]
On 2/24/2025 00:05, Dhananjay Ugwekar wrote: > On 2/20/2025 2:32 AM, Mario Limonciello wrote: >> From: Mario Limonciello <mario.limonciello@amd.com> >> >> Enums are effectively used as a boolean and don't show >> the return value of the failing call. >> >> Instead of using enums switch to returning the actual return >> code from the unit test. >> > > One query below, apart from that LGTM, > > Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com> Thanks! > >> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> drivers/cpufreq/amd-pstate-ut.c | 143 ++++++++++++-------------------- >> 1 file changed, 55 insertions(+), 88 deletions(-) >> >> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c >> index 0f0b867e271cc..028527a0019ca 100644 >> --- a/drivers/cpufreq/amd-pstate-ut.c >> +++ b/drivers/cpufreq/amd-pstate-ut.c >> @@ -32,30 +32,20 @@ >> >> #include "amd-pstate.h" >> >> -/* >> - * Abbreviations: >> - * amd_pstate_ut: used as a shortform for AMD P-State unit test. >> - * It helps to keep variable names smaller, simpler >> - */ >> -enum amd_pstate_ut_result { >> - AMD_PSTATE_UT_RESULT_PASS, >> - AMD_PSTATE_UT_RESULT_FAIL, >> -}; >> >> struct amd_pstate_ut_struct { >> const char *name; >> - void (*func)(u32 index); >> - enum amd_pstate_ut_result result; >> + int (*func)(u32 index); >> }; >> >> /* >> * Kernel module for testing the AMD P-State unit test >> */ >> -static void amd_pstate_ut_acpi_cpc_valid(u32 index); >> -static void amd_pstate_ut_check_enabled(u32 index); >> -static void amd_pstate_ut_check_perf(u32 index); >> -static void amd_pstate_ut_check_freq(u32 index); >> -static void amd_pstate_ut_check_driver(u32 index); >> +static int amd_pstate_ut_acpi_cpc_valid(u32 index); >> +static int amd_pstate_ut_check_enabled(u32 index); >> +static int amd_pstate_ut_check_perf(u32 index); >> +static int amd_pstate_ut_check_freq(u32 index); >> +static int amd_pstate_ut_check_driver(u32 index); >> >> static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { >> {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, >> @@ -78,51 +68,46 @@ static bool get_shared_mem(void) >> /* >> * check the _CPC object is present in SBIOS. >> */ >> -static void amd_pstate_ut_acpi_cpc_valid(u32 index) >> +static int amd_pstate_ut_acpi_cpc_valid(u32 index) >> { >> - if (acpi_cpc_valid()) >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; >> - else { >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; >> + if (!acpi_cpc_valid()) { >> pr_err("%s the _CPC object is not present in SBIOS!\n", __func__); >> + return -EINVAL; >> } >> + >> + return 0; >> } >> >> -static void amd_pstate_ut_pstate_enable(u32 index) >> +/* >> + * check if amd pstate is enabled >> + */ >> +static int amd_pstate_ut_check_enabled(u32 index) >> { >> - int ret = 0; >> u64 cppc_enable = 0; >> + int ret; >> + >> + if (get_shared_mem()) >> + return 0; > > What do you think about adding a "cppc_get_enable()" function in acpi_cppc.c so that we can > run this check for shared mem systems as well ? > I think it's a good idea. Would you mind working that out for after this series lands? > Thanks, > Dhananjay > >> >> ret = rdmsrl_safe(MSR_AMD_CPPC_ENABLE, &cppc_enable); >> if (ret) { >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; >> pr_err("%s rdmsrl_safe MSR_AMD_CPPC_ENABLE ret=%d error!\n", __func__, ret); >> - return; >> + return ret; >> } >> - if (cppc_enable) >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; >> - else { >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; >> + >> + if (!cppc_enable) { >> pr_err("%s amd pstate must be enabled!\n", __func__); >> + return -EINVAL; >> } >> -} >> >> -/* >> - * check if amd pstate is enabled >> - */ >> -static void amd_pstate_ut_check_enabled(u32 index) >> -{ >> - if (get_shared_mem()) >> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; >> - else >> - amd_pstate_ut_pstate_enable(index); >> + return 0; >> } >> >> /* >> * check if performance values are reasonable. >> * highest_perf >= nominal_perf > lowest_nonlinear_perf > lowest_perf > 0 >> */ >> -static void amd_pstate_ut_check_perf(u32 index) >> +static int amd_pstate_ut_check_perf(u32 index) >> { >> int cpu = 0, ret = 0; >> u32 highest_perf = 0, nominal_perf = 0, lowest_nonlinear_perf = 0, lowest_perf = 0; > [Snip]
On 2/25/2025 5:35 AM, Mario Limonciello wrote: > On 2/24/2025 00:05, Dhananjay Ugwekar wrote: >> On 2/20/2025 2:32 AM, Mario Limonciello wrote: >>> From: Mario Limonciello <mario.limonciello@amd.com> >>> >>> Enums are effectively used as a boolean and don't show >>> the return value of the failing call. >>> >>> Instead of using enums switch to returning the actual return >>> code from the unit test. >>> >> >> One query below, apart from that LGTM, >> >> Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com> > > Thanks! > >> >>> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> >>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>> --- >>> drivers/cpufreq/amd-pstate-ut.c | 143 ++++++++++++-------------------- >>> 1 file changed, 55 insertions(+), 88 deletions(-) >>> >>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c >>> index 0f0b867e271cc..028527a0019ca 100644 >>> --- a/drivers/cpufreq/amd-pstate-ut.c >>> +++ b/drivers/cpufreq/amd-pstate-ut.c >>> @@ -32,30 +32,20 @@ >>> #include "amd-pstate.h" >>> -/* >>> - * Abbreviations: >>> - * amd_pstate_ut: used as a shortform for AMD P-State unit test. >>> - * It helps to keep variable names smaller, simpler >>> - */ >>> -enum amd_pstate_ut_result { >>> - AMD_PSTATE_UT_RESULT_PASS, >>> - AMD_PSTATE_UT_RESULT_FAIL, >>> -}; >>> struct amd_pstate_ut_struct { >>> const char *name; >>> - void (*func)(u32 index); >>> - enum amd_pstate_ut_result result; >>> + int (*func)(u32 index); >>> }; >>> /* >>> * Kernel module for testing the AMD P-State unit test >>> */ >>> -static void amd_pstate_ut_acpi_cpc_valid(u32 index); >>> -static void amd_pstate_ut_check_enabled(u32 index); >>> -static void amd_pstate_ut_check_perf(u32 index); >>> -static void amd_pstate_ut_check_freq(u32 index); >>> -static void amd_pstate_ut_check_driver(u32 index); >>> +static int amd_pstate_ut_acpi_cpc_valid(u32 index); >>> +static int amd_pstate_ut_check_enabled(u32 index); >>> +static int amd_pstate_ut_check_perf(u32 index); >>> +static int amd_pstate_ut_check_freq(u32 index); >>> +static int amd_pstate_ut_check_driver(u32 index); >>> static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { >>> {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, >>> @@ -78,51 +68,46 @@ static bool get_shared_mem(void) >>> /* >>> * check the _CPC object is present in SBIOS. >>> */ >>> -static void amd_pstate_ut_acpi_cpc_valid(u32 index) >>> +static int amd_pstate_ut_acpi_cpc_valid(u32 index) >>> { >>> - if (acpi_cpc_valid()) >>> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; >>> - else { >>> - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; >>> + if (!acpi_cpc_valid()) { >>> pr_err("%s the _CPC object is not present in SBIOS!\n", __func__); >>> + return -EINVAL; >>> } >>> + >>> + return 0; >>> } >>> -static void amd_pstate_ut_pstate_enable(u32 index) >>> +/* >>> + * check if amd pstate is enabled >>> + */ >>> +static int amd_pstate_ut_check_enabled(u32 index) >>> { >>> - int ret = 0; >>> u64 cppc_enable = 0; >>> + int ret; >>> + >>> + if (get_shared_mem()) >>> + return 0; >> >> What do you think about adding a "cppc_get_enable()" function in acpi_cppc.c so that we can >> run this check for shared mem systems as well ? >> > > I think it's a good idea. Would you mind working that out for after this series lands? Sure, I'll take it up. > >> Thanks, >> Dhananjay
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c index 0f0b867e271cc..028527a0019ca 100644 --- a/drivers/cpufreq/amd-pstate-ut.c +++ b/drivers/cpufreq/amd-pstate-ut.c @@ -32,30 +32,20 @@ #include "amd-pstate.h" -/* - * Abbreviations: - * amd_pstate_ut: used as a shortform for AMD P-State unit test. - * It helps to keep variable names smaller, simpler - */ -enum amd_pstate_ut_result { - AMD_PSTATE_UT_RESULT_PASS, - AMD_PSTATE_UT_RESULT_FAIL, -}; struct amd_pstate_ut_struct { const char *name; - void (*func)(u32 index); - enum amd_pstate_ut_result result; + int (*func)(u32 index); }; /* * Kernel module for testing the AMD P-State unit test */ -static void amd_pstate_ut_acpi_cpc_valid(u32 index); -static void amd_pstate_ut_check_enabled(u32 index); -static void amd_pstate_ut_check_perf(u32 index); -static void amd_pstate_ut_check_freq(u32 index); -static void amd_pstate_ut_check_driver(u32 index); +static int amd_pstate_ut_acpi_cpc_valid(u32 index); +static int amd_pstate_ut_check_enabled(u32 index); +static int amd_pstate_ut_check_perf(u32 index); +static int amd_pstate_ut_check_freq(u32 index); +static int amd_pstate_ut_check_driver(u32 index); static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = { {"amd_pstate_ut_acpi_cpc_valid", amd_pstate_ut_acpi_cpc_valid }, @@ -78,51 +68,46 @@ static bool get_shared_mem(void) /* * check the _CPC object is present in SBIOS. */ -static void amd_pstate_ut_acpi_cpc_valid(u32 index) +static int amd_pstate_ut_acpi_cpc_valid(u32 index) { - if (acpi_cpc_valid()) - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; - else { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; + if (!acpi_cpc_valid()) { pr_err("%s the _CPC object is not present in SBIOS!\n", __func__); + return -EINVAL; } + + return 0; } -static void amd_pstate_ut_pstate_enable(u32 index) +/* + * check if amd pstate is enabled + */ +static int amd_pstate_ut_check_enabled(u32 index) { - int ret = 0; u64 cppc_enable = 0; + int ret; + + if (get_shared_mem()) + return 0; ret = rdmsrl_safe(MSR_AMD_CPPC_ENABLE, &cppc_enable); if (ret) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s rdmsrl_safe MSR_AMD_CPPC_ENABLE ret=%d error!\n", __func__, ret); - return; + return ret; } - if (cppc_enable) - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; - else { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; + + if (!cppc_enable) { pr_err("%s amd pstate must be enabled!\n", __func__); + return -EINVAL; } -} -/* - * check if amd pstate is enabled - */ -static void amd_pstate_ut_check_enabled(u32 index) -{ - if (get_shared_mem()) - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; - else - amd_pstate_ut_pstate_enable(index); + return 0; } /* * check if performance values are reasonable. * highest_perf >= nominal_perf > lowest_nonlinear_perf > lowest_perf > 0 */ -static void amd_pstate_ut_check_perf(u32 index) +static int amd_pstate_ut_check_perf(u32 index) { int cpu = 0, ret = 0; u32 highest_perf = 0, nominal_perf = 0, lowest_nonlinear_perf = 0, lowest_perf = 0; @@ -142,9 +127,8 @@ static void amd_pstate_ut_check_perf(u32 index) if (get_shared_mem()) { ret = cppc_get_perf_caps(cpu, &cppc_perf); if (ret) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s cppc_get_perf_caps ret=%d error!\n", __func__, ret); - return; + return ret; } highest_perf = cppc_perf.highest_perf; @@ -154,9 +138,8 @@ static void amd_pstate_ut_check_perf(u32 index) } else { ret = rdmsrl_safe_on_cpu(cpu, MSR_AMD_CPPC_CAP1, &cap1); if (ret) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s read CPPC_CAP1 ret=%d error!\n", __func__, ret); - return; + return ret; } highest_perf = AMD_CPPC_HIGHEST_PERF(cap1); @@ -169,32 +152,30 @@ static void amd_pstate_ut_check_perf(u32 index) if (highest_perf != cur_perf.highest_perf && !cpudata->hw_prefcore) { pr_err("%s cpu%d highest=%d %d highest perf doesn't match\n", __func__, cpu, highest_perf, cpudata->perf.highest_perf); - return; + return -EINVAL; } if (nominal_perf != cur_perf.nominal_perf || (lowest_nonlinear_perf != cur_perf.lowest_nonlinear_perf) || (lowest_perf != cur_perf.lowest_perf)) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s cpu%d nominal=%d %d lowest_nonlinear=%d %d lowest=%d %d, they should be equal!\n", __func__, cpu, nominal_perf, cpudata->perf.nominal_perf, lowest_nonlinear_perf, cpudata->perf.lowest_nonlinear_perf, lowest_perf, cpudata->perf.lowest_perf); - return; + return -EINVAL; } if (!((highest_perf >= nominal_perf) && (nominal_perf > lowest_nonlinear_perf) && (lowest_nonlinear_perf >= lowest_perf) && (lowest_perf > 0))) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s cpu%d highest=%d >= nominal=%d > lowest_nonlinear=%d > lowest=%d > 0, the formula is incorrect!\n", __func__, cpu, highest_perf, nominal_perf, lowest_nonlinear_perf, lowest_perf); - return; + return -EINVAL; } } - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; + return 0; } /* @@ -202,7 +183,7 @@ static void amd_pstate_ut_check_perf(u32 index) * max_freq >= nominal_freq > lowest_nonlinear_freq > min_freq > 0 * check max freq when set support boost mode. */ -static void amd_pstate_ut_check_freq(u32 index) +static int amd_pstate_ut_check_freq(u32 index) { int cpu = 0; struct amd_cpudata *cpudata = NULL; @@ -219,39 +200,33 @@ static void amd_pstate_ut_check_freq(u32 index) (cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) && (cpudata->lowest_nonlinear_freq >= policy->cpuinfo.min_freq) && (policy->cpuinfo.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, policy->cpuinfo.max_freq, cpudata->nominal_freq, cpudata->lowest_nonlinear_freq, policy->cpuinfo.min_freq); - return; + return -EINVAL; } if (cpudata->lowest_nonlinear_freq != policy->min) { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s cpu%d cpudata_lowest_nonlinear_freq=%d policy_min=%d, they should be equal!\n", __func__, cpu, cpudata->lowest_nonlinear_freq, policy->min); - return; + return -EINVAL; } if (cpudata->boost_supported) { - if ((policy->max == policy->cpuinfo.max_freq) || - (policy->max == cpudata->nominal_freq)) - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; - else { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; + if ((policy->max != policy->cpuinfo.max_freq) && + (policy->max != cpudata->nominal_freq)) { pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n", __func__, cpu, policy->max, policy->cpuinfo.max_freq, cpudata->nominal_freq); - return; + return -EINVAL; } } else { - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL; pr_err("%s cpu%d must support boost!\n", __func__, cpu); - return; + return -EINVAL; } } - amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; + return 0; } static int amd_pstate_set_mode(enum amd_pstate_mode mode) @@ -263,32 +238,28 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode) return amd_pstate_update_status(mode_str, strlen(mode_str)); } -static void amd_pstate_ut_check_driver(u32 index) +static int amd_pstate_ut_check_driver(u32 index) { enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE; - int ret; for (mode1 = AMD_PSTATE_DISABLE; mode1 < AMD_PSTATE_MAX; mode1++) { - ret = amd_pstate_set_mode(mode1); + int ret = amd_pstate_set_mode(mode1); if (ret) - goto out; + return ret; for (mode2 = AMD_PSTATE_DISABLE; mode2 < AMD_PSTATE_MAX; mode2++) { if (mode1 == mode2) continue; ret = amd_pstate_set_mode(mode2); - if (ret) - goto out; + if (ret) { + pr_err("%s: failed to update status for %s->%s\n", __func__, + amd_pstate_get_mode_string(mode1), + amd_pstate_get_mode_string(mode2)); + return ret; + } } } -out: - if (ret) - pr_warn("%s: failed to update status for %s->%s: %d\n", __func__, - amd_pstate_get_mode_string(mode1), - amd_pstate_get_mode_string(mode2), ret); - - amd_pstate_ut_cases[index].result = ret ? - AMD_PSTATE_UT_RESULT_FAIL : - AMD_PSTATE_UT_RESULT_PASS; + + return 0; } static int __init amd_pstate_ut_init(void) @@ -296,16 +267,12 @@ static int __init amd_pstate_ut_init(void) u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases); for (i = 0; i < arr_size; i++) { - amd_pstate_ut_cases[i].func(i); - switch (amd_pstate_ut_cases[i].result) { - case AMD_PSTATE_UT_RESULT_PASS: + int ret = amd_pstate_ut_cases[i].func(i); + + if (ret) + pr_err("%-4d %-20s\t fail: %d!\n", i+1, amd_pstate_ut_cases[i].name, ret); + else pr_info("%-4d %-20s\t success!\n", i+1, amd_pstate_ut_cases[i].name); - break; - case AMD_PSTATE_UT_RESULT_FAIL: - default: - pr_info("%-4d %-20s\t fail!\n", i+1, amd_pstate_ut_cases[i].name); - break; - } } return 0;