Re: [Xen-devel] [PATCH v5 7/9] x86/intel_pstate: add a booting param to select the driver to load

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.09.15 at 04:32, <wei.w.wang@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> @@ -40,6 +40,7 @@
>  #include <asm/processor.h>
>  #include <asm/percpu.h>
>  #include <asm/cpufeature.h>
> +#include <asm/cpufreq.h>

I think to make clear why this include is needed here it would be better
if you added the declaration of intel_pstate_init() also in this patch
instead of in the previous one.

> @@ -647,9 +648,11 @@ 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_INTEL)) {
> +            ret = intel_pstate_init();
> +            if (ret)
> +                ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> +    } else if ((cpufreq_controller == FREQCTL_xen) &&
>          (boot_cpu_data.x86_vendor == X86_VENDOR_AMD))
>          ret = powernow_register_driver();

Since you're basically modifying the entire body of the function,
please gets its coding style corrected as you fiddle with it.

> --- a/xen/arch/x86/acpi/cpufreq/intel_pstate.c
> +++ b/xen/arch/x86/acpi/cpufreq/intel_pstate.c
> @@ -843,7 +843,11 @@ int __init intel_pstate_init(void)
>      int cpu, rc = 0;
>      const struct x86_cpu_id *id;
>      struct cpu_defaults *cpu_info;
> +    static bool_t __read_mostly load;

Why __read_mostly when the function is __init?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel