| Message ID | 20230616120620.147643-2-wyes.karny@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cpupower: Add various feature control support for amd_pstate | expand |
On 6/16/23 07:06, Wyes Karny wrote: > amd-pstate active mode driver name is "amd-pstate-epp". Add this to the > string matching condition to recognise amd-pstate active mode driver. > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > --- > tools/power/cpupower/utils/helpers/misc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c > index 9547b29254a7..21f653cd472c 100644 > --- a/tools/power/cpupower/utils/helpers/misc.c > +++ b/tools/power/cpupower/utils/helpers/misc.c > @@ -95,7 +95,7 @@ bool cpupower_amd_pstate_enabled(void) > if (!driver) > return ret; > > - if (!strcmp(driver, "amd-pstate")) > + if (!strcmp(driver, "amd-pstate") || !strcmp(driver, "amd-pstate-epp")) To avoid getting caught in the case that a kernel didn't have the patch separated from this series (for example if a distro missed it in a backport from separate directories), how about using strncmp() instead and just look for the prefix? This would also let the tool be more future proofed in the case another amd-pstate driver was introduced later down the road as long as it stuck to "amd-pstate*" > ret = true; > > cpufreq_put_driver(driver);
Hi Mario, On 18 Jun 20:58, Mario Limonciello wrote: > On 6/16/23 07:06, Wyes Karny wrote: > > amd-pstate active mode driver name is "amd-pstate-epp". Add this to the > > string matching condition to recognise amd-pstate active mode driver. > > > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> > > Signed-off-by: Wyes Karny <wyes.karny@amd.com> > > --- > > tools/power/cpupower/utils/helpers/misc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c > > index 9547b29254a7..21f653cd472c 100644 > > --- a/tools/power/cpupower/utils/helpers/misc.c > > +++ b/tools/power/cpupower/utils/helpers/misc.c > > @@ -95,7 +95,7 @@ bool cpupower_amd_pstate_enabled(void) > > if (!driver) > > return ret; > > - if (!strcmp(driver, "amd-pstate")) > > + if (!strcmp(driver, "amd-pstate") || !strcmp(driver, "amd-pstate-epp")) > > To avoid getting caught in the case that a kernel didn't have the patch > separated from this series (for example if a distro missed it in a backport > from separate directories), how about using strncmp() instead and just look > for the prefix? Sure, I'll update the patch. I'm thinking of using strncmp(driver, "amd", 3), because in the above case the epp driver would be "amd_pstate_epp" therefore common prefix is only "amd". Thanks, Wyes > > This would also let the tool be more future proofed in the case another > amd-pstate driver was introduced later down the road as long as it stuck to > "amd-pstate*" > > > ret = true; > > cpufreq_put_driver(driver); >
On 6/19/2023 12:31 PM, Wyes Karny wrote: > Hi Mario, > > On 18 Jun 20:58, Mario Limonciello wrote: >> On 6/16/23 07:06, Wyes Karny wrote: >>> amd-pstate active mode driver name is "amd-pstate-epp". Add this to the >>> string matching condition to recognise amd-pstate active mode driver. >>> >>> Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> >>> Signed-off-by: Wyes Karny <wyes.karny@amd.com> >>> --- >>> tools/power/cpupower/utils/helpers/misc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c >>> index 9547b29254a7..21f653cd472c 100644 >>> --- a/tools/power/cpupower/utils/helpers/misc.c >>> +++ b/tools/power/cpupower/utils/helpers/misc.c >>> @@ -95,7 +95,7 @@ bool cpupower_amd_pstate_enabled(void) >>> if (!driver) >>> return ret; >>> - if (!strcmp(driver, "amd-pstate")) >>> + if (!strcmp(driver, "amd-pstate") || !strcmp(driver, "amd-pstate-epp")) >> To avoid getting caught in the case that a kernel didn't have the patch >> separated from this series (for example if a distro missed it in a backport >> from separate directories), how about using strncmp() instead and just look >> for the prefix? > Sure, I'll update the patch. > I'm thinking of using strncmp(driver, "amd", 3), because in the above > case the epp driver would be "amd_pstate_epp" therefore common prefix is > only "amd". > > Thanks, > Wyes Sounds good, thanks! >> This would also let the tool be more future proofed in the case another >> amd-pstate driver was introduced later down the road as long as it stuck to >> "amd-pstate*" >> >>> ret = true; >>> cpufreq_put_driver(driver);
diff --git a/tools/power/cpupower/utils/helpers/misc.c b/tools/power/cpupower/utils/helpers/misc.c index 9547b29254a7..21f653cd472c 100644 --- a/tools/power/cpupower/utils/helpers/misc.c +++ b/tools/power/cpupower/utils/helpers/misc.c @@ -95,7 +95,7 @@ bool cpupower_amd_pstate_enabled(void) if (!driver) return ret; - if (!strcmp(driver, "amd-pstate")) + if (!strcmp(driver, "amd-pstate") || !strcmp(driver, "amd-pstate-epp")) ret = true; cpufreq_put_driver(driver);