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

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>>> "Wang, Wei W" <wei.w.wang@xxxxxxxxx> 06/04/15 3:15 AM >>>
>On 03/06/2015 19:51, Jan Beulich wrote
>> >>> On 03.06.15 at 10:07, <wei.w.wang@xxxxxxxxx> wrote:
>>> @@ -650,9 +650,12 @@ 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)) {
>> >+        if (load_intel_pstate)
>> >+            ret = intel_pstate_init();
>> >+        if (!load_intel_pstate)
>> >+            ret = cpufreq_register_driver(&acpi_cpufreq_driver);
>
>>I don't see why you need load_intel_pstate here: Simply call the original 
>>function whenever 
>>intel_pstate_init() returns an error.
>
>I plan to change it to:
>if (load_intel_pstate)
>ret = intel_pstate_init();
>if (ret)
>ret = cpufreq_register_driver(&acpi_cpufreq_driver);
>
>This allows the case that the machine supports the intel_pstate driver but 
>people
> just prefer to use the old driver for their own reasons.

But there's no point in using load_intel_pstate here - it should be a variable 
local to
that driver. Simply call the function unconditionally and check the variable 
first thing
inside the function.

Jan


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