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);
--
2.34.1
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# bad: [e99058f87395eb9a263bcadca0ae402d34a96326] crypto: rsassa-pkcs1 - Migrate to sig_alg backend
git bisect start 'HEAD'
# status: waiting for good commit(s), bad commit known
# good: [4e46774408d942efe4eb35dc62e5af3af71b9a30] Merge tag 'for-6.12-rc4-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
git bisect good 4e46774408d942efe4eb35dc62e5af3af71b9a30
# bad: [8f803f3cc3ecbdb5ea61dbe6c38461f6ff75adae] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/ath/ath.git
git bisect bad 8f803f3cc3ecbdb5ea61dbe6c38461f6ff75adae
# good: [b5ad19502c85ad4d8c8237d614e383dd31a920b3] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/uml/linux.git
git bisect good b5ad19502c85ad4d8c8237d614e383dd31a920b3
# bad: [ba516a4971bb3ca67dff6dcd13d04dc63fbde292] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git
git bisect bad ba516a4971bb3ca67dff6dcd13d04dc63fbde292
# good: [e4f77835fcb3e30655e8a631358715ace26e3675] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git
git bisect good e4f77835fcb3e30655e8a631358715ace26e3675
# good: [21b9c329c48d88fb5adc1dbdc34f78b3da0d91a6] Merge branch 'docs-next' of git://git.lwn.net/linux.git
git bisect good 21b9c329c48d88fb5adc1dbdc34f78b3da0d91a6
# good: [e96c6b4a04e1ae12e0144300057af6939dfd35eb] Merge branch 'next' of git://git.linuxtv.org/media
git bisect good e96c6b4a04e1ae12e0144300057af6939dfd35eb
# good: [8a79bf447d8948d136ec64b7070bf7923c60dafd] Merge branch 'thermal-core' into linux-next
git bisect good 8a79bf447d8948d136ec64b7070bf7923c60dafd
# bad: [4ae7df8e2051753c5b6bed657a8d114d9faf02b7] Merge branch 'cpupower' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux.git
git bisect bad 4ae7df8e2051753c5b6bed657a8d114d9faf02b7
# bad: [9609386c86eedfb402b6c7d0c49f1b7cbb90729b] Merge tag 'amd-pstate-v6.13-2024-10-22' of ssh://gitolite.kernel.org/pub/scm/linux/kernel/git/superm1/linux
git bisect bad 9609386c86eedfb402b6c7d0c49f1b7cbb90729b
# good: [2af8e3e7d80086b617d4b952bff512fec2b9c909] cpufreq/amd-pstate: Remove the redundant verify() function
git bisect good 2af8e3e7d80086b617d4b952bff512fec2b9c909
# good: [55776e5ca936184bcb6805a7cfabbf5c7c7aaf17] cpufreq/amd-pstate: Remove the switch case in amd_pstate_init()
git bisect good 55776e5ca936184bcb6805a7cfabbf5c7c7aaf17
# bad: [61e9559176a1d7ebe7492f5203259ecf272e52b0] amd-pstate: Set min_perf to nominal_perf for active mode performance gov
git bisect bad 61e9559176a1d7ebe7492f5203259ecf272e52b0
# bad: [e238968a2087a0f7799ba3c5ae5d9a666c52e324] cpufreq/amd-pstate: Remove the redundant amd_pstate_set_driver() call
git bisect bad e238968a2087a0f7799ba3c5ae5d9a666c52e324
# first bad commit: [e238968a2087a0f7799ba3c5ae5d9a666c52e324] cpufreq/amd-pstate: Remove the redundant amd_pstate_set_driver() call
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);
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>
© 2016 - 2026 Red Hat, Inc.