> -----Original Message----- > From: Daniel Lezcano [mailto:daniel.lezc...@linaro.org] > Sent: Saturday, March 09, 2013 11:00 AM > To: Liu, Chuansheng > Cc: l...@kernel.org; Brown, Len; linux...@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....@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
