On 06.03.2025 09:39, Penny Zheng wrote:
> @@ -514,5 +515,14 @@ acpi_cpufreq_driver = {
>
> int __init acpi_cpufreq_register(void)
> {
> - return cpufreq_register_driver(&acpi_cpufreq_driver);
> + int ret;
> +
> + ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> + if ( ret )
> + return ret;
> +
> + if ( IS_ENABLED(CONFIG_AMD) )
> + xen_processor_pmbits &= ~XEN_PROCESSOR_PM_CPPC;
What's the purpose of the if() here?
> @@ -157,7 +161,35 @@ static int __init cf_check cpufreq_driver_init(void)
>
> case X86_VENDOR_AMD:
> case X86_VENDOR_HYGON:
> - ret = IS_ENABLED(CONFIG_AMD) ? powernow_register_driver() :
> -ENODEV;
> + if ( !IS_ENABLED(CONFIG_AMD) )
> + {
> + ret = -ENODEV;
> + break;
> + }
> + ret = -ENOENT;
> +
> + for ( unsigned int i = 0; i < cpufreq_xen_cnt; i++ )
> + {
> + switch ( cpufreq_xen_opts[i] )
> + {
> + case CPUFREQ_xen:
> + ret = powernow_register_driver();
> + break;
> + case CPUFREQ_amd_cppc:
> + ret = amd_cppc_register_driver();
> + break;
> + case CPUFREQ_none:
> + ret = 0;
> + break;
> + default:
> + printk(XENLOG_WARNING
> + "Unsupported cpufreq driver for vendor AMD\n");
What about Hygon?
> --- a/xen/include/acpi/cpufreq/cpufreq.h
> +++ b/xen/include/acpi/cpufreq/cpufreq.h
> @@ -28,6 +28,7 @@ enum cpufreq_xen_opt {
> CPUFREQ_none,
> CPUFREQ_xen,
> CPUFREQ_hwp,
> + CPUFREQ_amd_cppc,
> };
> extern enum cpufreq_xen_opt cpufreq_xen_opts[2];
I'm pretty sure I pointed out before that this array needs to grow, now that
you add a 3rd kind of handling.
Jan
