From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0
Date: Tue, 09 Oct 2012 16:49:50 -0600 [thread overview]
Message-ID: <5074AA0E.2080508@wwwdotorg.org> (raw)
In-Reply-To: <1349691981-31038-8-git-send-email-josephl@nvidia.com>
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?
next prev parent reply other threads:[~2012-10-09 22:49 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-08 10:26 [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Joseph Lo
2012-10-08 10:26 ` [PATCH 1/7] ARM: tegra: cpuidle: separate cpuidle driver for different chips Joseph Lo
2012-10-09 22:22 ` Stephen Warren
2012-10-11 6:42 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 2/7] ARM: tegra: cpuidle: add LP2 resume function Joseph Lo
2012-10-09 22:29 ` Stephen Warren
2012-10-11 7:08 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs Joseph Lo
2012-10-08 16:35 ` Lorenzo Pieralisi
2012-10-09 4:13 ` Joseph Lo
2012-10-09 8:38 ` Lorenzo Pieralisi
2012-10-09 9:18 ` Joseph Lo
2012-10-09 9:42 ` Lorenzo Pieralisi
2012-10-09 22:38 ` Stephen Warren
2012-10-11 9:15 ` Joseph Lo
2012-10-11 16:24 ` Stephen Warren
2012-10-12 3:21 ` Joseph Lo
[not found] ` <87sj8vr517.fsf@amiettinen-lnx.nvidia.com>
2012-10-30 22:27 ` Stephen Warren
2012-10-31 1:26 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 4/7] ARM: tegra30: common: enable csite clock Joseph Lo
2012-10-09 22:38 ` Stephen Warren
2012-10-11 10:28 ` Joseph Lo
2012-10-08 10:26 ` [PATCH 5/7] ARM: tegra30: clocks: add CPU low-power function into tegra_cpu_car_ops Joseph Lo
2012-10-08 10:26 ` [PATCH 6/7] ARM: tegra30: flowctrl: add cpu_suspend_exter/exit function Joseph Lo
2012-10-08 10:26 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Joseph Lo
2012-10-09 22:49 ` Stephen Warren [this message]
2012-10-11 11:24 ` Joseph Lo
2012-10-11 16:37 ` Stephen Warren
2012-10-11 16:48 ` Colin Cross
2012-10-12 7:11 ` Joseph Lo
2012-10-12 7:40 ` Joseph Lo
2012-10-12 7:54 ` Shawn Guo
2012-10-12 8:24 ` Joseph Lo
2012-10-12 8:30 ` Shawn Guo
2012-10-12 20:50 ` Colin Cross
2012-10-15 16:28 ` Use coupled cpuidle on imx6q Shawn Guo
2012-10-15 22:58 ` Colin Cross
2012-10-12 20:46 ` [PATCH 7/7] ARM: tegra30: cpuidle: add LP2 driver for CPU0 Colin Cross
2012-10-12 7:07 ` Joseph Lo
2012-10-12 21:04 ` Stephen Warren
2012-10-15 7:56 ` Joseph Lo
2012-10-15 15:59 ` Stephen Warren
2012-10-15 22:33 ` Colin Cross
2012-10-16 8:13 ` Peter De Schrijver
2012-10-16 8:06 ` Peter De Schrijver
2012-10-16 17:03 ` Stephen Warren
2012-10-18 9:24 ` Peter De Schrijver
2012-10-25 14:08 ` Peter De Schrijver
2012-10-09 22:26 ` [PATCH 0/7] ARM: tegra30: cpuidle: add LP2 support Stephen Warren
2012-10-11 6:39 ` Joseph Lo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5074AA0E.2080508@wwwdotorg.org \
--to=swarren@wwwdotorg$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox