| Message ID | 20241017100528.300143-5-Dhananjay.Ugwekar@amd.com |
|---|---|
| State | Accepted |
| Commit | b427ac4084753d7c62ef797bed7f30203f0ef5fb |
| Headers | show |
| Series | cpufreq/amd-pstate: Refactor amd_pstate_init() | expand |
On 2024-10-27 05:27, Mario Limonciello wrote: > > > On 10/26/24 06:58, Klara Modin wrote: >> Hi, >> >> On 2024-10-17 12:05, Dhananjay Ugwekar wrote: >>> amd_pstate_set_driver() is called twice, once in amd_pstate_init() >>> and once >>> as part of amd_pstate_register_driver(). Move around code and eliminate >>> the redundancy. >>> >>> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> >>> --- >>> drivers/cpufreq/amd-pstate.c | 12 ++++-------- >>> 1 file changed, 4 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c >>> index 13ee5cac901d..6f6d961879cc 100644 >>> --- a/drivers/cpufreq/amd-pstate.c >>> +++ b/drivers/cpufreq/amd-pstate.c >>> @@ -1848,9 +1848,11 @@ static int __init amd_pstate_init(void) >>> return -ENODEV; >>> } >>> - ret = amd_pstate_set_driver(cppc_state); >>> - if (ret) >>> + ret = amd_pstate_register_driver(cppc_state); >>> + if (ret) { >>> + pr_err("failed to register with return %d\n", ret); >>> return ret; >>> + } >>> /* capability check */ >>> if (cpu_feature_enabled(X86_FEATURE_CPPC)) { >>> @@ -1870,12 +1872,6 @@ static int __init amd_pstate_init(void) >>> return ret; >>> } >>> - ret = amd_pstate_register_driver(cppc_state); >>> - if (ret) { >>> - pr_err("failed to register with return %d\n", ret); >>> - return ret; >>> - } >>> - >>> dev_root = bus_get_dev_root(&cpu_subsys); >>> if (dev_root) { >>> ret = sysfs_create_group(&dev_root->kobj, >>> &amd_pstate_global_attr_group); >> >> >> This seems to break boot on my Zen 2 desktop (my Zen 4 laptop is fine, >> however). I don't see any messages on the console and nothing shows up >> in the pstore either. >> >> I'll attach the kernel log from a normal boot (with the patch >> reverted) in case it helps. >> >> Please let me know if there's anything else you need. >> >> Regards, >> Klara Modin > > Hi, > > Thanks for reporting. I think this might be a regression on shared > memory designs specifically because static calls weren't updated yet. > > Can you try this below diff? > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 206725219d8c9..2f0e29b05963d 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -1857,12 +1857,6 @@ static int __init amd_pstate_init(void) > return -ENODEV; > } > > - ret = amd_pstate_register_driver(cppc_state); > - if (ret) { > - pr_err("failed to register with return %d\n", ret); > - return ret; > - } > - > /* capability check */ > if (cpu_feature_enabled(X86_FEATURE_CPPC)) { > pr_debug("AMD CPPC MSR based functionality is > supported\n"); > @@ -1881,6 +1875,12 @@ static int __init amd_pstate_init(void) > return ret; > } > > + ret = amd_pstate_register_driver(cppc_state); > + if (ret) { > + pr_err("failed to register with return %d\n", ret); > + return ret; > + } > + > dev_root = bus_get_dev_root(&cpu_subsys); > if (dev_root) { > ret = sysfs_create_group(&dev_root->kobj, > &amd_pstate_global_attr_group); > Moving the it afterwards does fix the issue, thanks! Tested-by: Klara Modin <klarasmodin@gmail.com>
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 13ee5cac901d..6f6d961879cc 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -1848,9 +1848,11 @@ static int __init amd_pstate_init(void) return -ENODEV; } - ret = amd_pstate_set_driver(cppc_state); - if (ret) + ret = amd_pstate_register_driver(cppc_state); + if (ret) { + pr_err("failed to register with return %d\n", ret); return ret; + } /* capability check */ if (cpu_feature_enabled(X86_FEATURE_CPPC)) { @@ -1870,12 +1872,6 @@ static int __init amd_pstate_init(void) return ret; } - ret = amd_pstate_register_driver(cppc_state); - if (ret) { - pr_err("failed to register with return %d\n", ret); - return ret; - } - dev_root = bus_get_dev_root(&cpu_subsys); if (dev_root) { ret = sysfs_create_group(&dev_root->kobj, &amd_pstate_global_attr_group);
amd_pstate_set_driver() is called twice, once in amd_pstate_init() and once as part of amd_pstate_register_driver(). Move around code and eliminate the redundancy. Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> --- drivers/cpufreq/amd-pstate.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)