On 10/08/2012 04:26 AM, Joseph Lo wrote:
> The cpuidle LP2 is a power gating idle mode. It support power gating
> vdd_cpu rail after all cpu cores in LP2. For Tegra30, the CPU0 must
> be last one to go into LP2. We need to take care and make sure whole
> secondary CPUs were in LP2 by checking CPU and power gate status.
> After that, the CPU0 can go into LP2 safely. Then power gating the
> CPU rail.

> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c 
> b/arch/arm/mach-tegra/cpuidle-tegra30.c

> +static bool tegra30_idle_enter_lp2_cpu_0(struct cpuidle_device *dev,
> +                                      struct cpuidle_driver *drv,
> +                                      int index)
> +{
> +     struct cpuidle_state *state = &drv->states[index];
> +     u32 cpu_on_time = state->exit_latency;
> +     u32 cpu_off_time = state->target_residency - state->exit_latency;
> +
> +     if (num_online_cpus() > 1 && !tegra_cpu_rail_off_ready()) {

Should that be || not &&?

Isn't the "num_online_cpus() > 1" condition effectively checked at the
call site, i.e. in tegra30_idle_lp2() below via the if (last_cpu) check?

> @@ -85,16 +108,22 @@ static int __cpuinit tegra30_idle_lp2(struct 
> cpuidle_device *dev,
>                                     int index)
>  {
>       bool entered_lp2 = false;
> +     bool last_cpu;
>  
>       local_fiq_disable();
>  
> +     last_cpu = tegra_set_cpu_in_lp2(dev->cpu);
> +     if (dev->cpu == 0) {
> +             if (last_cpu)
> +                     entered_lp2 = tegra30_idle_enter_lp2_cpu_0(dev, drv,
> +                                                                index);
> +             else
> +                     cpu_do_idle();
> +     } else {
>               entered_lp2 = tegra30_idle_enter_lp2_cpu_n(dev, drv, index);
> +     }

Hmm. That means that if the last CPU to enter LP2 is e.g. CPU1, then
even though all CPUs are now in LP2, the complex as a whole doesn't
enter LP2. Is there a way to make the cluster as a whole enter LP2 in
this case? Isn't that what coupled cpuidle is for?

> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c

> +static void set_power_timers(unsigned long us_on, unsigned long us_off)

> +     if (tegra_pclk == NULL) {
> +             tegra_pclk = clk_get_sys(NULL, "pclk");
> +             if (IS_ERR(tegra_pclk)) {
> +                     /*
> +                      * pclk not been init or not exist.
> +                      * Use sclk to take the place of it.
> +                      * The default setting was pclk=sclk.
> +                      */
> +                     tegra_pclk = clk_get_sys(NULL, "sclk");
> +             }
> +     }

That's a little odd. Surely the HW has pclk or it doesn't? Why use
different clocks at different times for what is apparently the same thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to