From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/7] ARM: tegra30: cpuidle: add LP2 driver for secondary CPUs
Date: Thu, 11 Oct 2012 10:24:14 -0600 [thread overview]
Message-ID: <5076F2AE.6030509@wwwdotorg.org> (raw)
In-Reply-To: <1349946918.19413.130.camel@jlo-ubuntu-64.nvidia.com>
On 10/11/2012 03:15 AM, Joseph Lo wrote:
> On Wed, 2012-10-10 at 06:38 +0800, Stephen Warren wrote:
>> On 10/08/2012 04:26 AM, Joseph Lo wrote:
>>> This supports power-gated (LP2) idle on secondary CPUs for Tegra30.
>>> The secondary CPUs can go into LP2 state independently. When CPU goes
>>> into LP2 state, it saves it's state and puts itself to flow controlled
>>> WFI state. After that, it will been power gated.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
>>
>>> static struct cpuidle_driver tegra_idle_driver = {
>>> .name = "tegra_idle",
>>> .owner = THIS_MODULE,
>>> .en_core_tk_irqen = 1,
>>> - .state_count = 1,
>>> + .state_count = 2,
>>
>> Doesn't that assignment need to be ifdef'd just like the array entry
>> setup below:
>>
>>> .states = {
>>> [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
>>> +#ifdef CONFIG_PM_SLEEP
>>> + [1] = {
>>> + .enter = tegra30_idle_lp2,
>>> + .exit_latency = 2000,
>>> + .target_residency = 2200,
>>> + .power_usage = 0,
>>> + .flags = CPUIDLE_FLAG_TIME_VALID,
>>> + .name = "LP2",
>>> + .desc = "CPU power-gate",
>>> + },
>>> +#endif
>>> },
>>> };
>>
>>> @@ -41,6 +114,10 @@ int __init tegra30_cpuidle_init(void)
>>> struct cpuidle_device *dev;
>>> struct cpuidle_driver *drv = &tegra_idle_driver;
>>>
>>> +#ifndef CONFIG_PM_SLEEP
>>> + drv->state_count = 1; /* support clockgating only */
>>> +#endif
>>
>> Oh, I see it's done here. Just fixing the static initialization seems a
>> lot simpler?
>>
> OK. Will do.
>
>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> +void __cpuinit tegra_clear_cpu_in_lp2(int cpu)
>>> +{
>>> + spin_lock(&tegra_lp2_lock);
>>> + BUG_ON(!cpumask_test_cpu(cpu, &tegra_in_lp2));
>>> + cpumask_clear_cpu(cpu, &tegra_in_lp2);
>>> +
>>> + /*
>>> + * Update the IRAM copy used by the reset handler. The IRAM copy
>>> + * can't use used directly by cpumask_clear_cpu() because it uses
>>> + * LDREX/STREX which requires the addressed location to be inner
>>> + * cacheable and sharable which IRAM isn't.
>>> + */
>>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
>>> + dsb();
>>
>> Why not /just/ store the data in IRAM, and read/write directly to it,
>> rather than maintaining an SDRAM-based copy of it?
>>
>> Then, wouldn't the body of this function be simply:
>>
>> spin_lock();
>> BUG_ON(!(tegra_cpu_lp2_mask & BIT(cpu)));
>> tegra_cpu_lp2_mask |= BIT(cpu);
>> spin_unlock();
>>
>
> It may not simple like this. To maintain it identical to a cpumask. It
> may look likes below. Because I need to compare it with cpu_online_mask.
Oh, the comparison against cpu_online_mask() is what I was missing. I
guess that offline CPUs don't go into LP2, so you can't just check that
tegra_cpu_lp2_mask == (1 << num_cpus()) - 1.
One way to avoid that might be to maintain a cpu_in_lp2_count variable
alongside the mask, and simply compare that against num_online_cpus()
rather than comparing the two masks. At least that would avoid the
following line:
>>> + writel(tegra_in_lp2.bits[0], tegra_cpu_lp2_mask);
... making use of knowledge of the internal structure of the struct
cpumask type.
However, given the comparison requirement, either way is probably fine.
next prev parent reply other threads:[~2012-10-11 16:24 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 [this message]
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
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=5076F2AE.6030509@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