[PATCH V2 3/3] ARM: tegra114: cpuidle: add powered-down state

[Joseph Lo]

On Thu, 2013-06-13 at 17:30 +0800, Daniel Lezcano wrote:
> On 06/13/2013 04:13 AM, Joseph Lo wrote:
> > On Thu, 2013-06-13 at 01:41 +0800, Stephen Warren wrote:
> >> On 06/04/2013 03:40 PM, Daniel Lezcano wrote:
> >>> On 06/04/2013 12:48 PM, Joseph Lo wrote:
> >>>> This supports CPU core power down on each CPU when CPU idle. When CPU go
> >>>> into this state, it saves it's context and needs a proper configuration
> >>>> in flow controller to power gate the CPU when CPU runs into WFI
> >>>> instruction. And the CPU also needs to set the IRQ as CPU power down idle
> >>>> wake up event in flow controller.
> >>>>
> >>>> Signed-off-by: Joseph Lo <josephl at nvidia.com>
> >>>
> >>> I would like to understand why there is a WARN with the
> >>> CPUIDLE_FLAG_TIMER_STOP flag set before queuing this patch and ensure it
> >>> is not the tree hiding the forest.
> >>
> >> Joseph, are you planning to post an updated series or respond to resolve
> >> Daniel's question?
> > 
> > I need more time to investigate the detail about what caused the WARN
> > when apply the flag. And what's the difference if we didn't enable
> > CONFIG_CPU_IDLE_MULTIPLE_DRIVERS, then it only applies
> > CLOCK_EVT_NOTIFY_BROADCAST_ON on CPU0.
> 
> I think I got it for this one. It is a bug in the cpuidle driver's code.
> 
> cpuidle_register_driver does get_cpu => CPU0
> 
> Then __cpuidle_register_driver(drv, <CPU0>)
> 
>  => __cpuidle_driver_init(drv, <CPU0>)
> 
> Hence, the timer broadcast is initialized only for cpu0.
> 
> That should have been fixed with:
> 
OK, thanks.

> commit 82467a5a885ddd9f80309682159da8db510e7832
> Author: Daniel Lezcano <daniel.lezcano at linaro.org>
> Date:   Fri Jun 7 21:53:09 2013 +0000
> 
>     cpuidle: simplify multiple driver support
> 
>     Commit bf4d1b5 (cpuidle: support multiple drivers) introduced support
>     for using multiple cpuidle drivers at the same time.  It added a
>     couple of new APIs to register the driver per CPU, but that led to
>     some unnecessary code complexity related to the kernel config options
>     deciding whether or not the multiple driver support is enabled.  The
>     code has to work as it did before when the multiple driver support is
>     not enabled and the multiple driver support has to be compatible with
>     the previously existing API.
> 
>     Remove the new API, not used by any driver in the tree yet (but
>     needed for the HMP cpuidle drivers that will be submitted soon), and
>     add a new cpumask pointer to the cpuidle driver structure that will
>     point to the mask of CPUs handled by the given driver.  That will
>     allow the cpuidle_[un]register_driver() API to be used for the
>     multiple driver support along with the cpuidle_[un]register()
>     functions added recently.
> 
>     [rjw: Changelog]
>     Signed-off-by: Daniel Lezcano <daniel.lezcano at linaro.org>
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> 
> 
> > Why if I am moving the
> > clockevent_notify of CLOCK_EVT_NOTIFY_BROADCAST_EXIT before
> > "local_irq_enable" in the
> > "cpuidle_enter_state" (drivers/cpuidle/cpuidle.c), then the warning
> > message gone?
> 
> Is the warning coming immediately, at the first time
> CLOCK_EVT_NOTIFY_BROADCAST_EXIT is invoked or can it occur at different
> moment ?
> 
Not exactly at the 1st time, it may happen in the 3rd to 10th time. But
it only happens once. I mean I did check the
tick_broadcast_pending_mask, it only happens once the CPU didn't clear
the tick_broadcast_pending_mask after CLOCK_EVT_NOTIFY_BROADCAST_EXIT.
And cause the warning message. Then I didn't see it happen anymore.