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

[Mario Limonciello]

On 3/23/26 11:29 PM, Gautham R. Shenoy wrote:
> 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@xxxxxxx>
> > > ---
> > >    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)
Yeah that's good.

> 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