[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 04/11] x86/intel_pstate: relocate the driver register function
On 23/06/2015 16:11, Jan Beulich wrote: > >>> On 23.06.15 at 10:01, <wei.w.wang@xxxxxxxxx> wrote: > > 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? > > If this distinction is being made clear by comments accompanying the > definitions of the target and setpolicy fields, I'm fine with keeping it that > way. > Without making this explicit it would continue to look arbitrary (and prone to > break, should another driver elect to also implement the setpolicy hook [for > whatever purpose]). OK, I will add some explanation to the related commit message. Best, Wei _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |