On 12.11.2021 19:28, Andrew Cooper wrote:
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -640,13 +640,19 @@ static int __init cpufreq_driver_init(void)
> {
> int ret = 0;
>
> - if ((cpufreq_controller == FREQCTL_xen) &&
> - (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL))
> - ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> - else if ((cpufreq_controller == FREQCTL_xen) &&
> - (boot_cpu_data.x86_vendor &
> - (X86_VENDOR_AMD | X86_VENDOR_HYGON)))
> - ret = powernow_register_driver();
> + if ( cpufreq_controller == FREQCTL_xen )
> + {
> + switch ( boot_cpu_data.x86_vendor )
> + {
> + case X86_VENDOR_INTEL:
> + ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> + break;
I've been wondering why we continue to tie this to Intel. I don't
think there's much Intel specific in the ACPI driver, so I wonder
whether this shouldn't use "default:" instead. But I can agree
that's likely better to be done in a separate change.
> @@ -353,25 +349,13 @@ static const struct cpufreq_driver __initconstrel
> powernow_cpufreq_driver = {
> .update = powernow_cpufreq_update
> };
>
> -unsigned int __init powernow_register_driver()
> +unsigned int __init powernow_register_driver(void)
> {
> - unsigned int i, ret = 0;
> + if ( !(boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
> + return -ENODEV;
Ideally with this dropped (and of course with the issue pointed
out by Roger taken care of)
Acked-by: Jan Beulich <jbeul...@suse.com>
Jan
