Re: [Xen-devel] [PATCH v3 04/11] x86/intel_pstate: relocate the driver register function

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

On 23/06/2015 15:31, Jan Beulich wrote:
> >>> On 23.06.15 at 05:40, <wei.w.wang@xxxxxxxxx> wrote:
> > On 18/06/2015 22:30, Jan Beulich wrote:
> >> >>> On 11.06.15 at 10:27, <wei.w.wang@xxxxxxxxx> wrote:
> >> > -static int __init cpufreq_presmp_init(void)
> >> > +int cpufreq_register_driver(struct cpufreq_driver *driver_data)
> >> >  {
> >> >      void *cpu = (void *)(long)smp_processor_id();
> >> >      cpu_callback(&cpu_nfb, CPU_ONLINE, cpu);
> >> > +    if (!driver_data || !driver_data->init
> >> > +        || !driver_data->verify || !driver_data->exit
> >> > +        || (!driver_data->target == !driver_data->setpolicy))
> >>
> >> Do you really want/need to enforce this policy (target set if and
> >> only if setpolicy is not set) here? And if that's to uniformly hold,
> >> the two could be put into a union...
> >
> > driver_data->target() is used by a driver which relies on the old
> > Governor framework.
> > driver_data->setpolicy() is used by a driver which implements its
> > internal governor.
> > So, the driver either uses the old Governor framework or has its own
> > private internal governor.
> > We shouldn't change to use union, because in many places, we
> > distinguish the two by checking if it's using "->target" or "->setpolicy".
> 
> The distinction between the two driver modes shouldn't be based on
> arbitrary accessors they may or may not implement. There should be a
> dedicated flag or alike.

This is not arbitrary - "->target()" is dedicated to the Governor framework, 
and "->setpolicy" is dedicated to the internal governor implementation. The 
Linux kernel also takes advantage of this method. I think we don't need to add 
another new functionally equivalent flag to do so.
Shall we keep using it?

Best,
Wei

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