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)");
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 (ret)
pr_err("%-4d %-20s\t fail: %d!\n", i+1, amd_pstate_ut_cases[i].name, ret);
--
2.34.1
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)");
>
> 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.
>
> if (ret)
> pr_err("%-4d %-20s\t fail: %d!\n", i+1, amd_pstate_ut_cases[i].name, ret);
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.
Hello Gautham,
On 3/24/2026 9:59 AM, Gautham R. Shenoy wrote:
> +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, ';');
Any particular reason for using a ";" as the separator instead of ","?
I personally prefer "," because with ";", I need to explicitly add
'' around the test_list otherwise bash thinks the command ends at ";"
but with "," that is avoided.
Thoughts?
> + 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;
> +}
--
Thanks and Regards,
Prateek
On 3/24/26 23:28, K Prateek Nayak wrote:
> Hello Gautham,
>
> On 3/24/2026 9:59 AM, Gautham R. Shenoy wrote:
>> +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, ';');
>
> Any particular reason for using a ";" as the separator instead of ","?
>
> I personally prefer "," because with ";", I need to explicitly add
> '' around the test_list otherwise bash thinks the command ends at ";"
> but with "," that is avoided.
>
> Thoughts?
>
Sure, that sounds like a good reason to use a comma instead.
>> + 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;
>> +}
On Wed, Mar 25, 2026 at 08:45:05AM -0500, Mario Limonciello wrote:
>
>
> On 3/24/26 23:28, K Prateek Nayak wrote:
> > Hello Gautham,
> >
> > On 3/24/2026 9:59 AM, Gautham R. Shenoy wrote:
> > > +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, ';');
> >
> > Any particular reason for using a ";" as the separator instead of ","?
> >
> > I personally prefer "," because with ";", I need to explicitly add
> > '' around the test_list otherwise bash thinks the command ends at ";"
> > but with "," that is avoided.
> >
> > Thoughts?
> >
>
> Sure, that sounds like a good reason to use a comma instead.
Sure, I wil change it to a comma-separated list in v4.
--
Thanks and Regards
gautham.
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
>
© 2016 - 2026 Red Hat, Inc.