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

[Mario Limonciello <superm1@kernel.org>]

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@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)
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
>