Re: [PATCH] cpuidle : handle clockevent notify from the cpuidle framework

[Daniel Lezcano <daniel.lezcano@linaro.org>]

On 03/20/2013 10:21 PM, Thomas Gleixner wrote:
> On Wed, 20 Mar 2013, Daniel Lezcano wrote:
> 
>> When a cpu enters a deep idle state, the local timers are stopped and
>> the time framework falls back to the timer device used as a broadcast
>> timer.
>>
>> The different cpuidle drivers are calling clockevents_notify ENTER/EXIT
>> when the idle state stops the local timer.
>>
>> The proposed patch introduces a new flag CPUIDLE_FLAG_TIMER_STOP to let
> 
> Please stop using "proposed patch ...." wording in a patch
> description. The changelog you submit with your patch should be
> applicable w/o rewriting.

Oh, yes. Thanks for pointing this.

>> the cpuidle framework to call clockevents_notify instead of duplicating
>> again and again these lines in all the cpuidle drivers.
> 
> That's a good enough reason, really. So that paragraph should be
> something like:
> 
> "Add a new flag CPUIDLE_FLAG_TIMER_STOP which can be set by cpuidle
>  drivers. If the flag is set the cpuidle core code takes care of the
>  notification on behalf of the driver to avoid pointless code
>  duplication."
> 
> Ideally you would follow up with two or three drivers converted to
> that new infrastructure instead of sending that patch standalone. That
> way reviewers can really see the benefit in terms of reduced code
> duplication.

Thanks for your review. I will resend this patch with a better log and a
couple of patches on top using this flag.

 -- Daniel

-- 
 <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