cpufreq/amd-pstate-ut: Drop SUCCESS and FAIL enums

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.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
Reviewed-by: Dhananjay Ugwekar <dhananjay.ugwekar@amd.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index b3693a4c28d26760d75d64bc00de41b6e76f1d96..cd9a472e8dc3c9e86f6066ebae64135562e49456 100644 (file)
--- 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, cur_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, cur_perf.nominal_perf,
                                lowest_nonlinear_perf, cur_perf.lowest_nonlinear_perf,
                                lowest_perf, cur_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;