Re: [PATCH v3 08/12] amd-pstate-ut: Add ability to run a single testcase

["Gautham R. Shenoy" <gautham.shenoy@amd.com>]

Hello Mario,

On Mon, Mar 23, 2026 at 03:21:17PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:
> 
> 
> On 3/20/2026 9:43 AM, Gautham R. Shenoy wrote:
> > Currently when amd-pstate-ut test module is loaded, it runs all the
> > tests from amd_pstate_ut_cases[] array.
> > 
> > Add a module parameter named "run_only" that allows users to run a
> > single test from the array by specifying the test name string.
> > 
> > Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
> > ---
> >   drivers/cpufreq/amd-pstate-ut.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> > index 447b9aa5ce40..35e453a49c0f 100644
> > --- a/drivers/cpufreq/amd-pstate-ut.c
> > +++ b/drivers/cpufreq/amd-pstate-ut.c
> > @@ -35,6 +35,10 @@
> >   #include "amd-pstate.h"
> > +static char *run_only;
> > +module_param(run_only, charp, 0444);
> > +MODULE_PARM_DESC(run_only,
> > +	"Run only the named test case (default: run all)");
> 
> This default shows the end effect; but it doesn't make sense for this
> parameter IMO.
> 
> How about instead if you had a semicolon delimitted list and then defaulted
> an empty list to mean all tests?  Something like this:
> 
> static char *test_list;
> module_param(test_list, charp, 0444)
> MODULE_PARM_DESC(test_list,
> 	"Semicolon delimitted list of tests to run (empty means run all tests)");


This makes sense.

> 
> >   struct amd_pstate_ut_struct {
> >   	const char *name;
> > @@ -275,7 +279,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++) {
> > -		int ret = amd_pstate_ut_cases[i].func(i);
> > +		int ret;
> > +
> > +		if (run_only && strcmp(run_only, amd_pstate_ut_cases[i].name))
> > +			continue;
> > +
> > +		ret = amd_pstate_ut_cases[i].func(i);
> 
> If you take my suggestion then you would split this on semicolon or end of
> string and then allow matching multiple.

How about something like the following (diff on top of this
patch. Will fold it in and post a v4 if this looks ok)

x8----------------------------------------x8---------------------------------------------x8
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 5d87fb8a26df..5ef22a77a9c5 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -37,10 +37,10 @@
 
 #include "amd-pstate.h"
 
-static char *run_only;
-module_param(run_only, charp, 0444);
-MODULE_PARM_DESC(run_only,
-       "Run only the named test case (default: run all)");
+static char *test_list;
+module_param(test_list, charp, 0444);
+MODULE_PARM_DESC(test_list,
+       "Semicolon-delimited list of tests to run (empty means run all tests)");
 
 struct amd_pstate_ut_struct {
        const char *name;
@@ -403,6 +403,26 @@ static int amd_pstate_ut_check_freq_attrs(u32 index)
        return ret;
 }
 
+static bool test_in_list(const char *list, const char *name)
+{
+       size_t name_len = strlen(name);
+       const char *p = list;
+
+       while (*p) {
+               const char *sep = strchr(p, ';');
+               size_t token_len = sep ? sep - p : strlen(p);
+
+               if (token_len == name_len && !strncmp(p, name, token_len))
+                       return true;
+
+               if (!sep)
+                       break;
+               p = sep + 1;
+       }
+
+       return false;
+}
+
 static int __init amd_pstate_ut_init(void)
 {
        u32 i = 0, arr_size = ARRAY_SIZE(amd_pstate_ut_cases);
@@ -410,7 +430,8 @@ static int __init amd_pstate_ut_init(void)
        for (i = 0; i < arr_size; i++) {
                int ret;
 
-               if (run_only && strcmp(run_only, amd_pstate_ut_cases[i].name))
+               if (test_list && *test_list &&
+                   !test_in_list(test_list, amd_pstate_ut_cases[i].name))
                        continue;
 
                ret = amd_pstate_ut_cases[i].func(i);
x8----------------------------------------x8---------------------------------------------x8

-- 
Thanks and Regards
gautham.