On 04/06/2015 09:18, Wei Wang wrote > On 03/06/2015 19:51, Jan Beulich wrote > > >>> On 03.06.15 at 10:07, <wei.w.w...@intel.com> wrote: > > > On 26/05/2015 22:02, Jan Beulich wrote > > >> >>> On 13.05.16 at 09:51, <wei.w.w...@intel.com> 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.
Missed one thing, it should be like this: if (load_intel_pstate) ret = intel_pstate_init(); if (ret || ! load_intel_pstate) ret = cpufreq_register_driver(&acpi_cpufreq_driver); Best, Wei _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel