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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
