From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Wang, Wei W" Subject: Re: [PATCH v3 04/11] x86/intel_pstate: relocate the driver register function Date: Tue, 23 Jun 2015 08:01:42 +0000 Message-ID: <286AC319A985734F985F78AFA26841F7996F3E@shsmsx102.ccr.corp.intel.com> References: <1434011230-17052-1-git-send-email-wei.w.wang@intel.com> <5582F2140200007800086ADF@mail.emea.novell.com> <286AC319A985734F985F78AFA26841F7996BB9@shsmsx102.ccr.corp.intel.com> <558927600200007800087EFA@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <558927600200007800087EFA@mail.emea.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: "andrew.cooper3@citrix.com" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 23/06/2015 15:31, Jan Beulich wrote: > >>> On 23.06.15 at 05:40, wrote: > > On 18/06/2015 22:30, Jan Beulich wrote: > >> >>> On 11.06.15 at 10:27, 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