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

["Wang, Wei W" <wei.w.wang@xxxxxxxxx>]

On 03/06/2015 19:51, Jan Beulich wrote
> >>> On 03.06.15 at 10:07, <wei.w.wang@xxxxxxxxx> wrote:
> > On 26/05/2015 22:02, Jan Beulich wrote
> >> >>> On 13.05.16 at 09:51, <wei.w.wang@xxxxxxxxx> wrote:
> >> > --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> >> > +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> >> > --- a/xen/arch/x86/acpi/cpufreq/intel_pstate.c
> >> > +++ b/xen/arch/x86/acpi/cpufreq/intel_pstate.c
> >> > @@ -766,6 +766,8 @@ static struct cpufreq_driver intel_pstate_driver =
> {
> >> >      .name         = "intel_pstate",
> >> >  };
> >> >
> >> > +int __initdata load_intel_pstate = 0;
> >>
> >> static bool_t
> >
> > I think we cannot use "static" here, since load_intel_pstate is also
> > used in cpufreq.c to select which driver to load.
> 
> Iirc I had also requested to deal with that (as it looks pretty hackish right 
> now).

I'm not sure about this. Can you please elaborate it - why it looks hackish by 
doing so? 
What is your preferred way to do it? Thanks.

The following is your another comment on "load_intel_pstate":

>> @@ -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.

Best,
Wei

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