| Message ID | 1362755074.31506.45.camel@cliu38-desktop-build (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 03/08/2013 04:04 PM, Chuansheng Liu wrote: > > In function intel_idle_cpu_init() and intel_idle_cpuidle_driver_init(), > they are having the same for(;;) loop. > > Here in intel_idle_cpu_init(), the dev->state_count can be assigned by > drv->state_count directly. > > Signed-off-by: liu chuansheng <chuansheng.liu@intel.com> > --- > drivers/idle/intel_idle.c | 30 ++---------------------------- > 1 files changed, 2 insertions(+), 28 deletions(-) > > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > index 17c9cf9..503b401 100644 > --- a/drivers/idle/intel_idle.c > +++ b/drivers/idle/intel_idle.c > @@ -599,38 +599,12 @@ static int intel_idle_cpuidle_driver_init(void) > */ > static int intel_idle_cpu_init(int cpu) > { > - int cstate; > struct cpuidle_device *dev; > + struct cpuidle_driver *drv = &intel_idle_driver; > > dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu); > > - dev->state_count = 1; > - > - for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) { > - int num_substates, mwait_hint, mwait_cstate, mwait_substate; > - > - if (cpuidle_state_table[cstate].enter == NULL) > - break; > - > - if (cstate + 1 > max_cstate) { > - printk(PREFIX "max_cstate %d reached\n", max_cstate); > - break; > - } > - > - mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags); > - mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint); > - mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint); > - > - /* does the state exist in CPUID.MWAIT? */ > - num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4)) > - & MWAIT_SUBSTATE_MASK; > - > - /* if sub-state in table is not enumerated by CPUID */ > - if ((mwait_substate + 1) > num_substates) > - continue; > - > - dev->state_count += 1; > - } > + dev->state_count = drv->state_count; The cpuidle_register_device function already does this initialization. Probably you can get rid of this initialization and certainly factor out a bit the code in this case.
> -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org] > Sent: Saturday, March 09, 2013 11:00 AM > To: Liu, Chuansheng > Cc: lenb@kernel.org; Brown, Len; linux-pm@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/3] intel_idle: Removing the redundant calculating for > dev->state_count > > On 03/08/2013 04:04 PM, Chuansheng Liu wrote: > > > > In function intel_idle_cpu_init() and intel_idle_cpuidle_driver_init(), > > they are having the same for(;;) loop. > > > > Here in intel_idle_cpu_init(), the dev->state_count can be assigned by > > drv->state_count directly. > > > > Signed-off-by: liu chuansheng <chuansheng.liu@intel.com> > > --- > > drivers/idle/intel_idle.c | 30 ++---------------------------- > > 1 files changed, 2 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > > index 17c9cf9..503b401 100644 > > --- a/drivers/idle/intel_idle.c > > +++ b/drivers/idle/intel_idle.c > > @@ -599,38 +599,12 @@ static int intel_idle_cpuidle_driver_init(void) > > */ > > static int intel_idle_cpu_init(int cpu) > > { > > - int cstate; > > struct cpuidle_device *dev; > > + struct cpuidle_driver *drv = &intel_idle_driver; > > > > dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu); > > > > - dev->state_count = 1; > > - > > - for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) { > > - int num_substates, mwait_hint, mwait_cstate, mwait_substate; > > - > > - if (cpuidle_state_table[cstate].enter == NULL) > > - break; > > - > > - if (cstate + 1 > max_cstate) { > > - printk(PREFIX "max_cstate %d reached\n", max_cstate); > > - break; > > - } > > - > > - mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags); > > - mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint); > > - mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint); > > - > > - /* does the state exist in CPUID.MWAIT? */ > > - num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4)) > > - & MWAIT_SUBSTATE_MASK; > > - > > - /* if sub-state in table is not enumerated by CPUID */ > > - if ((mwait_substate + 1) > num_substates) > > - continue; > > - > > - dev->state_count += 1; > > - } > > + dev->state_count = drv->state_count; > > The cpuidle_register_device function already does this initialization. Thanks your remind, will send patch V2 to remove it. > > Probably you can get rid of this initialization and certainly factor out > a bit the code in this case. > > > -- > <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 17c9cf9..503b401 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -599,38 +599,12 @@ static int intel_idle_cpuidle_driver_init(void) */ static int intel_idle_cpu_init(int cpu) { - int cstate; struct cpuidle_device *dev; + struct cpuidle_driver *drv = &intel_idle_driver; dev = per_cpu_ptr(intel_idle_cpuidle_devices, cpu); - dev->state_count = 1; - - for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) { - int num_substates, mwait_hint, mwait_cstate, mwait_substate; - - if (cpuidle_state_table[cstate].enter == NULL) - break; - - if (cstate + 1 > max_cstate) { - printk(PREFIX "max_cstate %d reached\n", max_cstate); - break; - } - - mwait_hint = flg2MWAIT(cpuidle_state_table[cstate].flags); - mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint); - mwait_substate = MWAIT_HINT2SUBSTATE(mwait_hint); - - /* does the state exist in CPUID.MWAIT? */ - num_substates = (mwait_substates >> ((mwait_cstate + 1) * 4)) - & MWAIT_SUBSTATE_MASK; - - /* if sub-state in table is not enumerated by CPUID */ - if ((mwait_substate + 1) > num_substates) - continue; - - dev->state_count += 1; - } + dev->state_count = drv->state_count; dev->cpu = cpu;
In function intel_idle_cpu_init() and intel_idle_cpuidle_driver_init(), they are having the same for(;;) loop. Here in intel_idle_cpu_init(), the dev->state_count can be assigned by drv->state_count directly. Signed-off-by: liu chuansheng <chuansheng.liu@intel.com> --- drivers/idle/intel_idle.c | 30 ++---------------------------- 1 files changed, 2 insertions(+), 28 deletions(-)