[PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Joseph Lo]

This adds a "powered-down" state in cpuidle for Tegra20. It requires power
gating both CPU cores. When the CPU1 requests to enter "powered-down"
state, it saves its own state and then enters WFI. When the CPU0 requests
the same state, it attempts to put the CPU1 into reset to prevent it from
waking up. Then power down both CPUs together and turn off the CPU rail.

If the CPU1 be woken up before CPU0 entering powered-down state, then it
needs to restore it's CPU state and waits for next chance.

V3:
* sqash the last 2 patches in previors version to support coupled cpuidle
  directly

V2:
* add a new patch for checking if there is any pending SGI
* if there is a SGI pending for the CPU, then we will abort the
  "powered-down" idle for the both CPUs that already in coupled state

Verified on Seaboard(Tegra20) and Cardhu(Tegra30).

Joseph Lo (5):
  ARM: tegra: add pending SGI checking API
  ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  ARM: tegra20: clocks: add CPU low-power function into
    tegra_cpu_car_ops
  ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit
  ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |  201 ++++++++++++++++++++++++++++++++-
 arch/arm/mach-tegra/flowctrl.c        |   38 ++++++-
 arch/arm/mach-tegra/flowctrl.h        |    4 +
 arch/arm/mach-tegra/irq.c             |   15 +++
 arch/arm/mach-tegra/irq.h             |   22 ++++
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  200 ++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++
 arch/arm/mach-tegra/sleep.h           |   26 +++++
 arch/arm/mach-tegra/tegra20_clocks.c  |   99 ++++++++++++++++
 11 files changed, 617 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h

[PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Joseph Lo]

This adds a "powered-down" state in cpuidle for Tegra20. It requires power
gating both CPU cores. When the CPU1 requests to enter "powered-down"
state, it saves its own state and then enters WFI. When the CPU0 requests
the same state, it attempts to put the CPU1 into reset to prevent it from
waking up. Then power down both CPUs together and turn off the CPU rail.

If the CPU1 be woken up before CPU0 entering powered-down state, then it
needs to restore it's CPU state and waits for next chance.

V3:
* sqash the last 2 patches in previors version to support coupled cpuidle
  directly

V2:
* add a new patch for checking if there is any pending SGI
* if there is a SGI pending for the CPU, then we will abort the
  "powered-down" idle for the both CPUs that already in coupled state

Verified on Seaboard(Tegra20) and Cardhu(Tegra30).

Joseph Lo (5):
  ARM: tegra: add pending SGI checking API
  ARM: tegra20: cpuidle: add powered-down state for secondary CPU
  ARM: tegra20: clocks: add CPU low-power function into
    tegra_cpu_car_ops
  ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit
  ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |  201 ++++++++++++++++++++++++++++++++-
 arch/arm/mach-tegra/flowctrl.c        |   38 ++++++-
 arch/arm/mach-tegra/flowctrl.h        |    4 +
 arch/arm/mach-tegra/irq.c             |   15 +++
 arch/arm/mach-tegra/irq.h             |   22 ++++
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  200 ++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++
 arch/arm/mach-tegra/sleep.h           |   26 +++++
 arch/arm/mach-tegra/tegra20_clocks.c  |   99 ++++++++++++++++
 11 files changed, 617 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
include the power of GIC. That caused the SGI (Software Generated
Interrupt) been lost. Because the SGI can't wake up the CPU that in
the "powered-down" CPU idle mode. We need to check if there is any
pending SGI when go into "powered-down" CPU idle mode. This is important
especially when applying the coupled cpuidle framework into "power-down"
cpuidle dirver. Because the coupled cpuidle framework may have the
chance that misses IPI_SINGLE_FUNC handling sometimes.

For the PPI or SPI, something like the legacy peripheral interrupt. It
still can be maintained by Tegra legacy interrupt controller. If there
is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
CPU can be woken up immediately. So we don't need to take care the same
situation for PPI or SPI.

Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
V3:
* move the static mapping of GIC addr into tegra_pending_sgi
V2:
* new in V2
---
 arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
 arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h

diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index b7886f1..c9976e3 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -45,6 +45,8 @@
 
 #define FIRST_LEGACY_IRQ 32
 
+#define SGI_MASK 0xFFFF
+
 static int num_ictlrs;
 
 static void __iomem *ictlr_reg_base[] = {
@@ -55,6 +57,19 @@ static void __iomem *ictlr_reg_base[] = {
 	IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
 };
 
+bool tegra_pending_sgi(void)
+{
+	u32 pending_set;
+	void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
+
+	pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
+
+	if (pending_set & SGI_MASK)
+		return true;
+
+	return false;
+}
+
 static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
 {
 	void __iomem *base;
diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
new file mode 100644
index 0000000..5142649
--- /dev/null
+++ b/arch/arm/mach-tegra/irq.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __TEGRA_IRQ_H
+#define __TEGRA_IRQ_H
+
+bool tegra_pending_sgi(void);
+
+#endif
-- 
1.7.0.4

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
include the power of GIC. That caused the SGI (Software Generated
Interrupt) been lost. Because the SGI can't wake up the CPU that in
the "powered-down" CPU idle mode. We need to check if there is any
pending SGI when go into "powered-down" CPU idle mode. This is important
especially when applying the coupled cpuidle framework into "power-down"
cpuidle dirver. Because the coupled cpuidle framework may have the
chance that misses IPI_SINGLE_FUNC handling sometimes.

For the PPI or SPI, something like the legacy peripheral interrupt. It
still can be maintained by Tegra legacy interrupt controller. If there
is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
CPU can be woken up immediately. So we don't need to take care the same
situation for PPI or SPI.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* move the static mapping of GIC addr into tegra_pending_sgi
V2:
* new in V2
---
 arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
 arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-tegra/irq.h

diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
index b7886f1..c9976e3 100644
--- a/arch/arm/mach-tegra/irq.c
+++ b/arch/arm/mach-tegra/irq.c
@@ -45,6 +45,8 @@
 
 #define FIRST_LEGACY_IRQ 32
 
+#define SGI_MASK 0xFFFF
+
 static int num_ictlrs;
 
 static void __iomem *ictlr_reg_base[] = {
@@ -55,6 +57,19 @@ static void __iomem *ictlr_reg_base[] = {
 	IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
 };
 
+bool tegra_pending_sgi(void)
+{
+	u32 pending_set;
+	void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
+
+	pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
+
+	if (pending_set & SGI_MASK)
+		return true;
+
+	return false;
+}
+
 static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
 {
 	void __iomem *base;
diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
new file mode 100644
index 0000000..5142649
--- /dev/null
+++ b/arch/arm/mach-tegra/irq.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __TEGRA_IRQ_H
+#define __TEGRA_IRQ_H
+
+bool tegra_pending_sgi(void);
+
+#endif
-- 
1.7.0.4

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

The powered-down state of Tegra20 requires power gating both CPU cores.
When the secondary CPU requests to enter powered-down state, it saves
its own contexts and then enters WFI. The Tegra20 had a limition to
power down both CPU cores. The secondary CPU must waits for CPU0 in
powered-down state too. If the secondary CPU be woken up before CPU0
entering powered-down state, then it needs to restore its CPU states
and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code
which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Gary King <gking-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
V3:
* dynamic checking of the number of the state counts
* fix the code sequence for aborting cpu_suspend in
  tegra20_sleep_cpu_secondary_finish
V2:
* no change
---
 arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.h           |   23 +++++
 4 files changed, 261 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index d32e8b0..716aef3 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -22,28 +22,112 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/clockchips.h>
 
 #include <asm/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/smp_plat.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra20_idle_lp2(struct cpuidle_device *dev,
+			    struct cpuidle_driver *drv,
+			    int index);
+#endif
+
+static struct cpuidle_state tegra_idle_states[] = {
+	[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+	[1] = {
+		.enter			= tegra20_idle_lp2,
+		.exit_latency		= 5000,
+		.target_residency	= 10000,
+		.power_usage		= 0,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "powered-down",
+		.desc			= "CPU power gated",
+	},
+#endif
+};
 
 static struct cpuidle_driver tegra_idle_driver = {
 	.name = "tegra_idle",
 	.owner = THIS_MODULE,
 	.en_core_tk_irqen = 1,
-	.state_count = 1,
-	.states = {
-		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
-	},
 };
 
 static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
+#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_SMP
+static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+					 struct cpuidle_driver *drv,
+					 int index)
+{
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
+
+	tegra20_cpu_clear_resettable();
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	return true;
+}
+#else
+static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+						struct cpuidle_driver *drv,
+						int index)
+{
+	return true;
+}
+#endif
+
+static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
+				      struct cpuidle_driver *drv,
+				      int index)
+{
+	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+	bool entered_lp2 = false;
+
+	local_fiq_disable();
+
+	tegra_set_cpu_in_lp2(cpu);
+	cpu_pm_enter();
+
+	if (cpu == 0)
+		cpu_do_idle();
+	else
+		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
+
+	cpu_pm_exit();
+	tegra_clear_cpu_in_lp2(cpu);
+
+	local_fiq_enable();
+
+	smp_rmb();
+
+	return entered_lp2 ? index : 0;
+}
+#endif
+
 int __init tegra20_cpuidle_init(void)
 {
-	int ret;
+	int ret, i;
 	unsigned int cpu;
 	struct cpuidle_device *dev;
 	struct cpuidle_driver *drv = &tegra_idle_driver;
 
+	drv->state_count = sizeof(tegra_idle_states) /
+			   sizeof(struct cpuidle_state);
+	for (i = 0; i < drv->state_count; i++)
+		memcpy(&drv->states[i], &tegra_idle_states[i],
+		       sizeof(struct cpuidle_state));
+
 	ret = cpuidle_register_driver(&tegra_idle_driver);
 	if (ret) {
 		pr_err("CPUidle driver registration failed\n");
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 1b11707..db72ea9 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
 
 	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
 		last_cpu = true;
+	else if (phy_cpu_id == 1)
+		tegra20_cpu_set_resettable_soon();
 
 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index 72ce709..d04693b 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -21,6 +21,8 @@
 #include <linux/linkage.h>
 
 #include <asm/assembler.h>
+#include <asm/proc-fns.h>
+#include <asm/cp15.h>
 
 #include "sleep.h"
 #include "flowctrl.h"
@@ -78,3 +80,148 @@ ENTRY(tegra20_cpu_shutdown)
 	mov	pc, lr
 ENDPROC(tegra20_cpu_shutdown)
 #endif
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * tegra_pen_lock
+ *
+ * spinlock implementation with no atomic test-and-set and no coherence
+ * using Peterson's algorithm on strongly-ordered registers
+ * used to synchronize a cpu waking up from wfi with entering lp2 on idle
+ *
+ * The reference link of Peterson's algorithm:
+ * http://en.wikipedia.org/wiki/Peterson's_algorithm
+ *
+ * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)
+ * on cpu 0:
+ * r2 = flag[0] (in SCRATCH38)
+ * r3 = flag[1] (in SCRATCH39)
+ * on cpu1:
+ * r2 = flag[1] (in SCRATCH39)
+ * r3 = flag[0] (in SCRATCH38)
+ *
+ * must be called with MMU on
+ * corrupts r0-r3, r12
+ */
+ENTRY(tegra_pen_lock)
+	mov32	r3, TEGRA_PMC_VIRT
+	cpu_id	r0
+	add	r1, r3, #PMC_SCRATCH37
+	cmp	r0, #0
+	addeq	r2, r3, #PMC_SCRATCH38
+	addeq	r3, r3, #PMC_SCRATCH39
+	addne	r2, r3, #PMC_SCRATCH39
+	addne	r3, r3, #PMC_SCRATCH38
+
+	mov	r12, #1
+	str	r12, [r2]		@ flag[cpu] = 1
+	dsb
+	str	r12, [r1]		@ !turn = cpu
+1:	dsb
+	ldr	r12, [r3]
+	cmp	r12, #1			@ flag[!cpu] == 1?
+	ldreq	r12, [r1]
+	cmpeq	r12, r0			@ !turn == cpu?
+	beq	1b			@ while !turn == cpu && flag[!cpu] == 1
+
+	mov	pc, lr			@ locked
+ENDPROC(tegra_pen_lock)
+
+ENTRY(tegra_pen_unlock)
+	dsb
+	mov32	r3, TEGRA_PMC_VIRT
+	cpu_id	r0
+	cmp	r0, #0
+	addeq	r2, r3, #PMC_SCRATCH38
+	addne	r2, r3, #PMC_SCRATCH39
+	mov	r12, #0
+	str	r12, [r2]
+	mov     pc, lr
+ENDPROC(tegra_pen_unlock)
+
+/*
+ * tegra20_cpu_clear_resettable(void)
+ *
+ * Called to clear the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_clear_resettable)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_NOT_RESETTABLE
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_clear_resettable)
+
+/*
+ * tegra20_cpu_set_resettable_soon(void)
+ *
+ * Called to set the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_set_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE_SOON
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_set_resettable_soon)
+
+/*
+ * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
+ *
+ * Enters WFI on secondary CPU by exiting coherency.
+ */
+ENTRY(tegra20_sleep_cpu_secondary_finish)
+	stmfd	sp!, {r4-r11, lr}
+
+	mrc	p15, 0, r11, c1, c0, 1  @ save actlr before exiting coherency
+
+	/* Flush and disable the L1 data cache */
+	bl	tegra_disable_clean_inv_dcache
+
+	mov32	r0, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r3, #CPU_RESETTABLE
+	str	r3, [r0]
+
+	bl	cpu_do_idle
+
+	/*
+	 * cpu may be reset while in wfi, which will return through
+	 * tegra_resume to cpu_resume
+	 * or interrupt may wake wfi, which will return here
+	 * cpu state is unchanged - MMU is on, cache is on, coherency
+	 * is off, and the data cache is off
+	 *
+	 * r11 contains the original actlr
+	 */
+
+	bl	tegra_pen_lock
+
+	mov32	r3, TEGRA_PMC_VIRT
+	add	r0, r3, #PMC_SCRATCH41
+	mov	r3, #CPU_NOT_RESETTABLE
+	str	r3, [r0]
+
+	bl	tegra_pen_unlock
+
+	/* Re-enable the data cache */
+	mrc	p15, 0, r10, c1, c0, 0
+	orr	r10, r10, #CR_C
+	mcr	p15, 0, r10, c1, c0, 0
+	isb
+
+	mcr	p15, 0, r11, c1, c0, 1	@ reenable coherency
+
+	/* Invalidate the TLBs & BTAC */
+	mov	r1, #0
+	mcr	p15, 0, r1, c8, c3, 0	@ invalidate shared TLBs
+	mcr	p15, 0, r1, c7, c1, 6	@ invalidate shared BTAC
+	dsb
+	isb
+
+	/* the cpu was running with coherency disabled,
+	 * caches may be out of date */
+	bl	v7_flush_kern_cache_louis
+
+	ldmfd	sp!, {r4 - r11, pc}
+ENDPROC(tegra20_sleep_cpu_secondary_finish)
+#endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 9821ee7..a02f71a 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -25,6 +25,19 @@
 					+ IO_PPSB_VIRT)
 #define TEGRA_CLK_RESET_VIRT (TEGRA_CLK_RESET_BASE - IO_PPSB_PHYS \
 					+ IO_PPSB_VIRT)
+#define TEGRA_PMC_VIRT	(TEGRA_PMC_BASE - IO_APB_PHYS + IO_APB_VIRT)
+
+/* PMC_SCRATCH37-39 and 41 are used for tegra_pen_lock and idle */
+#define PMC_SCRATCH37	0x130
+#define PMC_SCRATCH38	0x134
+#define PMC_SCRATCH39	0x138
+#define PMC_SCRATCH41	0x140
+
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+#define CPU_RESETTABLE		2
+#define CPU_RESETTABLE_SOON	1
+#define CPU_NOT_RESETTABLE	0
+#endif
 
 #ifdef __ASSEMBLY__
 /* returns the offset of the flow controller halt register for a cpu */
@@ -104,6 +117,8 @@ exit_l2_resume:
 .endm
 #endif /* CONFIG_CACHE_L2X0 */
 #else
+void tegra_pen_lock(void);
+void tegra_pen_unlock(void);
 void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
 
@@ -115,6 +130,14 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_clear_resettable(void);
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+void tegra20_cpu_set_resettable_soon(void);
+#else
+static inline void tegra20_cpu_set_resettable_soon(void) {}
+#endif
+
+int tegra20_sleep_cpu_secondary_finish(unsigned long);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
-- 
1.7.0.4

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

The powered-down state of Tegra20 requires power gating both CPU cores.
When the secondary CPU requests to enter powered-down state, it saves
its own contexts and then enters WFI. The Tegra20 had a limition to
power down both CPU cores. The secondary CPU must waits for CPU0 in
powered-down state too. If the secondary CPU be woken up before CPU0
entering powered-down state, then it needs to restore its CPU states
and waits for next chance.

Be aware of that, you may see the legacy power state "LP2" in the code
which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <ccross@android.com>
Gary King <gking@nvidia.com>

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* dynamic checking of the number of the state counts
* fix the code sequence for aborting cpu_suspend in
  tegra20_sleep_cpu_secondary_finish
V2:
* no change
---
 arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
 arch/arm/mach-tegra/pm.c              |    2 +
 arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/sleep.h           |   23 +++++
 4 files changed, 261 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index d32e8b0..716aef3 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -22,28 +22,112 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/clockchips.h>
 
 #include <asm/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <asm/suspend.h>
+#include <asm/smp_plat.h>
+
+#include "pm.h"
+#include "sleep.h"
+
+#ifdef CONFIG_PM_SLEEP
+static int tegra20_idle_lp2(struct cpuidle_device *dev,
+			    struct cpuidle_driver *drv,
+			    int index);
+#endif
+
+static struct cpuidle_state tegra_idle_states[] = {
+	[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
+#ifdef CONFIG_PM_SLEEP
+	[1] = {
+		.enter			= tegra20_idle_lp2,
+		.exit_latency		= 5000,
+		.target_residency	= 10000,
+		.power_usage		= 0,
+		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.name			= "powered-down",
+		.desc			= "CPU power gated",
+	},
+#endif
+};
 
 static struct cpuidle_driver tegra_idle_driver = {
 	.name = "tegra_idle",
 	.owner = THIS_MODULE,
 	.en_core_tk_irqen = 1,
-	.state_count = 1,
-	.states = {
-		[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
-	},
 };
 
 static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
+#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_SMP
+static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+					 struct cpuidle_driver *drv,
+					 int index)
+{
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
+
+	tegra20_cpu_clear_resettable();
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	return true;
+}
+#else
+static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
+						struct cpuidle_driver *drv,
+						int index)
+{
+	return true;
+}
+#endif
+
+static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
+				      struct cpuidle_driver *drv,
+				      int index)
+{
+	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
+	bool entered_lp2 = false;
+
+	local_fiq_disable();
+
+	tegra_set_cpu_in_lp2(cpu);
+	cpu_pm_enter();
+
+	if (cpu == 0)
+		cpu_do_idle();
+	else
+		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
+
+	cpu_pm_exit();
+	tegra_clear_cpu_in_lp2(cpu);
+
+	local_fiq_enable();
+
+	smp_rmb();
+
+	return entered_lp2 ? index : 0;
+}
+#endif
+
 int __init tegra20_cpuidle_init(void)
 {
-	int ret;
+	int ret, i;
 	unsigned int cpu;
 	struct cpuidle_device *dev;
 	struct cpuidle_driver *drv = &tegra_idle_driver;
 
+	drv->state_count = sizeof(tegra_idle_states) /
+			   sizeof(struct cpuidle_state);
+	for (i = 0; i < drv->state_count; i++)
+		memcpy(&drv->states[i], &tegra_idle_states[i],
+		       sizeof(struct cpuidle_state));
+
 	ret = cpuidle_register_driver(&tegra_idle_driver);
 	if (ret) {
 		pr_err("CPUidle driver registration failed\n");
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 1b11707..db72ea9 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
 
 	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
 		last_cpu = true;
+	else if (phy_cpu_id == 1)
+		tegra20_cpu_set_resettable_soon();
 
 	spin_unlock(&tegra_lp2_lock);
 	return last_cpu;
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index 72ce709..d04693b 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -21,6 +21,8 @@
 #include <linux/linkage.h>
 
 #include <asm/assembler.h>
+#include <asm/proc-fns.h>
+#include <asm/cp15.h>
 
 #include "sleep.h"
 #include "flowctrl.h"
@@ -78,3 +80,148 @@ ENTRY(tegra20_cpu_shutdown)
 	mov	pc, lr
 ENDPROC(tegra20_cpu_shutdown)
 #endif
+
+#ifdef CONFIG_PM_SLEEP
+/*
+ * tegra_pen_lock
+ *
+ * spinlock implementation with no atomic test-and-set and no coherence
+ * using Peterson's algorithm on strongly-ordered registers
+ * used to synchronize a cpu waking up from wfi with entering lp2 on idle
+ *
+ * The reference link of Peterson's algorithm:
+ * http://en.wikipedia.org/wiki/Peterson's_algorithm
+ *
+ * SCRATCH37 = r1 = !turn (inverted from Peterson's algorithm)
+ * on cpu 0:
+ * r2 = flag[0] (in SCRATCH38)
+ * r3 = flag[1] (in SCRATCH39)
+ * on cpu1:
+ * r2 = flag[1] (in SCRATCH39)
+ * r3 = flag[0] (in SCRATCH38)
+ *
+ * must be called with MMU on
+ * corrupts r0-r3, r12
+ */
+ENTRY(tegra_pen_lock)
+	mov32	r3, TEGRA_PMC_VIRT
+	cpu_id	r0
+	add	r1, r3, #PMC_SCRATCH37
+	cmp	r0, #0
+	addeq	r2, r3, #PMC_SCRATCH38
+	addeq	r3, r3, #PMC_SCRATCH39
+	addne	r2, r3, #PMC_SCRATCH39
+	addne	r3, r3, #PMC_SCRATCH38
+
+	mov	r12, #1
+	str	r12, [r2]		@ flag[cpu] = 1
+	dsb
+	str	r12, [r1]		@ !turn = cpu
+1:	dsb
+	ldr	r12, [r3]
+	cmp	r12, #1			@ flag[!cpu] == 1?
+	ldreq	r12, [r1]
+	cmpeq	r12, r0			@ !turn == cpu?
+	beq	1b			@ while !turn == cpu && flag[!cpu] == 1
+
+	mov	pc, lr			@ locked
+ENDPROC(tegra_pen_lock)
+
+ENTRY(tegra_pen_unlock)
+	dsb
+	mov32	r3, TEGRA_PMC_VIRT
+	cpu_id	r0
+	cmp	r0, #0
+	addeq	r2, r3, #PMC_SCRATCH38
+	addne	r2, r3, #PMC_SCRATCH39
+	mov	r12, #0
+	str	r12, [r2]
+	mov     pc, lr
+ENDPROC(tegra_pen_unlock)
+
+/*
+ * tegra20_cpu_clear_resettable(void)
+ *
+ * Called to clear the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_clear_resettable)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_NOT_RESETTABLE
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_clear_resettable)
+
+/*
+ * tegra20_cpu_set_resettable_soon(void)
+ *
+ * Called to set the "resettable soon" flag in PMC_SCRATCH41 when
+ * it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_set_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE_SOON
+	str	r12, [r1]
+	mov	pc, lr
+ENDPROC(tegra20_cpu_set_resettable_soon)
+
+/*
+ * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
+ *
+ * Enters WFI on secondary CPU by exiting coherency.
+ */
+ENTRY(tegra20_sleep_cpu_secondary_finish)
+	stmfd	sp!, {r4-r11, lr}
+
+	mrc	p15, 0, r11, c1, c0, 1  @ save actlr before exiting coherency
+
+	/* Flush and disable the L1 data cache */
+	bl	tegra_disable_clean_inv_dcache
+
+	mov32	r0, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r3, #CPU_RESETTABLE
+	str	r3, [r0]
+
+	bl	cpu_do_idle
+
+	/*
+	 * cpu may be reset while in wfi, which will return through
+	 * tegra_resume to cpu_resume
+	 * or interrupt may wake wfi, which will return here
+	 * cpu state is unchanged - MMU is on, cache is on, coherency
+	 * is off, and the data cache is off
+	 *
+	 * r11 contains the original actlr
+	 */
+
+	bl	tegra_pen_lock
+
+	mov32	r3, TEGRA_PMC_VIRT
+	add	r0, r3, #PMC_SCRATCH41
+	mov	r3, #CPU_NOT_RESETTABLE
+	str	r3, [r0]
+
+	bl	tegra_pen_unlock
+
+	/* Re-enable the data cache */
+	mrc	p15, 0, r10, c1, c0, 0
+	orr	r10, r10, #CR_C
+	mcr	p15, 0, r10, c1, c0, 0
+	isb
+
+	mcr	p15, 0, r11, c1, c0, 1	@ reenable coherency
+
+	/* Invalidate the TLBs & BTAC */
+	mov	r1, #0
+	mcr	p15, 0, r1, c8, c3, 0	@ invalidate shared TLBs
+	mcr	p15, 0, r1, c7, c1, 6	@ invalidate shared BTAC
+	dsb
+	isb
+
+	/* the cpu was running with coherency disabled,
+	 * caches may be out of date */
+	bl	v7_flush_kern_cache_louis
+
+	ldmfd	sp!, {r4 - r11, pc}
+ENDPROC(tegra20_sleep_cpu_secondary_finish)
+#endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 9821ee7..a02f71a 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -25,6 +25,19 @@
 					+ IO_PPSB_VIRT)
 #define TEGRA_CLK_RESET_VIRT (TEGRA_CLK_RESET_BASE - IO_PPSB_PHYS \
 					+ IO_PPSB_VIRT)
+#define TEGRA_PMC_VIRT	(TEGRA_PMC_BASE - IO_APB_PHYS + IO_APB_VIRT)
+
+/* PMC_SCRATCH37-39 and 41 are used for tegra_pen_lock and idle */
+#define PMC_SCRATCH37	0x130
+#define PMC_SCRATCH38	0x134
+#define PMC_SCRATCH39	0x138
+#define PMC_SCRATCH41	0x140
+
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+#define CPU_RESETTABLE		2
+#define CPU_RESETTABLE_SOON	1
+#define CPU_NOT_RESETTABLE	0
+#endif
 
 #ifdef __ASSEMBLY__
 /* returns the offset of the flow controller halt register for a cpu */
@@ -104,6 +117,8 @@ exit_l2_resume:
 .endm
 #endif /* CONFIG_CACHE_L2X0 */
 #else
+void tegra_pen_lock(void);
+void tegra_pen_unlock(void);
 void tegra_resume(void);
 int tegra_sleep_cpu_finish(unsigned long);
 
@@ -115,6 +130,14 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_clear_resettable(void);
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+void tegra20_cpu_set_resettable_soon(void);
+#else
+static inline void tegra20_cpu_set_resettable_soon(void) {}
+#endif
+
+int tegra20_sleep_cpu_secondary_finish(unsigned long);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
-- 
1.7.0.4

[PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Joseph Lo]

Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
functions were used for CPU powered-down state maintenance.

Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
V3:
* no change
V2:
* refine the code sequence in "tegra20_cpu_rail_off_ready"
---
 arch/arm/mach-tegra/tegra20_clocks.c |   99 ++++++++++++++++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c
index 4eb6bc8..1780268 100644
--- a/arch/arm/mach-tegra/tegra20_clocks.c
+++ b/arch/arm/mach-tegra/tegra20_clocks.c
@@ -159,6 +159,31 @@
 #define CPU_CLOCK(cpu)	(0x1 << (8 + cpu))
 #define CPU_RESET(cpu)	(0x1111ul << (cpu))
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+#define CLK_RESET_PLLX_BASE	0xe0
+#define CLK_RESET_PLLX_MISC	0xe4
+
+#define CLK_RESET_SOURCE_CSITE	0x1d4
+
+#define CLK_RESET_CCLK_BURST_POLICY_SHIFT	28
+#define CLK_RESET_CCLK_RUN_POLICY_SHIFT		4
+#define CLK_RESET_CCLK_IDLE_POLICY_SHIFT	0
+#define CLK_RESET_CCLK_IDLE_POLICY		1
+#define CLK_RESET_CCLK_RUN_POLICY		2
+#define CLK_RESET_CCLK_BURST_POLICY_PLLX	8
+
+#ifdef CONFIG_PM_SLEEP
+static struct cpu_clk_suspend_context {
+	u32 pllx_misc;
+	u32 pllx_base;
+
+	u32 cpu_burst;
+	u32 clk_csite_src;
+	u32 cclk_divider;
+} tegra20_cpu_clk_sctx;
+#endif
+
 static void __iomem *reg_clk_base = IO_ADDRESS(TEGRA_CLK_RESET_BASE);
 static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
 
@@ -1609,12 +1634,86 @@ static void tegra20_disable_cpu_clock(u32 cpu)
 	       reg_clk_base + TEGRA_CLK_RST_CONTROLLER_CLK_CPU_CMPLX);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static bool tegra20_cpu_rail_off_ready(void)
+{
+	unsigned int cpu_rst_status;
+
+	cpu_rst_status = readl(reg_clk_base +
+			       TEGRA_CLK_RST_CONTROLLER_RST_CPU_CMPLX_SET);
+
+	return !!(cpu_rst_status & 0x2);
+}
+
+static void tegra20_cpu_clock_suspend(void)
+{
+	/* switch coresite to clk_m, save off original source */
+	tegra20_cpu_clk_sctx.clk_csite_src =
+				readl(reg_clk_base + CLK_RESET_SOURCE_CSITE);
+	writel(3<<30, reg_clk_base + CLK_RESET_SOURCE_CSITE);
+
+	tegra20_cpu_clk_sctx.cpu_burst =
+				readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	tegra20_cpu_clk_sctx.pllx_base =
+				readl(reg_clk_base + CLK_RESET_PLLX_BASE);
+	tegra20_cpu_clk_sctx.pllx_misc =
+				readl(reg_clk_base + CLK_RESET_PLLX_MISC);
+	tegra20_cpu_clk_sctx.cclk_divider =
+				readl(reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+}
+
+static void tegra20_cpu_clock_resume(void)
+{
+	unsigned int reg, policy;
+
+	/* Is CPU complex already running on PLLX? */
+	reg = readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	policy = (reg >> CLK_RESET_CCLK_BURST_POLICY_SHIFT) & 0xF;
+
+	if (policy == CLK_RESET_CCLK_IDLE_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_IDLE_POLICY_SHIFT) & 0xF;
+	else if (policy == CLK_RESET_CCLK_RUN_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_RUN_POLICY_SHIFT) & 0xF;
+	else
+		BUG();
+
+	if (reg != CLK_RESET_CCLK_BURST_POLICY_PLLX) {
+		/* restore PLLX settings if CPU is on different PLL */
+		writel(tegra20_cpu_clk_sctx.pllx_misc,
+					reg_clk_base + CLK_RESET_PLLX_MISC);
+		writel(tegra20_cpu_clk_sctx.pllx_base,
+					reg_clk_base + CLK_RESET_PLLX_BASE);
+
+		/* wait for PLL stabilization if PLLX was enabled */
+		if (tegra20_cpu_clk_sctx.pllx_base & (1 << 30))
+			udelay(300);
+	}
+
+	/*
+	 * Restore original burst policy setting for calls resulting from CPU
+	 * LP2 in idle or system suspend.
+	 */
+	writel(tegra20_cpu_clk_sctx.cclk_divider,
+					reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+	writel(tegra20_cpu_clk_sctx.cpu_burst,
+					reg_clk_base + CLK_RESET_CCLK_BURST);
+
+	writel(tegra20_cpu_clk_sctx.clk_csite_src,
+					reg_clk_base + CLK_RESET_SOURCE_CSITE);
+}
+#endif
+
 static struct tegra_cpu_car_ops tegra20_cpu_car_ops = {
 	.wait_for_reset	= tegra20_wait_cpu_in_reset,
 	.put_in_reset	= tegra20_put_cpu_in_reset,
 	.out_of_reset	= tegra20_cpu_out_of_reset,
 	.enable_clock	= tegra20_enable_cpu_clock,
 	.disable_clock	= tegra20_disable_cpu_clock,
+#ifdef CONFIG_PM_SLEEP
+	.rail_off_ready = tegra20_cpu_rail_off_ready,
+	.suspend	= tegra20_cpu_clock_suspend,
+	.resume		= tegra20_cpu_clock_resume,
+#endif
 };
 
 void __init tegra20_cpu_car_ops_init(void)
-- 
1.7.0.4

[PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Joseph Lo]

Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
functions were used for CPU powered-down state maintenance.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* no change
V2:
* refine the code sequence in "tegra20_cpu_rail_off_ready"
---
 arch/arm/mach-tegra/tegra20_clocks.c |   99 ++++++++++++++++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/tegra20_clocks.c b/arch/arm/mach-tegra/tegra20_clocks.c
index 4eb6bc8..1780268 100644
--- a/arch/arm/mach-tegra/tegra20_clocks.c
+++ b/arch/arm/mach-tegra/tegra20_clocks.c
@@ -159,6 +159,31 @@
 #define CPU_CLOCK(cpu)	(0x1 << (8 + cpu))
 #define CPU_RESET(cpu)	(0x1111ul << (cpu))
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+#define CLK_RESET_PLLX_BASE	0xe0
+#define CLK_RESET_PLLX_MISC	0xe4
+
+#define CLK_RESET_SOURCE_CSITE	0x1d4
+
+#define CLK_RESET_CCLK_BURST_POLICY_SHIFT	28
+#define CLK_RESET_CCLK_RUN_POLICY_SHIFT		4
+#define CLK_RESET_CCLK_IDLE_POLICY_SHIFT	0
+#define CLK_RESET_CCLK_IDLE_POLICY		1
+#define CLK_RESET_CCLK_RUN_POLICY		2
+#define CLK_RESET_CCLK_BURST_POLICY_PLLX	8
+
+#ifdef CONFIG_PM_SLEEP
+static struct cpu_clk_suspend_context {
+	u32 pllx_misc;
+	u32 pllx_base;
+
+	u32 cpu_burst;
+	u32 clk_csite_src;
+	u32 cclk_divider;
+} tegra20_cpu_clk_sctx;
+#endif
+
 static void __iomem *reg_clk_base = IO_ADDRESS(TEGRA_CLK_RESET_BASE);
 static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
 
@@ -1609,12 +1634,86 @@ static void tegra20_disable_cpu_clock(u32 cpu)
 	       reg_clk_base + TEGRA_CLK_RST_CONTROLLER_CLK_CPU_CMPLX);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static bool tegra20_cpu_rail_off_ready(void)
+{
+	unsigned int cpu_rst_status;
+
+	cpu_rst_status = readl(reg_clk_base +
+			       TEGRA_CLK_RST_CONTROLLER_RST_CPU_CMPLX_SET);
+
+	return !!(cpu_rst_status & 0x2);
+}
+
+static void tegra20_cpu_clock_suspend(void)
+{
+	/* switch coresite to clk_m, save off original source */
+	tegra20_cpu_clk_sctx.clk_csite_src =
+				readl(reg_clk_base + CLK_RESET_SOURCE_CSITE);
+	writel(3<<30, reg_clk_base + CLK_RESET_SOURCE_CSITE);
+
+	tegra20_cpu_clk_sctx.cpu_burst =
+				readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	tegra20_cpu_clk_sctx.pllx_base =
+				readl(reg_clk_base + CLK_RESET_PLLX_BASE);
+	tegra20_cpu_clk_sctx.pllx_misc =
+				readl(reg_clk_base + CLK_RESET_PLLX_MISC);
+	tegra20_cpu_clk_sctx.cclk_divider =
+				readl(reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+}
+
+static void tegra20_cpu_clock_resume(void)
+{
+	unsigned int reg, policy;
+
+	/* Is CPU complex already running on PLLX? */
+	reg = readl(reg_clk_base + CLK_RESET_CCLK_BURST);
+	policy = (reg >> CLK_RESET_CCLK_BURST_POLICY_SHIFT) & 0xF;
+
+	if (policy == CLK_RESET_CCLK_IDLE_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_IDLE_POLICY_SHIFT) & 0xF;
+	else if (policy == CLK_RESET_CCLK_RUN_POLICY)
+		reg = (reg >> CLK_RESET_CCLK_RUN_POLICY_SHIFT) & 0xF;
+	else
+		BUG();
+
+	if (reg != CLK_RESET_CCLK_BURST_POLICY_PLLX) {
+		/* restore PLLX settings if CPU is on different PLL */
+		writel(tegra20_cpu_clk_sctx.pllx_misc,
+					reg_clk_base + CLK_RESET_PLLX_MISC);
+		writel(tegra20_cpu_clk_sctx.pllx_base,
+					reg_clk_base + CLK_RESET_PLLX_BASE);
+
+		/* wait for PLL stabilization if PLLX was enabled */
+		if (tegra20_cpu_clk_sctx.pllx_base & (1 << 30))
+			udelay(300);
+	}
+
+	/*
+	 * Restore original burst policy setting for calls resulting from CPU
+	 * LP2 in idle or system suspend.
+	 */
+	writel(tegra20_cpu_clk_sctx.cclk_divider,
+					reg_clk_base + CLK_RESET_CCLK_DIVIDER);
+	writel(tegra20_cpu_clk_sctx.cpu_burst,
+					reg_clk_base + CLK_RESET_CCLK_BURST);
+
+	writel(tegra20_cpu_clk_sctx.clk_csite_src,
+					reg_clk_base + CLK_RESET_SOURCE_CSITE);
+}
+#endif
+
 static struct tegra_cpu_car_ops tegra20_cpu_car_ops = {
 	.wait_for_reset	= tegra20_wait_cpu_in_reset,
 	.put_in_reset	= tegra20_put_cpu_in_reset,
 	.out_of_reset	= tegra20_cpu_out_of_reset,
 	.enable_clock	= tegra20_enable_cpu_clock,
 	.disable_clock	= tegra20_disable_cpu_clock,
+#ifdef CONFIG_PM_SLEEP
+	.rail_off_ready = tegra20_cpu_rail_off_ready,
+	.suspend	= tegra20_cpu_clock_suspend,
+	.resume		= tegra20_cpu_clock_resume,
+#endif
 };
 
 void __init tegra20_cpu_car_ops_init(void)
-- 
1.7.0.4

[PATCH V3 4/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit

[Joseph Lo]

The flow controller can help CPU to go into suspend mode (powered-down
state). When CPU go into powered-down state, it needs some careful
settings before getting into and after leaving. The enter and exit
functions do that by configuring appropriate mode for flow controller.

Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
V3:
* no change
V2:
* no change
---
 arch/arm/mach-tegra/flowctrl.c |   38 +++++++++++++++++++++++++++++++++-----
 arch/arm/mach-tegra/flowctrl.h |    4 ++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-tegra/flowctrl.c b/arch/arm/mach-tegra/flowctrl.c
index a2250dd..9c44788 100644
--- a/arch/arm/mach-tegra/flowctrl.c
+++ b/arch/arm/mach-tegra/flowctrl.c
@@ -25,6 +25,7 @@
 
 #include "flowctrl.h"
 #include "iomap.h"
+#include "fuse.h"
 
 u8 flowctrl_offset_halt_cpu[] = {
 	FLOW_CTRL_HALT_CPU0_EVENTS,
@@ -75,11 +76,26 @@ void flowctrl_cpu_suspend_enter(unsigned int cpuid)
 	int i;
 
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfe */
+		reg |= TEGRA20_FLOW_CTRL_CSR_WFE_CPU0 << cpuid;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfi */
+		reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;
+		break;
+	}
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr flag */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event flag */
-	reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;	/* pwr gating on wfi */
 	reg |= FLOW_CTRL_CSR_ENABLE;			/* pwr gating */
 	flowctrl_write_cpu_csr(cpuid, reg);
 
@@ -99,8 +115,20 @@ void flowctrl_cpu_suspend_exit(unsigned int cpuid)
 
 	/* Disable powergating via flow controller for CPU0 */
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	}
 	reg &= ~FLOW_CTRL_CSR_ENABLE;			/* clear enable */
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event */
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 0798dec..67eab56 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -34,6 +34,10 @@
 #define FLOW_CTRL_HALT_CPU1_EVENTS	0x14
 #define FLOW_CTRL_CPU1_CSR		0x18
 
+#define TEGRA20_FLOW_CTRL_CSR_WFE_CPU0		(1 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP	(3 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP	0
+
 #define TEGRA30_FLOW_CTRL_CSR_WFI_CPU0		(1 << 8)
 #define TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP	(0xF << 4)
 #define TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP	(0xF << 8)
-- 
1.7.0.4

[PATCH V3 4/5] ARM: tegra20: flowctrl: add support for cpu_suspend_enter/exit

[Joseph Lo]

The flow controller can help CPU to go into suspend mode (powered-down
state). When CPU go into powered-down state, it needs some careful
settings before getting into and after leaving. The enter and exit
functions do that by configuring appropriate mode for flow controller.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* no change
V2:
* no change
---
 arch/arm/mach-tegra/flowctrl.c |   38 +++++++++++++++++++++++++++++++++-----
 arch/arm/mach-tegra/flowctrl.h |    4 ++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-tegra/flowctrl.c b/arch/arm/mach-tegra/flowctrl.c
index a2250dd..9c44788 100644
--- a/arch/arm/mach-tegra/flowctrl.c
+++ b/arch/arm/mach-tegra/flowctrl.c
@@ -25,6 +25,7 @@
 
 #include "flowctrl.h"
 #include "iomap.h"
+#include "fuse.h"
 
 u8 flowctrl_offset_halt_cpu[] = {
 	FLOW_CTRL_HALT_CPU0_EVENTS,
@@ -75,11 +76,26 @@ void flowctrl_cpu_suspend_enter(unsigned int cpuid)
 	int i;
 
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfe */
+		reg |= TEGRA20_FLOW_CTRL_CSR_WFE_CPU0 << cpuid;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		/* pwr gating on wfi */
+		reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;
+		break;
+	}
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr flag */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event flag */
-	reg |= TEGRA30_FLOW_CTRL_CSR_WFI_CPU0 << cpuid;	/* pwr gating on wfi */
 	reg |= FLOW_CTRL_CSR_ENABLE;			/* pwr gating */
 	flowctrl_write_cpu_csr(cpuid, reg);
 
@@ -99,8 +115,20 @@ void flowctrl_cpu_suspend_exit(unsigned int cpuid)
 
 	/* Disable powergating via flow controller for CPU0 */
 	reg = flowctrl_read_cpu_csr(cpuid);
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;	/* clear wfe bitmap */
-	reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;	/* clear wfi bitmap */
+	switch (tegra_chip_id) {
+	case TEGRA20:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	case TEGRA30:
+		/* clear wfe bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP;
+		/* clear wfi bitmap */
+		reg &= ~TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP;
+		break;
+	}
 	reg &= ~FLOW_CTRL_CSR_ENABLE;			/* clear enable */
 	reg |= FLOW_CTRL_CSR_INTR_FLAG;			/* clear intr */
 	reg |= FLOW_CTRL_CSR_EVENT_FLAG;		/* clear event */
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 0798dec..67eab56 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -34,6 +34,10 @@
 #define FLOW_CTRL_HALT_CPU1_EVENTS	0x14
 #define FLOW_CTRL_CPU1_CSR		0x18
 
+#define TEGRA20_FLOW_CTRL_CSR_WFE_CPU0		(1 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFE_BITMAP	(3 << 4)
+#define TEGRA20_FLOW_CTRL_CSR_WFI_BITMAP	0
+
 #define TEGRA30_FLOW_CTRL_CSR_WFI_CPU0		(1 << 8)
 #define TEGRA30_FLOW_CTRL_CSR_WFE_BITMAP	(0xF << 4)
 #define TEGRA30_FLOW_CTRL_CSR_WFI_BITMAP	(0xF << 8)
-- 
1.7.0.4

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Joseph Lo]

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
core to go into this mode before other core. The coupled cpuidle framework
can help to sync the MPCore to coupled state then go into "powered-down"
idle mode together. The driver can just assume the MPCore come into
"powered-down" mode at the same time. No need to take care if the CPU_0
goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores.
When the secondary CPU requests to enter powered-down state, it saves
its own contexts and then enters WFI for waiting CPU0 in the same state.
When the CPU0 requests powered-down state, it attempts to put the secondary
CPU into reset to prevent it from waking up. Then power down both CPUs
together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code
which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
Gary King <gking-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
V3:
* sqash last two patch in previous version to support coupled cpuidle
  directly
V2:
* refine the cpu control function that dedicate for CPU_1
* rename "tegra_cpu_pllp" to "tegra_switch_cpu_to_pllp"
---
 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |  125 ++++++++++++++++++++++++++++++---
 arch/arm/mach-tegra/sleep-tegra20.S   |   53 ++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++++
 arch/arm/mach-tegra/sleep.h           |    3 +
 5 files changed, 192 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index b442f15..b46ff2e 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -4,6 +4,7 @@ comment "NVIDIA Tegra options"
 
 config ARCH_TEGRA_2x_SOC
 	bool "Enable support for Tegra20 family"
+	select ARCH_NEEDS_CPU_IDLE_COUPLED
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_742230
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 716aef3..7c7bb3f 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -32,22 +32,29 @@
 
 #include "pm.h"
 #include "sleep.h"
+#include "iomap.h"
+#include "irq.h"
+#include "tegra_cpu_car.h"
+#include "flowctrl.h"
 
 #ifdef CONFIG_PM_SLEEP
-static int tegra20_idle_lp2(struct cpuidle_device *dev,
-			    struct cpuidle_driver *drv,
-			    int index);
+static atomic_t abort_flag;
+static atomic_t abort_barrier;
+static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+				    struct cpuidle_driver *drv,
+				    int index);
 #endif
 
 static struct cpuidle_state tegra_idle_states[] = {
 	[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
 #ifdef CONFIG_PM_SLEEP
 	[1] = {
-		.enter			= tegra20_idle_lp2,
+		.enter			= tegra20_idle_lp2_coupled,
 		.exit_latency		= 5000,
 		.target_residency	= 10000,
 		.power_usage		= 0,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.flags			= CPUIDLE_FLAG_TIME_VALID |
+					  CPUIDLE_FLAG_COUPLED,
 		.name			= "powered-down",
 		.desc			= "CPU power gated",
 	},
@@ -64,6 +71,88 @@ static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
 #ifdef CONFIG_PM_SLEEP
 #ifdef CONFIG_SMP
+static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
+
+static int tegra20_reset_sleeping_cpu_1(void)
+{
+	int ret = 0;
+
+	tegra_pen_lock();
+
+	if (readl(pmc + PMC_SCRATCH41) == CPU_RESETTABLE)
+		tegra20_cpu_shutdown(1);
+	else
+		ret = -EINVAL;
+
+	tegra_pen_unlock();
+
+	return ret;
+}
+
+static void tegra20_wake_reset_cpu_1(void)
+{
+	tegra_pen_lock();
+
+	tegra20_cpu_clear_resettable();
+
+	/* enable cpu clock on cpu */
+	tegra_enable_cpu_clock(1);
+
+	/* take the CPU out of reset */
+	tegra_cpu_out_of_reset(1);
+
+	/* unhalt the cpu */
+	flowctrl_write_cpu_halt(1, 0);
+
+	tegra_pen_unlock();
+}
+
+static int tegra20_reset_cpu_1(void)
+{
+	if (!cpu_online(1) || !tegra20_reset_sleeping_cpu_1())
+		return 0;
+
+	tegra20_wake_reset_cpu_1();
+	return -EBUSY;
+}
+#else
+static inline void tegra20_wake_reset_cpu_1(void)
+{
+}
+
+static inline int tegra20_reset_cpu_1(void)
+{
+	return 0;
+}
+#endif
+
+static bool tegra20_cpu_cluster_power_down(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;
+
+	while (tegra20_cpu_is_resettable_soon())
+		cpu_relax();
+
+	if (tegra20_reset_cpu_1() || !tegra_cpu_rail_off_ready())
+		return false;
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	if (cpu_online(1))
+		tegra20_wake_reset_cpu_1();
+
+	return true;
+}
+
+#ifdef CONFIG_SMP
 static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 					 struct cpuidle_driver *drv,
 					 int index)
@@ -87,20 +176,31 @@ static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 }
 #endif
 
-static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
-				      struct cpuidle_driver *drv,
-				      int index)
+static int __cpuinit tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+					      struct cpuidle_driver *drv,
+					      int index)
 {
 	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
 	bool entered_lp2 = false;
 
+	if (tegra_pending_sgi())
+		atomic_inc(&abort_flag);
+
+	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
+
+	if (atomic_read(&abort_flag) > 0) {
+		cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
+		atomic_set(&abort_flag, 0);	/* clean flag for next coming */
+		return -EINTR;
+	}
+
 	local_fiq_disable();
 
 	tegra_set_cpu_in_lp2(cpu);
 	cpu_pm_enter();
 
 	if (cpu == 0)
-		cpu_do_idle();
+		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
 	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
 
@@ -122,6 +222,10 @@ int __init tegra20_cpuidle_init(void)
 	struct cpuidle_device *dev;
 	struct cpuidle_driver *drv = &tegra_idle_driver;
 
+#ifdef CONFIG_PM_SLEEP
+	tegra_tear_down_cpu = tegra20_tear_down_cpu;
+#endif
+
 	drv->state_count = sizeof(tegra_idle_states) /
 			   sizeof(struct cpuidle_state);
 	for (i = 0; i < drv->state_count; i++)
@@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
 	for_each_possible_cpu(cpu) {
 		dev = &per_cpu(tegra_idle_device, cpu);
 		dev->cpu = cpu;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif
 
 		dev->state_count = drv->state_count;
 		ret = cpuidle_register_device(dev);
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index d04693b..d57aea0 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -60,6 +60,9 @@ ENDPROC(tegra20_hotplug_shutdown)
 ENTRY(tegra20_cpu_shutdown)
 	cmp	r0, #0
 	moveq	pc, lr			@ must not be called for CPU 0
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE
+	str	r12, [r1]
 
 	cpu_to_halt_reg r1, r0
 	ldr	r3, =TEGRA_FLOW_CTRL_VIRT
@@ -166,6 +169,21 @@ ENTRY(tegra20_cpu_set_resettable_soon)
 ENDPROC(tegra20_cpu_set_resettable_soon)
 
 /*
+ * tegra20_cpu_is_resettable_soon(void)
+ *
+ * Returns true if the "resettable soon" flag in PMC_SCRATCH41 has been
+ * set because it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_is_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	ldr	r12, [r1]
+	cmp	r12, #CPU_RESETTABLE_SOON
+	moveq	r0, #1
+	movne	r0, #0
+	mov	pc, lr
+ENDPROC(tegra20_cpu_is_resettable_soon)
+
+/*
  * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
  *
  * Enters WFI on secondary CPU by exiting coherency.
@@ -224,4 +242,39 @@ ENTRY(tegra20_sleep_cpu_secondary_finish)
 
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(tegra20_sleep_cpu_secondary_finish)
+
+/*
+ * tegra20_tear_down_cpu
+ *
+ * Switches the CPU cluster to PLL-P and enters sleep.
+ */
+ENTRY(tegra20_tear_down_cpu)
+	bl	tegra_switch_cpu_to_pllp
+	b	tegra20_enter_sleep
+ENDPROC(tegra20_tear_down_cpu)
+
+/*
+ * tegra20_enter_sleep
+ *
+ * uses flow controller to enter sleep state
+ * executes from IRAM with SDRAM in selfrefresh when target state is LP0 or LP1
+ * executes from SDRAM with target state is LP2
+ */
+tegra20_enter_sleep:
+	mov32   r6, TEGRA_FLOW_CTRL_BASE
+
+	mov     r0, #FLOW_CTRL_WAIT_FOR_INTERRUPT
+	orr	r0, r0, #FLOW_CTRL_HALT_CPU_IRQ | FLOW_CTRL_HALT_CPU_FIQ
+	cpu_id	r1
+	cpu_to_halt_reg r1, r1
+	str	r0, [r6, r1]
+	dsb
+	ldr	r0, [r6, r1] /* memory barrier */
+
+halted:
+	dsb
+	wfe	/* CPU should be power gated here */
+	isb
+	b	halted
+
 #endif
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 26afa7c..bbda6e9 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -34,6 +34,9 @@
 #include "flowctrl.h"
 #include "sleep.h"
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+
 #ifdef CONFIG_PM_SLEEP
 /*
  * tegra_disable_clean_inv_dcache
@@ -108,4 +111,20 @@ ENTRY(tegra_shut_off_mmu)
 	mov	pc, r0
 ENDPROC(tegra_shut_off_mmu)
 	.popsection
+
+/*
+ * tegra_switch_cpu_to_pllp
+ *
+ * In LP2 the normal cpu clock pllx will be turned off. Switch the CPU to pllp
+ */
+ENTRY(tegra_switch_cpu_to_pllp)
+	/* in LP2 idle (SDRAM active), set the CPU burst policy to PLLP */
+	mov32	r5, TEGRA_CLK_RESET_BASE
+	mov	r0, #(2 << 28)			@ burst policy = run mode
+	orr	r0, r0, #(4 << 4)		@ use PLLP in run mode burst
+	str	r0, [r5, #CLK_RESET_CCLK_BURST]
+	mov	r0, #0
+	str	r0, [r5, #CLK_RESET_CCLK_DIVIDER]
+	mov	pc, lr
+ENDPROC(tegra_switch_cpu_to_pllp)
 #endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index a02f71a..4d2b173 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -130,6 +130,8 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_shutdown(int cpu);
+int tegra20_cpu_is_resettable_soon(void);
 void tegra20_cpu_clear_resettable(void);
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 void tegra20_cpu_set_resettable_soon(void);
@@ -138,6 +140,7 @@ static inline void tegra20_cpu_set_resettable_soon(void) {}
 #endif
 
 int tegra20_sleep_cpu_secondary_finish(unsigned long);
+void tegra20_tear_down_cpu(void);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
-- 
1.7.0.4

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Joseph Lo]

The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
core to go into this mode before other core. The coupled cpuidle framework
can help to sync the MPCore to coupled state then go into "powered-down"
idle mode together. The driver can just assume the MPCore come into
"powered-down" mode at the same time. No need to take care if the CPU_0
goes into this mode along and only can put it into safe idle mode (WFI).

The powered-down state of Tegra20 requires power gating both CPU cores.
When the secondary CPU requests to enter powered-down state, it saves
its own contexts and then enters WFI for waiting CPU0 in the same state.
When the CPU0 requests powered-down state, it attempts to put the secondary
CPU into reset to prevent it from waking up. Then power down both CPUs
together and power off the cpu rail.

Be aware of that, you may see the legacy power state "LP2" in the code
which is exactly the same meaning of "CPU power down".

Based on the work by:
Colin Cross <ccross@android.com>
Gary King <gking@nvidia.com>

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
V3:
* sqash last two patch in previous version to support coupled cpuidle
  directly
V2:
* refine the cpu control function that dedicate for CPU_1
* rename "tegra_cpu_pllp" to "tegra_switch_cpu_to_pllp"
---
 arch/arm/mach-tegra/Kconfig           |    1 +
 arch/arm/mach-tegra/cpuidle-tegra20.c |  125 ++++++++++++++++++++++++++++++---
 arch/arm/mach-tegra/sleep-tegra20.S   |   53 ++++++++++++++
 arch/arm/mach-tegra/sleep.S           |   19 +++++
 arch/arm/mach-tegra/sleep.h           |    3 +
 5 files changed, 192 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index b442f15..b46ff2e 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -4,6 +4,7 @@ comment "NVIDIA Tegra options"
 
 config ARCH_TEGRA_2x_SOC
 	bool "Enable support for Tegra20 family"
+	select ARCH_NEEDS_CPU_IDLE_COUPLED
 	select ARCH_REQUIRE_GPIOLIB
 	select ARM_ERRATA_720789
 	select ARM_ERRATA_742230
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 716aef3..7c7bb3f 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -32,22 +32,29 @@
 
 #include "pm.h"
 #include "sleep.h"
+#include "iomap.h"
+#include "irq.h"
+#include "tegra_cpu_car.h"
+#include "flowctrl.h"
 
 #ifdef CONFIG_PM_SLEEP
-static int tegra20_idle_lp2(struct cpuidle_device *dev,
-			    struct cpuidle_driver *drv,
-			    int index);
+static atomic_t abort_flag;
+static atomic_t abort_barrier;
+static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+				    struct cpuidle_driver *drv,
+				    int index);
 #endif
 
 static struct cpuidle_state tegra_idle_states[] = {
 	[0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
 #ifdef CONFIG_PM_SLEEP
 	[1] = {
-		.enter			= tegra20_idle_lp2,
+		.enter			= tegra20_idle_lp2_coupled,
 		.exit_latency		= 5000,
 		.target_residency	= 10000,
 		.power_usage		= 0,
-		.flags			= CPUIDLE_FLAG_TIME_VALID,
+		.flags			= CPUIDLE_FLAG_TIME_VALID |
+					  CPUIDLE_FLAG_COUPLED,
 		.name			= "powered-down",
 		.desc			= "CPU power gated",
 	},
@@ -64,6 +71,88 @@ static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
 
 #ifdef CONFIG_PM_SLEEP
 #ifdef CONFIG_SMP
+static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
+
+static int tegra20_reset_sleeping_cpu_1(void)
+{
+	int ret = 0;
+
+	tegra_pen_lock();
+
+	if (readl(pmc + PMC_SCRATCH41) == CPU_RESETTABLE)
+		tegra20_cpu_shutdown(1);
+	else
+		ret = -EINVAL;
+
+	tegra_pen_unlock();
+
+	return ret;
+}
+
+static void tegra20_wake_reset_cpu_1(void)
+{
+	tegra_pen_lock();
+
+	tegra20_cpu_clear_resettable();
+
+	/* enable cpu clock on cpu */
+	tegra_enable_cpu_clock(1);
+
+	/* take the CPU out of reset */
+	tegra_cpu_out_of_reset(1);
+
+	/* unhalt the cpu */
+	flowctrl_write_cpu_halt(1, 0);
+
+	tegra_pen_unlock();
+}
+
+static int tegra20_reset_cpu_1(void)
+{
+	if (!cpu_online(1) || !tegra20_reset_sleeping_cpu_1())
+		return 0;
+
+	tegra20_wake_reset_cpu_1();
+	return -EBUSY;
+}
+#else
+static inline void tegra20_wake_reset_cpu_1(void)
+{
+}
+
+static inline int tegra20_reset_cpu_1(void)
+{
+	return 0;
+}
+#endif
+
+static bool tegra20_cpu_cluster_power_down(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;
+
+	while (tegra20_cpu_is_resettable_soon())
+		cpu_relax();
+
+	if (tegra20_reset_cpu_1() || !tegra_cpu_rail_off_ready())
+		return false;
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+
+	tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+
+	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+
+	if (cpu_online(1))
+		tegra20_wake_reset_cpu_1();
+
+	return true;
+}
+
+#ifdef CONFIG_SMP
 static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 					 struct cpuidle_driver *drv,
 					 int index)
@@ -87,20 +176,31 @@ static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
 }
 #endif
 
-static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
-				      struct cpuidle_driver *drv,
-				      int index)
+static int __cpuinit tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
+					      struct cpuidle_driver *drv,
+					      int index)
 {
 	u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
 	bool entered_lp2 = false;
 
+	if (tegra_pending_sgi())
+		atomic_inc(&abort_flag);
+
+	cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
+
+	if (atomic_read(&abort_flag) > 0) {
+		cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
+		atomic_set(&abort_flag, 0);	/* clean flag for next coming */
+		return -EINTR;
+	}
+
 	local_fiq_disable();
 
 	tegra_set_cpu_in_lp2(cpu);
 	cpu_pm_enter();
 
 	if (cpu == 0)
-		cpu_do_idle();
+		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
 	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
 
@@ -122,6 +222,10 @@ int __init tegra20_cpuidle_init(void)
 	struct cpuidle_device *dev;
 	struct cpuidle_driver *drv = &tegra_idle_driver;
 
+#ifdef CONFIG_PM_SLEEP
+	tegra_tear_down_cpu = tegra20_tear_down_cpu;
+#endif
+
 	drv->state_count = sizeof(tegra_idle_states) /
 			   sizeof(struct cpuidle_state);
 	for (i = 0; i < drv->state_count; i++)
@@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
 	for_each_possible_cpu(cpu) {
 		dev = &per_cpu(tegra_idle_device, cpu);
 		dev->cpu = cpu;
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+		dev->coupled_cpus = *cpu_online_mask;
+#endif
 
 		dev->state_count = drv->state_count;
 		ret = cpuidle_register_device(dev);
diff --git a/arch/arm/mach-tegra/sleep-tegra20.S b/arch/arm/mach-tegra/sleep-tegra20.S
index d04693b..d57aea0 100644
--- a/arch/arm/mach-tegra/sleep-tegra20.S
+++ b/arch/arm/mach-tegra/sleep-tegra20.S
@@ -60,6 +60,9 @@ ENDPROC(tegra20_hotplug_shutdown)
 ENTRY(tegra20_cpu_shutdown)
 	cmp	r0, #0
 	moveq	pc, lr			@ must not be called for CPU 0
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	mov	r12, #CPU_RESETTABLE
+	str	r12, [r1]
 
 	cpu_to_halt_reg r1, r0
 	ldr	r3, =TEGRA_FLOW_CTRL_VIRT
@@ -166,6 +169,21 @@ ENTRY(tegra20_cpu_set_resettable_soon)
 ENDPROC(tegra20_cpu_set_resettable_soon)
 
 /*
+ * tegra20_cpu_is_resettable_soon(void)
+ *
+ * Returns true if the "resettable soon" flag in PMC_SCRATCH41 has been
+ * set because it is expected that the secondary CPU will be idle soon.
+ */
+ENTRY(tegra20_cpu_is_resettable_soon)
+	mov32	r1, TEGRA_PMC_VIRT + PMC_SCRATCH41
+	ldr	r12, [r1]
+	cmp	r12, #CPU_RESETTABLE_SOON
+	moveq	r0, #1
+	movne	r0, #0
+	mov	pc, lr
+ENDPROC(tegra20_cpu_is_resettable_soon)
+
+/*
  * tegra20_sleep_cpu_secondary_finish(unsigned long v2p)
  *
  * Enters WFI on secondary CPU by exiting coherency.
@@ -224,4 +242,39 @@ ENTRY(tegra20_sleep_cpu_secondary_finish)
 
 	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(tegra20_sleep_cpu_secondary_finish)
+
+/*
+ * tegra20_tear_down_cpu
+ *
+ * Switches the CPU cluster to PLL-P and enters sleep.
+ */
+ENTRY(tegra20_tear_down_cpu)
+	bl	tegra_switch_cpu_to_pllp
+	b	tegra20_enter_sleep
+ENDPROC(tegra20_tear_down_cpu)
+
+/*
+ * tegra20_enter_sleep
+ *
+ * uses flow controller to enter sleep state
+ * executes from IRAM with SDRAM in selfrefresh when target state is LP0 or LP1
+ * executes from SDRAM with target state is LP2
+ */
+tegra20_enter_sleep:
+	mov32   r6, TEGRA_FLOW_CTRL_BASE
+
+	mov     r0, #FLOW_CTRL_WAIT_FOR_INTERRUPT
+	orr	r0, r0, #FLOW_CTRL_HALT_CPU_IRQ | FLOW_CTRL_HALT_CPU_FIQ
+	cpu_id	r1
+	cpu_to_halt_reg r1, r1
+	str	r0, [r6, r1]
+	dsb
+	ldr	r0, [r6, r1] /* memory barrier */
+
+halted:
+	dsb
+	wfe	/* CPU should be power gated here */
+	isb
+	b	halted
+
 #endif
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 26afa7c..bbda6e9 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -34,6 +34,9 @@
 #include "flowctrl.h"
 #include "sleep.h"
 
+#define CLK_RESET_CCLK_BURST	0x20
+#define CLK_RESET_CCLK_DIVIDER  0x24
+
 #ifdef CONFIG_PM_SLEEP
 /*
  * tegra_disable_clean_inv_dcache
@@ -108,4 +111,20 @@ ENTRY(tegra_shut_off_mmu)
 	mov	pc, r0
 ENDPROC(tegra_shut_off_mmu)
 	.popsection
+
+/*
+ * tegra_switch_cpu_to_pllp
+ *
+ * In LP2 the normal cpu clock pllx will be turned off. Switch the CPU to pllp
+ */
+ENTRY(tegra_switch_cpu_to_pllp)
+	/* in LP2 idle (SDRAM active), set the CPU burst policy to PLLP */
+	mov32	r5, TEGRA_CLK_RESET_BASE
+	mov	r0, #(2 << 28)			@ burst policy = run mode
+	orr	r0, r0, #(4 << 4)		@ use PLLP in run mode burst
+	str	r0, [r5, #CLK_RESET_CCLK_BURST]
+	mov	r0, #0
+	str	r0, [r5, #CLK_RESET_CCLK_DIVIDER]
+	mov	pc, lr
+ENDPROC(tegra_switch_cpu_to_pllp)
 #endif
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index a02f71a..4d2b173 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -130,6 +130,8 @@ static inline void tegra20_hotplug_init(void) {}
 static inline void tegra30_hotplug_init(void) {}
 #endif
 
+void tegra20_cpu_shutdown(int cpu);
+int tegra20_cpu_is_resettable_soon(void);
 void tegra20_cpu_clear_resettable(void);
 #ifdef CONFIG_ARCH_TEGRA_2x_SOC
 void tegra20_cpu_set_resettable_soon(void);
@@ -138,6 +140,7 @@ static inline void tegra20_cpu_set_resettable_soon(void) {}
 #endif
 
 int tegra20_sleep_cpu_secondary_finish(unsigned long);
+void tegra20_tear_down_cpu(void);
 int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
-- 
1.7.0.4

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Colin Cross]

On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> include the power of GIC. That caused the SGI (Software Generated
> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> the "powered-down" CPU idle mode. We need to check if there is any
> pending SGI when go into "powered-down" CPU idle mode. This is important
> especially when applying the coupled cpuidle framework into "power-down"
> cpuidle dirver. Because the coupled cpuidle framework may have the
> chance that misses IPI_SINGLE_FUNC handling sometimes.

This problem exists for any GIC-based SoC, and needs to be fixed in
gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

> For the PPI or SPI, something like the legacy peripheral interrupt. It
> still can be maintained by Tegra legacy interrupt controller. If there
> is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> CPU can be woken up immediately. So we don't need to take care the same
> situation for PPI or SPI.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * move the static mapping of GIC addr into tegra_pending_sgi
> V2:
> * new in V2
> ---
>  arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
>  arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-tegra/irq.h
>
> diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
> index b7886f1..c9976e3 100644
> --- a/arch/arm/mach-tegra/irq.c
> +++ b/arch/arm/mach-tegra/irq.c
> @@ -45,6 +45,8 @@
>
>  #define FIRST_LEGACY_IRQ 32
>
> +#define SGI_MASK 0xFFFF
> +
>  static int num_ictlrs;
>
>  static void __iomem *ictlr_reg_base[] = {
> @@ -55,6 +57,19 @@ static void __iomem *ictlr_reg_base[] = {
>         IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
>  };
>
> +bool tegra_pending_sgi(void)
> +{
> +       u32 pending_set;
> +       void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
> +
> +       pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
> +
> +       if (pending_set & SGI_MASK)
> +               return true;
> +
> +       return false;
> +}
> +
>  static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
>  {
>         void __iomem *base;
> diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
> new file mode 100644
> index 0000000..5142649
> --- /dev/null
> +++ b/arch/arm/mach-tegra/irq.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __TEGRA_IRQ_H
> +#define __TEGRA_IRQ_H
> +
> +bool tegra_pending_sgi(void);
> +
> +#endif
> --
> 1.7.0.4
>

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Colin Cross]

On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> include the power of GIC. That caused the SGI (Software Generated
> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> the "powered-down" CPU idle mode. We need to check if there is any
> pending SGI when go into "powered-down" CPU idle mode. This is important
> especially when applying the coupled cpuidle framework into "power-down"
> cpuidle dirver. Because the coupled cpuidle framework may have the
> chance that misses IPI_SINGLE_FUNC handling sometimes.

This problem exists for any GIC-based SoC, and needs to be fixed in
gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

> For the PPI or SPI, something like the legacy peripheral interrupt. It
> still can be maintained by Tegra legacy interrupt controller. If there
> is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> CPU can be woken up immediately. So we don't need to take care the same
> situation for PPI or SPI.
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * move the static mapping of GIC addr into tegra_pending_sgi
> V2:
> * new in V2
> ---
>  arch/arm/mach-tegra/irq.c |   15 +++++++++++++++
>  arch/arm/mach-tegra/irq.h |   22 ++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/mach-tegra/irq.h
>
> diff --git a/arch/arm/mach-tegra/irq.c b/arch/arm/mach-tegra/irq.c
> index b7886f1..c9976e3 100644
> --- a/arch/arm/mach-tegra/irq.c
> +++ b/arch/arm/mach-tegra/irq.c
> @@ -45,6 +45,8 @@
>
>  #define FIRST_LEGACY_IRQ 32
>
> +#define SGI_MASK 0xFFFF
> +
>  static int num_ictlrs;
>
>  static void __iomem *ictlr_reg_base[] = {
> @@ -55,6 +57,19 @@ static void __iomem *ictlr_reg_base[] = {
>         IO_ADDRESS(TEGRA_QUINARY_ICTLR_BASE),
>  };
>
> +bool tegra_pending_sgi(void)
> +{
> +       u32 pending_set;
> +       void __iomem *distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
> +
> +       pending_set = readl_relaxed(distbase + GIC_DIST_PENDING_SET);
> +
> +       if (pending_set & SGI_MASK)
> +               return true;
> +
> +       return false;
> +}
> +
>  static inline void tegra_irq_write_mask(unsigned int irq, unsigned long reg)
>  {
>         void __iomem *base;
> diff --git a/arch/arm/mach-tegra/irq.h b/arch/arm/mach-tegra/irq.h
> new file mode 100644
> index 0000000..5142649
> --- /dev/null
> +++ b/arch/arm/mach-tegra/irq.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (c) 2012, NVIDIA Corporation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __TEGRA_IRQ_H
> +#define __TEGRA_IRQ_H
> +
> +bool tegra_pending_sgi(void);
> +
> +#endif
> --
> 1.7.0.4
>

Re: [PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Colin Cross]

On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI. The Tegra20 had a limition to
> power down both CPU cores. The secondary CPU must waits for CPU0 in
> powered-down state too. If the secondary CPU be woken up before CPU0
> entering powered-down state, then it needs to restore its CPU states
> and waits for next chance.
>
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".
>
> Based on the work by:
> Colin Cross <ccross@android.com>
> Gary King <gking@nvidia.com>
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * dynamic checking of the number of the state counts
> * fix the code sequence for aborting cpu_suspend in
>   tegra20_sleep_cpu_secondary_finish
> V2:
> * no change
> ---
>  arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
>  arch/arm/mach-tegra/pm.c              |    2 +
>  arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
>  arch/arm/mach-tegra/sleep.h           |   23 +++++
>  4 files changed, 261 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> index d32e8b0..716aef3 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> @@ -22,28 +22,112 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clockchips.h>
>
>  #include <asm/cpuidle.h>
> +#include <asm/proc-fns.h>
> +#include <asm/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra20_idle_lp2(struct cpuidle_device *dev,
> +                           struct cpuidle_driver *drv,
> +                           int index);
> +#endif
> +
> +static struct cpuidle_state tegra_idle_states[] = {
> +       [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> +       [1] = {
> +               .enter                  = tegra20_idle_lp2,
> +               .exit_latency           = 5000,
> +               .target_residency       = 10000,
> +               .power_usage            = 0,
> +               .flags                  = CPUIDLE_FLAG_TIME_VALID,
> +               .name                   = "powered-down",
> +               .desc                   = "CPU power gated",
> +       },
> +#endif
> +};
>
>  static struct cpuidle_driver tegra_idle_driver = {
>         .name = "tegra_idle",
>         .owner = THIS_MODULE,
>         .en_core_tk_irqen = 1,
> -       .state_count = 1,
> -       .states = {
> -               [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> -       },
>  };
>
>  static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
>
> +#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_SMP
> +static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
> +                                        struct cpuidle_driver *drv,
> +                                        int index)
> +{
> +       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> +
> +       cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
> +
> +       tegra20_cpu_clear_resettable();
> +
> +       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> +       return true;
> +}
> +#else
> +static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
> +                                               struct cpuidle_driver *drv,
> +                                               int index)
> +{
> +       return true;
> +}
> +#endif
> +
> +static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
> +                                     struct cpuidle_driver *drv,
> +                                     int index)
> +{
> +       u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
> +       bool entered_lp2 = false;
> +
> +       local_fiq_disable();
> +
> +       tegra_set_cpu_in_lp2(cpu);
> +       cpu_pm_enter();

You must check the return value from cpu_pm_enter and synchronize and
abort both cpus.

> +
> +       if (cpu == 0)
> +               cpu_do_idle();
> +       else
> +               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
> +
> +       cpu_pm_exit();
> +       tegra_clear_cpu_in_lp2(cpu);
> +
> +       local_fiq_enable();
> +
> +       smp_rmb();
> +
> +       return entered_lp2 ? index : 0;
> +}
> +#endif
> +
>  int __init tegra20_cpuidle_init(void)
>  {
> -       int ret;
> +       int ret, i;
>         unsigned int cpu;
>         struct cpuidle_device *dev;
>         struct cpuidle_driver *drv = &tegra_idle_driver;
>
> +       drv->state_count = sizeof(tegra_idle_states) /
> +                          sizeof(struct cpuidle_state);
> +       for (i = 0; i < drv->state_count; i++)
> +               memcpy(&drv->states[i], &tegra_idle_states[i],
> +                      sizeof(struct cpuidle_state));
> +
>         ret = cpuidle_register_driver(&tegra_idle_driver);
>         if (ret) {
>                 pr_err("CPUidle driver registration failed\n");

Is there a call to cpu_cluster_pm_enter/exit somewhere else?

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Colin Cross]

On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI. The Tegra20 had a limition to
> power down both CPU cores. The secondary CPU must waits for CPU0 in
> powered-down state too. If the secondary CPU be woken up before CPU0
> entering powered-down state, then it needs to restore its CPU states
> and waits for next chance.
>
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".
>
> Based on the work by:
> Colin Cross <ccross@android.com>
> Gary King <gking@nvidia.com>
>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * dynamic checking of the number of the state counts
> * fix the code sequence for aborting cpu_suspend in
>   tegra20_sleep_cpu_secondary_finish
> V2:
> * no change
> ---
>  arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
>  arch/arm/mach-tegra/pm.c              |    2 +
>  arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
>  arch/arm/mach-tegra/sleep.h           |   23 +++++
>  4 files changed, 261 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> index d32e8b0..716aef3 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> @@ -22,28 +22,112 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/clockchips.h>
>
>  #include <asm/cpuidle.h>
> +#include <asm/proc-fns.h>
> +#include <asm/suspend.h>
> +#include <asm/smp_plat.h>
> +
> +#include "pm.h"
> +#include "sleep.h"
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra20_idle_lp2(struct cpuidle_device *dev,
> +                           struct cpuidle_driver *drv,
> +                           int index);
> +#endif
> +
> +static struct cpuidle_state tegra_idle_states[] = {
> +       [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> +#ifdef CONFIG_PM_SLEEP
> +       [1] = {
> +               .enter                  = tegra20_idle_lp2,
> +               .exit_latency           = 5000,
> +               .target_residency       = 10000,
> +               .power_usage            = 0,
> +               .flags                  = CPUIDLE_FLAG_TIME_VALID,
> +               .name                   = "powered-down",
> +               .desc                   = "CPU power gated",
> +       },
> +#endif
> +};
>
>  static struct cpuidle_driver tegra_idle_driver = {
>         .name = "tegra_idle",
>         .owner = THIS_MODULE,
>         .en_core_tk_irqen = 1,
> -       .state_count = 1,
> -       .states = {
> -               [0] = ARM_CPUIDLE_WFI_STATE_PWR(600),
> -       },
>  };
>
>  static DEFINE_PER_CPU(struct cpuidle_device, tegra_idle_device);
>
> +#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_SMP
> +static bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
> +                                        struct cpuidle_driver *drv,
> +                                        int index)
> +{
> +       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
> +
> +       cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
> +
> +       tegra20_cpu_clear_resettable();
> +
> +       clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
> +
> +       return true;
> +}
> +#else
> +static inline bool tegra20_idle_enter_lp2_cpu_1(struct cpuidle_device *dev,
> +                                               struct cpuidle_driver *drv,
> +                                               int index)
> +{
> +       return true;
> +}
> +#endif
> +
> +static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
> +                                     struct cpuidle_driver *drv,
> +                                     int index)
> +{
> +       u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
> +       bool entered_lp2 = false;
> +
> +       local_fiq_disable();
> +
> +       tegra_set_cpu_in_lp2(cpu);
> +       cpu_pm_enter();

You must check the return value from cpu_pm_enter and synchronize and
abort both cpus.

> +
> +       if (cpu == 0)
> +               cpu_do_idle();
> +       else
> +               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
> +
> +       cpu_pm_exit();
> +       tegra_clear_cpu_in_lp2(cpu);
> +
> +       local_fiq_enable();
> +
> +       smp_rmb();
> +
> +       return entered_lp2 ? index : 0;
> +}
> +#endif
> +
>  int __init tegra20_cpuidle_init(void)
>  {
> -       int ret;
> +       int ret, i;
>         unsigned int cpu;
>         struct cpuidle_device *dev;
>         struct cpuidle_driver *drv = &tegra_idle_driver;
>
> +       drv->state_count = sizeof(tegra_idle_states) /
> +                          sizeof(struct cpuidle_state);
> +       for (i = 0; i < drv->state_count; i++)
> +               memcpy(&drv->states[i], &tegra_idle_states[i],
> +                      sizeof(struct cpuidle_state));
> +
>         ret = cpuidle_register_driver(&tegra_idle_driver);
>         if (ret) {
>                 pr_err("CPUidle driver registration failed\n");

Is there a call to cpu_cluster_pm_enter/exit somewhere else?

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Tue, 2012-12-18 at 10:42 +0800, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> 
Indeed, but may not need to be taken care for every SoC. It's depend on
HW design. Different SoC had different design about it. For ex, some SoC
only put CPU core to power saving mode not include GIC, or there is
another irq controller can handle the case when CPU go into power saving
mode. Differenc SoC had different usage here (some need to check all
pending irq, some need to check SGI only and some even no need to
consider this). So putting this into to common code may not a good
choice.

Thanks,
Joseph

> > For the PPI or SPI, something like the legacy peripheral interrupt. It
> > still can be maintained by Tegra legacy interrupt controller. If there
> > is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> > CPU can be woken up immediately. So we don't need to take care the same
> > situation for PPI or SPI.
> >
> > Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > V3:
> > * move the static mapping of GIC addr into tegra_pending_sgi
> > V2:
> > * new in V2
> > ---

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Tue, 2012-12-18 at 10:42 +0800, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> 
Indeed, but may not need to be taken care for every SoC. It's depend on
HW design. Different SoC had different design about it. For ex, some SoC
only put CPU core to power saving mode not include GIC, or there is
another irq controller can handle the case when CPU go into power saving
mode. Differenc SoC had different usage here (some need to check all
pending irq, some need to check SGI only and some even no need to
consider this). So putting this into to common code may not a good
choice.

Thanks,
Joseph

> > For the PPI or SPI, something like the legacy peripheral interrupt. It
> > still can be maintained by Tegra legacy interrupt controller. If there
> > is any pending PPI or SPI when CPU in "powered-down" CPU idle mode. The
> > CPU can be woken up immediately. So we don't need to take care the same
> > situation for PPI or SPI.
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
> > V3:
> > * move the static mapping of GIC addr into tegra_pending_sgi
> > V2:
> > * new in V2
> > ---

Re: [PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

On Tue, 2012-12-18 at 10:46 +0800, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI. The Tegra20 had a limition to
> > power down both CPU cores. The secondary CPU must waits for CPU0 in
> > powered-down state too. If the secondary CPU be woken up before CPU0
> > entering powered-down state, then it needs to restore its CPU states
> > and waits for next chance.
> >
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> >
> > Based on the work by:
> > Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> > Gary King <gking-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> >
> > Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> > V3:
> > * dynamic checking of the number of the state counts
> > * fix the code sequence for aborting cpu_suspend in
> >   tegra20_sleep_cpu_secondary_finish
> > V2:
> > * no change
> > ---
> >  arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
> >  arch/arm/mach-tegra/pm.c              |    2 +
> >  arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
> >  arch/arm/mach-tegra/sleep.h           |   23 +++++
> >  4 files changed, 261 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > index d32e8b0..716aef3 100644
> > --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > +static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
> > +                                     struct cpuidle_driver *drv,
> > +                                     int index)
> > +{
> > +       u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
> > +       bool entered_lp2 = false;
> > +
> > +       local_fiq_disable();
> > +
> > +       tegra_set_cpu_in_lp2(cpu);
> > +       cpu_pm_enter();
> 
> You must check the return value from cpu_pm_enter and synchronize and
> abort both cpus.
> 
Please check the sequence in last patch. Although it used the API that
be provided by patch 1 to check the pending sgi. 

> > +
> > +       if (cpu == 0)
> > +               cpu_do_idle();
> > +       else
> > +               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
> > +
> > +       cpu_pm_exit();
> > +       tegra_clear_cpu_in_lp2(cpu);
> > +
> > +       local_fiq_enable();
> > +
> > +       smp_rmb();
> > +
> > +       return entered_lp2 ? index : 0;
> > +}
> > +#endif
> > +
> >  int __init tegra20_cpuidle_init(void)
> >  {
> > -       int ret;
> > +       int ret, i;
> >         unsigned int cpu;
> >         struct cpuidle_device *dev;
> >         struct cpuidle_driver *drv = &tegra_idle_driver;
> >
> > +       drv->state_count = sizeof(tegra_idle_states) /
> > +                          sizeof(struct cpuidle_state);
> > +       for (i = 0; i < drv->state_count; i++)
> > +               memcpy(&drv->states[i], &tegra_idle_states[i],
> > +                      sizeof(struct cpuidle_state));
> > +
> >         ret = cpuidle_register_driver(&tegra_idle_driver);
> >         if (ret) {
> >                 pr_err("CPUidle driver registration failed\n");
> 
> Is there a call to cpu_cluster_pm_enter/exit somewhere else?

Yes, there is. Please check the fuction "tegra_idle_lp2_last" in "pm.c",
because it's a common code of "powered-down" idle state for CPU0 on
Tegra20 & Tegra30.

Thanks,
Joseph

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

On Tue, 2012-12-18 at 10:46 +0800, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI. The Tegra20 had a limition to
> > power down both CPU cores. The secondary CPU must waits for CPU0 in
> > powered-down state too. If the secondary CPU be woken up before CPU0
> > entering powered-down state, then it needs to restore its CPU states
> > and waits for next chance.
> >
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> >
> > Based on the work by:
> > Colin Cross <ccross@android.com>
> > Gary King <gking@nvidia.com>
> >
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
> > V3:
> > * dynamic checking of the number of the state counts
> > * fix the code sequence for aborting cpu_suspend in
> >   tegra20_sleep_cpu_secondary_finish
> > V2:
> > * no change
> > ---
> >  arch/arm/mach-tegra/cpuidle-tegra20.c |   94 ++++++++++++++++++++-
> >  arch/arm/mach-tegra/pm.c              |    2 +
> >  arch/arm/mach-tegra/sleep-tegra20.S   |  147 +++++++++++++++++++++++++++++++++
> >  arch/arm/mach-tegra/sleep.h           |   23 +++++
> >  4 files changed, 261 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > index d32e8b0..716aef3 100644
> > --- a/arch/arm/mach-tegra/cpuidle-tegra20.c
> > +++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
> > +static int __cpuinit tegra20_idle_lp2(struct cpuidle_device *dev,
> > +                                     struct cpuidle_driver *drv,
> > +                                     int index)
> > +{
> > +       u32 cpu = is_smp() ? cpu_logical_map(dev->cpu) : dev->cpu;
> > +       bool entered_lp2 = false;
> > +
> > +       local_fiq_disable();
> > +
> > +       tegra_set_cpu_in_lp2(cpu);
> > +       cpu_pm_enter();
> 
> You must check the return value from cpu_pm_enter and synchronize and
> abort both cpus.
> 
Please check the sequence in last patch. Although it used the API that
be provided by patch 1 to check the pending sgi. 

> > +
> > +       if (cpu == 0)
> > +               cpu_do_idle();
> > +       else
> > +               entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
> > +
> > +       cpu_pm_exit();
> > +       tegra_clear_cpu_in_lp2(cpu);
> > +
> > +       local_fiq_enable();
> > +
> > +       smp_rmb();
> > +
> > +       return entered_lp2 ? index : 0;
> > +}
> > +#endif
> > +
> >  int __init tegra20_cpuidle_init(void)
> >  {
> > -       int ret;
> > +       int ret, i;
> >         unsigned int cpu;
> >         struct cpuidle_device *dev;
> >         struct cpuidle_driver *drv = &tegra_idle_driver;
> >
> > +       drv->state_count = sizeof(tegra_idle_states) /
> > +                          sizeof(struct cpuidle_state);
> > +       for (i = 0; i < drv->state_count; i++)
> > +               memcpy(&drv->states[i], &tegra_idle_states[i],
> > +                      sizeof(struct cpuidle_state));
> > +
> >         ret = cpuidle_register_driver(&tegra_idle_driver);
> >         if (ret) {
> >                 pr_err("CPUidle driver registration failed\n");
> 
> Is there a call to cpu_cluster_pm_enter/exit somewhere else?

Yes, there is. Please check the fuction "tegra_idle_lp2_last" in "pm.c",
because it's a common code of "powered-down" idle state for CPU0 on
Tegra20 & Tegra30.

Thanks,
Joseph

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
cluster is railgated, including the GIC. This causes a pending IPI to be lost.
But for example on OMAP4, only the actual CPU cores are powergated. The GIC
stays alive until also the core domain hits idle. By that time a potential
pending IPI has long woken up the target CPU again, so no additional
checks are needed for functional correct behavior.

Cheers,

Peter.

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > include the power of GIC. That caused the SGI (Software Generated
> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > the "powered-down" CPU idle mode. We need to check if there is any
> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > especially when applying the coupled cpuidle framework into "power-down"
> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> 
> This problem exists for any GIC-based SoC, and needs to be fixed in
> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.

Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
cluster is railgated, including the GIC. This causes a pending IPI to be lost.
But for example on OMAP4, only the actual CPU cores are powergated. The GIC
stays alive until also the core domain hits idle. By that time a potential
pending IPI has long woken up the target CPU again, so no additional
checks are needed for functional correct behavior.

Cheers,

Peter.

Re: [PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Peter De Schrijver]

On Tue, Dec 18, 2012 at 03:31:01AM +0100, Joseph Lo wrote:
> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> core to go into this mode before other core. The coupled cpuidle framework
> can help to sync the MPCore to coupled state then go into "powered-down"
> idle mode together. The driver can just assume the MPCore come into
> "powered-down" mode at the same time. No need to take care if the CPU_0
> goes into this mode along and only can put it into safe idle mode (WFI).
> 
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI for waiting CPU0 in the same state.
> When the CPU0 requests powered-down state, it attempts to put the secondary
> CPU into reset to prevent it from waking up. Then power down both CPUs
> together and power off the cpu rail.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".
> 
> Based on the work by:
> Colin Cross <ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>
> Gary King <gking-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> 
> Signed-off-by: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> V3:
> * sqash last two patch in previous version to support coupled cpuidle
>   directly

nitpick - it should be 'squash'.

Cheers,

Peter.

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Peter De Schrijver]

On Tue, Dec 18, 2012 at 03:31:01AM +0100, Joseph Lo wrote:
> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> core to go into this mode before other core. The coupled cpuidle framework
> can help to sync the MPCore to coupled state then go into "powered-down"
> idle mode together. The driver can just assume the MPCore come into
> "powered-down" mode at the same time. No need to take care if the CPU_0
> goes into this mode along and only can put it into safe idle mode (WFI).
> 
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI for waiting CPU0 in the same state.
> When the CPU0 requests powered-down state, it attempts to put the secondary
> CPU into reset to prevent it from waking up. Then power down both CPUs
> together and power off the cpu rail.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".
> 
> Based on the work by:
> Colin Cross <ccross@android.com>
> Gary King <gking@nvidia.com>
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> V3:
> * sqash last two patch in previous version to support coupled cpuidle
>   directly

nitpick - it should be 'squash'.

Cheers,

Peter.

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Colin Cross]

On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>> > include the power of GIC. That caused the SGI (Software Generated
>> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
>> > the "powered-down" CPU idle mode. We need to check if there is any
>> > pending SGI when go into "powered-down" CPU idle mode. This is important
>> > especially when applying the coupled cpuidle framework into "power-down"
>> > cpuidle dirver. Because the coupled cpuidle framework may have the
>> > chance that misses IPI_SINGLE_FUNC handling sometimes.
>>
>> This problem exists for any GIC-based SoC, and needs to be fixed in
>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>
> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> stays alive until also the core domain hits idle. By that time a potential
> pending IPI has long woken up the target CPU again, so no additional
> checks are needed for functional correct behavior.

I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
rail for the GIC in retention, and I don't think an IPI will wake it
up.  I believe the same problem also exists for Exynos5.  In any case,
checking for an IPI early during idle and aborting won't hurt those
platforms, so I still think it should be in the GIC driver and not by
mapping the GIC registers into a separate driver.

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Colin Cross]

On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>> > include the power of GIC. That caused the SGI (Software Generated
>> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
>> > the "powered-down" CPU idle mode. We need to check if there is any
>> > pending SGI when go into "powered-down" CPU idle mode. This is important
>> > especially when applying the coupled cpuidle framework into "power-down"
>> > cpuidle dirver. Because the coupled cpuidle framework may have the
>> > chance that misses IPI_SINGLE_FUNC handling sometimes.
>>
>> This problem exists for any GIC-based SoC, and needs to be fixed in
>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>
> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> stays alive until also the core domain hits idle. By that time a potential
> pending IPI has long woken up the target CPU again, so no additional
> checks are needed for functional correct behavior.

I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
rail for the GIC in retention, and I don't think an IPI will wake it
up.  I believe the same problem also exists for Exynos5.  In any case,
checking for an IPI early during idle and aborting won't hurt those
platforms, so I still think it should be in the GIC driver and not by
mapping the GIC registers into a separate driver.

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >> > include the power of GIC. That caused the SGI (Software Generated
> >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >> > the "powered-down" CPU idle mode. We need to check if there is any
> >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> >> > especially when applying the coupled cpuidle framework into "power-down"
> >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>
> >> This problem exists for any GIC-based SoC, and needs to be fixed in
> >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >
> > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > stays alive until also the core domain hits idle. By that time a potential
> > pending IPI has long woken up the target CPU again, so no additional
> > checks are needed for functional correct behavior.
> 
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.

Hi Colin,

If I move this code into common GIC driver, should I just take care
pending SGI also?

I will try to create a RFC patch for this.

Thanks,
Joseph

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
> > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >> > include the power of GIC. That caused the SGI (Software Generated
> >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >> > the "powered-down" CPU idle mode. We need to check if there is any
> >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> >> > especially when applying the coupled cpuidle framework into "power-down"
> >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>
> >> This problem exists for any GIC-based SoC, and needs to be fixed in
> >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >
> > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > stays alive until also the core domain hits idle. By that time a potential
> > pending IPI has long woken up the target CPU again, so no additional
> > checks are needed for functional correct behavior.
> 
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.

Hi Colin,

If I move this code into common GIC driver, should I just take care
pending SGI also?

I will try to create a RFC patch for this.

Thanks,
Joseph

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Wed, 2012-12-19 at 09:06 +0800, Joseph Lo wrote:
> On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> > >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > >> > include the power of GIC. That caused the SGI (Software Generated
> > >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > >> > the "powered-down" CPU idle mode. We need to check if there is any
> > >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > >> > especially when applying the coupled cpuidle framework into "power-down"
> > >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> > >>
> > >> This problem exists for any GIC-based SoC, and needs to be fixed in
> > >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> > >
> > > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > > stays alive until also the core domain hits idle. By that time a potential
> > > pending IPI has long woken up the target CPU again, so no additional
> > > checks are needed for functional correct behavior.
> > 
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> 
> If I move this code into common GIC driver, should I just take care
> pending SGI also?
> 
> I will try to create a RFC patch for this.
> 
Hi Colin,

I just checked the scenario to abort cpu_pm_enter when there is any
pending SGI. This may break the current CPU idle driver that already
using cpu_pm_enter. If these drivers didn't handle the error return of
"cpu_pm_enter", the error will cause the CPU_PM_ENTER_FAILED be
triggered. Then the idle code will fail.

The other solution may just like this patch did. Adding an API
"gic_pending_sgi" to check if there is a pending SGI before putting the
CPU into low power mode. But I still don't think this is a common code
to be put there. It's still SoC specific code. So I still prefer the
current solution in this patch for Tegra.

Thanks,
Joseph

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Joseph Lo]

On Wed, 2012-12-19 at 09:06 +0800, Joseph Lo wrote:
> On Wed, 2012-12-19 at 03:36 +0800, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> > > On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> > >> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> > >> > The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> > >> > include the power of GIC. That caused the SGI (Software Generated
> > >> > Interrupt) been lost. Because the SGI can't wake up the CPU that in
> > >> > the "powered-down" CPU idle mode. We need to check if there is any
> > >> > pending SGI when go into "powered-down" CPU idle mode. This is important
> > >> > especially when applying the coupled cpuidle framework into "power-down"
> > >> > cpuidle dirver. Because the coupled cpuidle framework may have the
> > >> > chance that misses IPI_SINGLE_FUNC handling sometimes.
> > >>
> > >> This problem exists for any GIC-based SoC, and needs to be fixed in
> > >> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> > >
> > > Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> > > cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> > > But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> > > stays alive until also the core domain hits idle. By that time a potential
> > > pending IPI has long woken up the target CPU again, so no additional
> > > checks are needed for functional correct behavior.
> > 
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> 
> If I move this code into common GIC driver, should I just take care
> pending SGI also?
> 
> I will try to create a RFC patch for this.
> 
Hi Colin,

I just checked the scenario to abort cpu_pm_enter when there is any
pending SGI. This may break the current CPU idle driver that already
using cpu_pm_enter. If these drivers didn't handle the error return of
"cpu_pm_enter", the error will cause the CPU_PM_ENTER_FAILED be
triggered. Then the idle code will fail.

The other solution may just like this patch did. Adding an API
"gic_pending_sgi" to check if there is a pending SGI before putting the
CPU into low power mode. But I still don't think this is a common code
to be put there. It's still SoC specific code. So I still prefer the
current solution in this patch for Tegra.

Thanks,
Joseph

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>> include the power of GIC. That caused the SGI (Software Generated
>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>
>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>
>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>> stays alive until also the core domain hits idle. By that time a potential
>> pending IPI has long woken up the target CPU again, so no additional
>> checks are needed for functional correct behavior.
>
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.
>
I second Colin on the GIC power states on OMAP.

Regards
Santosh

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>> include the power of GIC. That caused the SGI (Software Generated
>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>
>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>
>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>> stays alive until also the core domain hits idle. By that time a potential
>> pending IPI has long woken up the target CPU again, so no additional
>> checks are needed for functional correct behavior.
>
> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> rail for the GIC in retention, and I don't think an IPI will wake it
> up.  I believe the same problem also exists for Exynos5.  In any case,
> checking for an IPI early during idle and aborting won't hurt those
> platforms, so I still think it should be in the GIC driver and not by
> mapping the GIC registers into a separate driver.
>
I second Colin on the GIC power states on OMAP.

Regards
Santosh

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>> include the power of GIC. That caused the SGI (Software Generated
> >>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>
> >>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>
> >> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >> stays alive until also the core domain hits idle. By that time a potential
> >> pending IPI has long woken up the target CPU again, so no additional
> >> checks are needed for functional correct behavior.
> >
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> >
> I second Colin on the GIC power states on OMAP.

But even then PRCM will only trigger the transition to retention voltage after
both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
PRCM starts the transition.

Cheers,

Peter.

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> > On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> > <pdeschrijver@nvidia.com> wrote:
> >> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>> include the power of GIC. That caused the SGI (Software Generated
> >>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>
> >>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>
> >> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >> stays alive until also the core domain hits idle. By that time a potential
> >> pending IPI has long woken up the target CPU again, so no additional
> >> checks are needed for functional correct behavior.
> >
> > I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> > rail for the GIC in retention, and I don't think an IPI will wake it
> > up.  I believe the same problem also exists for Exynos5.  In any case,
> > checking for an IPI early during idle and aborting won't hurt those
> > platforms, so I still think it should be in the GIC driver and not by
> > mapping the GIC registers into a separate driver.
> >
> I second Colin on the GIC power states on OMAP.

But even then PRCM will only trigger the transition to retention voltage after
both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
PRCM starts the transition.

Cheers,

Peter.

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>
>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>
>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>> stays alive until also the core domain hits idle. By that time a potential
>>>> pending IPI has long woken up the target CPU again, so no additional
>>>> checks are needed for functional correct behavior.
>>>
>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>> checking for an IPI early during idle and aborting won't hurt those
>>> platforms, so I still think it should be in the GIC driver and not by
>>> mapping the GIC registers into a separate driver.
>>>
>> I second Colin on the GIC power states on OMAP.
>
> But even then PRCM will only trigger the transition to retention voltage after
> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> PRCM starts the transition.
>
No it won't. You are talking about the voltage domain retention while
the point is SGI becomes useless once CPU power domain transition to
low power state. And btw, voltage transition also can not happen just
with two CPU's in retention. It does have dependency like cluster power 
state to hit retention.

CPUs have their own power domain state and once they enter into any low
power state apart from ON power state, SGI doesn't work. hope this is
clear.

Regards
Santosh

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>> <pdeschrijver@nvidia.com> wrote:
>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>
>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>
>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>> stays alive until also the core domain hits idle. By that time a potential
>>>> pending IPI has long woken up the target CPU again, so no additional
>>>> checks are needed for functional correct behavior.
>>>
>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>> checking for an IPI early during idle and aborting won't hurt those
>>> platforms, so I still think it should be in the GIC driver and not by
>>> mapping the GIC registers into a separate driver.
>>>
>> I second Colin on the GIC power states on OMAP.
>
> But even then PRCM will only trigger the transition to retention voltage after
> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> PRCM starts the transition.
>
No it won't. You are talking about the voltage domain retention while
the point is SGI becomes useless once CPU power domain transition to
low power state. And btw, voltage transition also can not happen just
with two CPU's in retention. It does have dependency like cluster power 
state to hit retention.

CPUs have their own power domain state and once they enter into any low
power state apart from ON power state, SGI doesn't work. hope this is
clear.

Regards
Santosh

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> > On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> >> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> >>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> >>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>>>> include the power of GIC. That caused the SGI (Software Generated
> >>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>>>
> >>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>>>
> >>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >>>> stays alive until also the core domain hits idle. By that time a potential
> >>>> pending IPI has long woken up the target CPU again, so no additional
> >>>> checks are needed for functional correct behavior.
> >>>
> >>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> >>> rail for the GIC in retention, and I don't think an IPI will wake it
> >>> up.  I believe the same problem also exists for Exynos5.  In any case,
> >>> checking for an IPI early during idle and aborting won't hurt those
> >>> platforms, so I still think it should be in the GIC driver and not by
> >>> mapping the GIC registers into a separate driver.
> >>>
> >> I second Colin on the GIC power states on OMAP.
> >
> > But even then PRCM will only trigger the transition to retention voltage after
> > both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> > PRCM starts the transition.
> >
> No it won't. You are talking about the voltage domain retention while
> the point is SGI becomes useless once CPU power domain transition to
> low power state. And btw, voltage transition also can not happen just
> with two CPU's in retention. It does have dependency like cluster power 
> state to hit retention.
> 
> CPUs have their own power domain state and once they enter into any low
> power state apart from ON power state, SGI doesn't work. hope this is
> clear.

So why does OMAP4 have working coupled idle without the SGI check then?

Cheers,

Peter.

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
> > On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
> >> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
> >>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
> >>> <pdeschrijver@nvidia.com> wrote:
> >>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
> >>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
> >>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
> >>>>>> include the power of GIC. That caused the SGI (Software Generated
> >>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
> >>>>>> the "powered-down" CPU idle mode. We need to check if there is any
> >>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
> >>>>>> especially when applying the coupled cpuidle framework into "power-down"
> >>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
> >>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
> >>>>>
> >>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
> >>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
> >>>>
> >>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
> >>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
> >>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
> >>>> stays alive until also the core domain hits idle. By that time a potential
> >>>> pending IPI has long woken up the target CPU again, so no additional
> >>>> checks are needed for functional correct behavior.
> >>>
> >>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
> >>> rail for the GIC in retention, and I don't think an IPI will wake it
> >>> up.  I believe the same problem also exists for Exynos5.  In any case,
> >>> checking for an IPI early during idle and aborting won't hurt those
> >>> platforms, so I still think it should be in the GIC driver and not by
> >>> mapping the GIC registers into a separate driver.
> >>>
> >> I second Colin on the GIC power states on OMAP.
> >
> > But even then PRCM will only trigger the transition to retention voltage after
> > both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
> > PRCM starts the transition.
> >
> No it won't. You are talking about the voltage domain retention while
> the point is SGI becomes useless once CPU power domain transition to
> low power state. And btw, voltage transition also can not happen just
> with two CPU's in retention. It does have dependency like cluster power 
> state to hit retention.
> 
> CPUs have their own power domain state and once they enter into any low
> power state apart from ON power state, SGI doesn't work. hope this is
> clear.

So why does OMAP4 have working coupled idle without the SGI check then?

Cheers,

Peter.

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 03:29 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
>> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
>>> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>>>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>>>> <pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>>>
>>>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>>>
>>>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>>>> stays alive until also the core domain hits idle. By that time a potential
>>>>>> pending IPI has long woken up the target CPU again, so no additional
>>>>>> checks are needed for functional correct behavior.
>>>>>
>>>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>>>> checking for an IPI early during idle and aborting won't hurt those
>>>>> platforms, so I still think it should be in the GIC driver and not by
>>>>> mapping the GIC registers into a separate driver.
>>>>>
>>>> I second Colin on the GIC power states on OMAP.
>>>
>>> But even then PRCM will only trigger the transition to retention voltage after
>>> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
>>> PRCM starts the transition.
>>>
>> No it won't. You are talking about the voltage domain retention while
>> the point is SGI becomes useless once CPU power domain transition to
>> low power state. And btw, voltage transition also can not happen just
>> with two CPU's in retention. It does have dependency like cluster power
>> state to hit retention.
>>
>> CPUs have their own power domain state and once they enter into any low
>> power state apart from ON power state, SGI doesn't work. hope this is
>> clear.
>
> So why does OMAP4 have working coupled idle without the SGI check then?
>
Because whenever the OMAP4 CPUs enter into any power states apart from
ON, clock-domain force wakeup method us being used to wakeup
secondary CPUs. And secondly CPU RET is not supported on purpose because
of some known IP block issues.

Regards
Santosh

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 03:29 PM, Peter De Schrijver wrote:
> On Thu, Dec 20, 2012 at 10:49:57AM +0100, Santosh Shilimkar wrote:
>> On Thursday 20 December 2012 03:04 PM, Peter De Schrijver wrote:
>>> On Thu, Dec 20, 2012 at 08:16:33AM +0100, Santosh Shilimkar wrote:
>>>> On Wednesday 19 December 2012 01:06 AM, Colin Cross wrote:
>>>>> On Tue, Dec 18, 2012 at 2:15 AM, Peter De Schrijver
>>>>> <pdeschrijver@nvidia.com> wrote:
>>>>>> On Tue, Dec 18, 2012 at 03:42:24AM +0100, Colin Cross wrote:
>>>>>>> On Mon, Dec 17, 2012 at 6:30 PM, Joseph Lo <josephl@nvidia.com> wrote:
>>>>>>>> The "powered-down" CPU idle mode of Tegra cut off the vdd_cpu rail, it
>>>>>>>> include the power of GIC. That caused the SGI (Software Generated
>>>>>>>> Interrupt) been lost. Because the SGI can't wake up the CPU that in
>>>>>>>> the "powered-down" CPU idle mode. We need to check if there is any
>>>>>>>> pending SGI when go into "powered-down" CPU idle mode. This is important
>>>>>>>> especially when applying the coupled cpuidle framework into "power-down"
>>>>>>>> cpuidle dirver. Because the coupled cpuidle framework may have the
>>>>>>>> chance that misses IPI_SINGLE_FUNC handling sometimes.
>>>>>>>
>>>>>>> This problem exists for any GIC-based SoC, and needs to be fixed in
>>>>>>> gic_cpu_save or gic_dist_save, whichever one loses the interrupt.
>>>>>>
>>>>>> Not necessarily. It depends on the SoC design. On Tegra20, the entire CPU
>>>>>> cluster is railgated, including the GIC. This causes a pending IPI to be lost.
>>>>>> But for example on OMAP4, only the actual CPU cores are powergated. The GIC
>>>>>> stays alive until also the core domain hits idle. By that time a potential
>>>>>> pending IPI has long woken up the target CPU again, so no additional
>>>>>> checks are needed for functional correct behavior.
>>>>>
>>>>> I'm not sure that is correct for OMAP4.  C2 and C3 will put the power
>>>>> rail for the GIC in retention, and I don't think an IPI will wake it
>>>>> up.  I believe the same problem also exists for Exynos5.  In any case,
>>>>> checking for an IPI early during idle and aborting won't hurt those
>>>>> platforms, so I still think it should be in the GIC driver and not by
>>>>> mapping the GIC registers into a separate driver.
>>>>>
>>>> I second Colin on the GIC power states on OMAP.
>>>
>>> But even then PRCM will only trigger the transition to retention voltage after
>>> both CPUs hit off mode. I guess a pending IPI will wakeup the core before the
>>> PRCM starts the transition.
>>>
>> No it won't. You are talking about the voltage domain retention while
>> the point is SGI becomes useless once CPU power domain transition to
>> low power state. And btw, voltage transition also can not happen just
>> with two CPU's in retention. It does have dependency like cluster power
>> state to hit retention.
>>
>> CPUs have their own power domain state and once they enter into any low
>> power state apart from ON power state, SGI doesn't work. hope this is
>> clear.
>
> So why does OMAP4 have working coupled idle without the SGI check then?
>
Because whenever the OMAP4 CPUs enter into any power states apart from
ON, clock-domain force wakeup method us being used to wakeup
secondary CPUs. And secondly CPU RET is not supported on purpose because
of some known IP block issues.

Regards
Santosh

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

> >
> > So why does OMAP4 have working coupled idle without the SGI check then?
> >
> Because whenever the OMAP4 CPUs enter into any power states apart from
> ON, clock-domain force wakeup method us being used to wakeup
> secondary CPUs. And secondly CPU RET is not supported on purpose because
> of some known IP block issues.
> 

Aha. That explains indeed why you don't need the check.

Cheers,

Peter.

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Peter De Schrijver]

> >
> > So why does OMAP4 have working coupled idle without the SGI check then?
> >
> Because whenever the OMAP4 CPUs enter into any power states apart from
> ON, clock-domain force wakeup method us being used to wakeup
> secondary CPUs. And secondly CPU RET is not supported on purpose because
> of some known IP block issues.
> 

Aha. That explains indeed why you don't need the check.

Cheers,

Peter.

Re: [PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 04:44 PM, Peter De Schrijver wrote:
>>>
>>> So why does OMAP4 have working coupled idle without the SGI check then?
>>>
>> Because whenever the OMAP4 CPUs enter into any power states apart from
>> ON, clock-domain force wakeup method us being used to wakeup
>> secondary CPUs. And secondly CPU RET is not supported on purpose because
>> of some known IP block issues.
>>
>
> Aha. That explains indeed why you don't need the check.
>
Just for the information on OMAP5 SOC, CPU RET is supported and the
couple idle works on it without IPI. As I mentioned, we do use
clock-domain force wakeup method for wakeup. Code is out of the
tree hence i didn't mention that in first place.

Regards
Santosh

[PATCH V3 1/5] ARM: tegra: add pending SGI checking API

[Santosh Shilimkar]

On Thursday 20 December 2012 04:44 PM, Peter De Schrijver wrote:
>>>
>>> So why does OMAP4 have working coupled idle without the SGI check then?
>>>
>> Because whenever the OMAP4 CPUs enter into any power states apart from
>> ON, clock-domain force wakeup method us being used to wakeup
>> secondary CPUs. And secondly CPU RET is not supported on purpose because
>> of some known IP block issues.
>>
>
> Aha. That explains indeed why you don't need the check.
>
Just for the information on OMAP5 SOC, CPU RET is supported and the
couple idle works on it without IPI. As I mentioned, we do use
clock-domain force wakeup method for wakeup. Code is out of the
tree hence i didn't mention that in first place.

Regards
Santosh

Re: [PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI. The Tegra20 had a limition to
> power down both CPU cores. The secondary CPU must waits for CPU0 in
> powered-down state too. If the secondary CPU be woken up before CPU0
> entering powered-down state, then it needs to restore its CPU states
> and waits for next chance.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".

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

>  int __init tegra20_cpuidle_init(void)

> +	drv->state_count = sizeof(tegra_idle_states) /
> +			   sizeof(struct cpuidle_state);

Use ARRAY_SIZE() there?


> +	for (i = 0; i < drv->state_count; i++)
> +		memcpy(&drv->states[i], &tegra_idle_states[i],
> +		       sizeof(struct cpuidle_state));

Can't you call memcpy() once:

memcpy(drv->states, tegra_idle_states,
		drv->state_count * sizeof(drv->states[0]));

... although I personally much preferred when all this was just static
initialization directly in tegra_idle_driver, rather than all this messy
copying. Really, struct cpuidle_driver should point at the array, rather
than including it.

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

> @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
>  
>  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
>  		last_cpu = true;
> +	else if (phy_cpu_id == 1)
> +		tegra20_cpu_set_resettable_soon();
>  
>  	spin_unlock(&tegra_lp2_lock);
>  	return last_cpu;

Shouldn't the code in that else branch have a run-time check for whether
it's running on Tegra20? When compiled without Tegra20 support,
tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
both Tegra20 and Tegra30 are compiled in, isn't that code going to run
when it shouldn't; pm.c being a common file.

(Peter again, can you please review/ack this series. Thanks)

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI. The Tegra20 had a limition to
> power down both CPU cores. The secondary CPU must waits for CPU0 in
> powered-down state too. If the secondary CPU be woken up before CPU0
> entering powered-down state, then it needs to restore its CPU states
> and waits for next chance.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".

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

>  int __init tegra20_cpuidle_init(void)

> +	drv->state_count = sizeof(tegra_idle_states) /
> +			   sizeof(struct cpuidle_state);

Use ARRAY_SIZE() there?


> +	for (i = 0; i < drv->state_count; i++)
> +		memcpy(&drv->states[i], &tegra_idle_states[i],
> +		       sizeof(struct cpuidle_state));

Can't you call memcpy() once:

memcpy(drv->states, tegra_idle_states,
		drv->state_count * sizeof(drv->states[0]));

... although I personally much preferred when all this was just static
initialization directly in tegra_idle_driver, rather than all this messy
copying. Really, struct cpuidle_driver should point at the array, rather
than including it.

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

> @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
>  
>  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
>  		last_cpu = true;
> +	else if (phy_cpu_id == 1)
> +		tegra20_cpu_set_resettable_soon();
>  
>  	spin_unlock(&tegra_lp2_lock);
>  	return last_cpu;

Shouldn't the code in that else branch have a run-time check for whether
it's running on Tegra20? When compiled without Tegra20 support,
tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
both Tegra20 and Tegra30 are compiled in, isn't that code going to run
when it shouldn't; pm.c being a common file.

(Peter again, can you please review/ack this series. Thanks)

Re: [PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> functions were used for CPU powered-down state maintenance.

I think this isn't adding those functions into tegra_cpu_car_ops, but
rather implementing the already existing function pointers for Tegra20,
right?

This patch touches the clock driver and Prashant will soon be posting
patches that significantly rework the clock driver. Those will conflict.
How have you worked with Prashant to resolve the conflicts? Will
Prashant's patches be based on top of this series? Please explicitly
describe the dependencies and resolution of conflicts when you post the
next version. (below the ---, along with the changelog)

[PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> functions were used for CPU powered-down state maintenance.

I think this isn't adding those functions into tegra_cpu_car_ops, but
rather implementing the already existing function pointers for Tegra20,
right?

This patch touches the clock driver and Prashant will soon be posting
patches that significantly rework the clock driver. Those will conflict.
How have you worked with Prashant to resolve the conflicts? Will
Prashant's patches be based on top of this series? Please explicitly
describe the dependencies and resolution of conflicts when you post the
next version. (below the ---, along with the changelog)

Re: [PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Stephen Warren]

On 12/17/2012 07:31 PM, Joseph Lo wrote:
> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> core to go into this mode before other core. The coupled cpuidle framework
> can help to sync the MPCore to coupled state then go into "powered-down"
> idle mode together. The driver can just assume the MPCore come into
> "powered-down" mode at the same time. No need to take care if the CPU_0
> goes into this mode along and only can put it into safe idle mode (WFI).
> 
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI for waiting CPU0 in the same state.
> When the CPU0 requests powered-down state, it attempts to put the secondary
> CPU into reset to prevent it from waking up. Then power down both CPUs
> together and power off the cpu rail.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".

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

> +static void tegra20_wake_reset_cpu_1(void)

Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
more descriptive name? I assume the function works on CPU1, which is
assumed to be in reset, and removes reset from the CPU so it boots.

> @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
>  	for_each_possible_cpu(cpu) {
>  		dev = &per_cpu(tegra_idle_device, cpu);
>  		dev->cpu = cpu;
> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> +		dev->coupled_cpus = *cpu_online_mask;
> +#endif

That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
enabled for this file to be compiled. So, you can drop the ifdef, and
make the code uncondtional.

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Stephen Warren]

On 12/17/2012 07:31 PM, Joseph Lo wrote:
> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> core to go into this mode before other core. The coupled cpuidle framework
> can help to sync the MPCore to coupled state then go into "powered-down"
> idle mode together. The driver can just assume the MPCore come into
> "powered-down" mode at the same time. No need to take care if the CPU_0
> goes into this mode along and only can put it into safe idle mode (WFI).
> 
> The powered-down state of Tegra20 requires power gating both CPU cores.
> When the secondary CPU requests to enter powered-down state, it saves
> its own contexts and then enters WFI for waiting CPU0 in the same state.
> When the CPU0 requests powered-down state, it attempts to put the secondary
> CPU into reset to prevent it from waking up. Then power down both CPUs
> together and power off the cpu rail.
> 
> Be aware of that, you may see the legacy power state "LP2" in the code
> which is exactly the same meaning of "CPU power down".

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

> +static void tegra20_wake_reset_cpu_1(void)

Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
more descriptive name? I assume the function works on CPU1, which is
assumed to be in reset, and removes reset from the CPU so it boots.

> @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
>  	for_each_possible_cpu(cpu) {
>  		dev = &per_cpu(tegra_idle_device, cpu);
>  		dev->cpu = cpu;
> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> +		dev->coupled_cpus = *cpu_online_mask;
> +#endif

That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
enabled for this file to be compiled. So, you can drop the ifdef, and
make the code uncondtional.

Re: [PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Joseph Lo]

On Fri, 2012-12-21 at 01:46 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> > functions were used for CPU powered-down state maintenance.
> 
> I think this isn't adding those functions into tegra_cpu_car_ops, but
> rather implementing the already existing function pointers for Tegra20,
> right?
> 
Yes, exactly.

> This patch touches the clock driver and Prashant will soon be posting
> patches that significantly rework the clock driver. Those will conflict.
> How have you worked with Prashant to resolve the conflicts? Will
> Prashant's patches be based on top of this series? Please explicitly
> describe the dependencies and resolution of conflicts when you post the
> next version. (below the ---, along with the changelog)
> 
Yes, there are many patch series may have dependency each other
recently. We also want to know what the sequence you expect to merge the
patch series to your tree. Evan my own patch series had dependency too.
For ex, hotplug enhancement, cpuidle, SMP works on UP and
suspend/resume. Currently, I think we are all work the the top of
linux-next tree. If we can't expect the sequence of the patch series to
be merged, we will always have some conflicts with each other.

Depend on the timing, I think it should be better to know which patch
series should be going first or recently and which should go later and
depend on which series. Then we can rearrange our patches on top of it.
And reducing the job to fix conflict on your side.

What do you think, Prashant? If your rework of clock driver will show up
soon and Stephen also expect that will be merged first, I am fine to
re-base on it and re-send my next version after new clock driver be
done.

Thanks,
Joseph

[PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Joseph Lo]

On Fri, 2012-12-21 at 01:46 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
> > functions were used for CPU powered-down state maintenance.
> 
> I think this isn't adding those functions into tegra_cpu_car_ops, but
> rather implementing the already existing function pointers for Tegra20,
> right?
> 
Yes, exactly.

> This patch touches the clock driver and Prashant will soon be posting
> patches that significantly rework the clock driver. Those will conflict.
> How have you worked with Prashant to resolve the conflicts? Will
> Prashant's patches be based on top of this series? Please explicitly
> describe the dependencies and resolution of conflicts when you post the
> next version. (below the ---, along with the changelog)
> 
Yes, there are many patch series may have dependency each other
recently. We also want to know what the sequence you expect to merge the
patch series to your tree. Evan my own patch series had dependency too.
For ex, hotplug enhancement, cpuidle, SMP works on UP and
suspend/resume. Currently, I think we are all work the the top of
linux-next tree. If we can't expect the sequence of the patch series to
be merged, we will always have some conflicts with each other.

Depend on the timing, I think it should be better to know which patch
series should be going first or recently and which should go later and
depend on which series. Then we can rearrange our patches on top of it.
And reducing the job to fix conflict on your side.

What do you think, Prashant? If your rework of clock driver will show up
soon and Stephen also expect that will be merged first, I am fine to
re-base on it and re-send my next version after new clock driver be
done.

Thanks,
Joseph

Re: [PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

On Fri, 2012-12-21 at 01:43 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI. The Tegra20 had a limition to
> > power down both CPU cores. The secondary CPU must waits for CPU0 in
> > powered-down state too. If the secondary CPU be woken up before CPU0
> > entering powered-down state, then it needs to restore its CPU states
> > and waits for next chance.
> > 
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> 
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> 
> >  int __init tegra20_cpuidle_init(void)
> 
> > +	drv->state_count = sizeof(tegra_idle_states) /
> > +			   sizeof(struct cpuidle_state);
> 
> Use ARRAY_SIZE() there?
> 
OK.
> 
> > +	for (i = 0; i < drv->state_count; i++)
> > +		memcpy(&drv->states[i], &tegra_idle_states[i],
> > +		       sizeof(struct cpuidle_state));
> 
> Can't you call memcpy() once:
> 
> memcpy(drv->states, tegra_idle_states,
> 		drv->state_count * sizeof(drv->states[0]));
> 
> ... although I personally much preferred when all this was just static
> initialization directly in tegra_idle_driver, rather than all this messy
> copying. Really, struct cpuidle_driver should point at the array, rather
> than including it.
> 
I think so. If you strongly prefer the original style, I can rollback to
the previous version here.

> > diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> 
> > @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
> >  
> >  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
> >  		last_cpu = true;
> > +	else if (phy_cpu_id == 1)
> > +		tegra20_cpu_set_resettable_soon();
> >  
> >  	spin_unlock(&tegra_lp2_lock);
> >  	return last_cpu;
> 
> Shouldn't the code in that else branch have a run-time check for whether
> it's running on Tegra20? When compiled without Tegra20 support,
> tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
> both Tegra20 and Tegra30 are compiled in, isn't that code going to run
> when it shouldn't; pm.c being a common file.
> 
Because the code didn't hurt Tegra30, so I didn't add a runtime
detection there. If you have concern, I can add runtime detection there.

Thanks,
Joseph

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Joseph Lo]

On Fri, 2012-12-21 at 01:43 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI. The Tegra20 had a limition to
> > power down both CPU cores. The secondary CPU must waits for CPU0 in
> > powered-down state too. If the secondary CPU be woken up before CPU0
> > entering powered-down state, then it needs to restore its CPU states
> > and waits for next chance.
> > 
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> 
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> 
> >  int __init tegra20_cpuidle_init(void)
> 
> > +	drv->state_count = sizeof(tegra_idle_states) /
> > +			   sizeof(struct cpuidle_state);
> 
> Use ARRAY_SIZE() there?
> 
OK.
> 
> > +	for (i = 0; i < drv->state_count; i++)
> > +		memcpy(&drv->states[i], &tegra_idle_states[i],
> > +		       sizeof(struct cpuidle_state));
> 
> Can't you call memcpy() once:
> 
> memcpy(drv->states, tegra_idle_states,
> 		drv->state_count * sizeof(drv->states[0]));
> 
> ... although I personally much preferred when all this was just static
> initialization directly in tegra_idle_driver, rather than all this messy
> copying. Really, struct cpuidle_driver should point at the array, rather
> than including it.
> 
I think so. If you strongly prefer the original style, I can rollback to
the previous version here.

> > diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
> 
> > @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
> >  
> >  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
> >  		last_cpu = true;
> > +	else if (phy_cpu_id == 1)
> > +		tegra20_cpu_set_resettable_soon();
> >  
> >  	spin_unlock(&tegra_lp2_lock);
> >  	return last_cpu;
> 
> Shouldn't the code in that else branch have a run-time check for whether
> it's running on Tegra20? When compiled without Tegra20 support,
> tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
> both Tegra20 and Tegra30 are compiled in, isn't that code going to run
> when it shouldn't; pm.c being a common file.
> 
Because the code didn't hurt Tegra30, so I didn't add a runtime
detection there. If you have concern, I can add runtime detection there.

Thanks,
Joseph

Re: [PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Joseph Lo]

On Fri, 2012-12-21 at 01:54 +0800, Stephen Warren wrote:
> On 12/17/2012 07:31 PM, Joseph Lo wrote:
> > The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> > core to go into this mode before other core. The coupled cpuidle framework
> > can help to sync the MPCore to coupled state then go into "powered-down"
> > idle mode together. The driver can just assume the MPCore come into
> > "powered-down" mode at the same time. No need to take care if the CPU_0
> > goes into this mode along and only can put it into safe idle mode (WFI).
> > 
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI for waiting CPU0 in the same state.
> > When the CPU0 requests powered-down state, it attempts to put the secondary
> > CPU into reset to prevent it from waking up. Then power down both CPUs
> > together and power off the cpu rail.
> > 
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> 
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> 
> > +static void tegra20_wake_reset_cpu_1(void)
> 
> Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
> more descriptive name? I assume the function works on CPU1, which is
> assumed to be in reset, and removes reset from the CPU so it boots.
> 
Yes.
Do you mean this function running on CPU1? It runs on CPU0 to wake up
CPU1. About the function name, it looks more appropriate.

> > @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
> >  	for_each_possible_cpu(cpu) {
> >  		dev = &per_cpu(tegra_idle_device, cpu);
> >  		dev->cpu = cpu;
> > +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> > +		dev->coupled_cpus = *cpu_online_mask;
> > +#endif
> 
> That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
> enabled for this file to be compiled. So, you can drop the ifdef, and
> make the code uncondtional.

No, I found I need this if I want to make the device works on UP case.
Then I need to do something like below, when we disable SMP.

>	config ARCH_TEGRA_2x_SOC 
>	bool "Enable support for Tegra20 family" 
>+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 
>	select ARCH_REQUIRE_GPIOLIB 
>	select ARM_ERRATA_720789 
>	select ARM_ERRATA_742230 

This will be updated in next version as well.

Thanks,
Joseph

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Joseph Lo]

On Fri, 2012-12-21 at 01:54 +0800, Stephen Warren wrote:
> On 12/17/2012 07:31 PM, Joseph Lo wrote:
> > The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
> > core to go into this mode before other core. The coupled cpuidle framework
> > can help to sync the MPCore to coupled state then go into "powered-down"
> > idle mode together. The driver can just assume the MPCore come into
> > "powered-down" mode at the same time. No need to take care if the CPU_0
> > goes into this mode along and only can put it into safe idle mode (WFI).
> > 
> > The powered-down state of Tegra20 requires power gating both CPU cores.
> > When the secondary CPU requests to enter powered-down state, it saves
> > its own contexts and then enters WFI for waiting CPU0 in the same state.
> > When the CPU0 requests powered-down state, it attempts to put the secondary
> > CPU into reset to prevent it from waking up. Then power down both CPUs
> > together and power off the cpu rail.
> > 
> > Be aware of that, you may see the legacy power state "LP2" in the code
> > which is exactly the same meaning of "CPU power down".
> 
> > diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
> 
> > +static void tegra20_wake_reset_cpu_1(void)
> 
> Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
> more descriptive name? I assume the function works on CPU1, which is
> assumed to be in reset, and removes reset from the CPU so it boots.
> 
Yes.
Do you mean this function running on CPU1? It runs on CPU0 to wake up
CPU1. About the function name, it looks more appropriate.

> > @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
> >  	for_each_possible_cpu(cpu) {
> >  		dev = &per_cpu(tegra_idle_device, cpu);
> >  		dev->cpu = cpu;
> > +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
> > +		dev->coupled_cpus = *cpu_online_mask;
> > +#endif
> 
> That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
> enabled for this file to be compiled. So, you can drop the ifdef, and
> make the code uncondtional.

No, I found I need this if I want to make the device works on UP case.
Then I need to do something like below, when we disable SMP.

>	config ARCH_TEGRA_2x_SOC 
>	bool "Enable support for Tegra20 family" 
>+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 
>	select ARCH_REQUIRE_GPIOLIB 
>	select ARM_ERRATA_720789 
>	select ARM_ERRATA_742230 

This will be updated in next version as well.

Thanks,
Joseph

Re: [PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Stephen Warren]

On 12/20/2012 11:36 PM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:43 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:30 PM, Joseph Lo wrote:
>>> The powered-down state of Tegra20 requires power gating both CPU cores.
>>> When the secondary CPU requests to enter powered-down state, it saves
>>> its own contexts and then enters WFI. The Tegra20 had a limition to
>>> power down both CPU cores. The secondary CPU must waits for CPU0 in
>>> powered-down state too. If the secondary CPU be woken up before CPU0
>>> entering powered-down state, then it needs to restore its CPU states
>>> and waits for next chance.
>>>
>>> Be aware of that, you may see the legacy power state "LP2" in the code
>>> which is exactly the same meaning of "CPU power down".

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

[ code to set up the idle state array]

>> ... although I personally much preferred when all this was just static
>> initialization directly in tegra_idle_driver, rather than all this messy
>> copying. Really, struct cpuidle_driver should point at the array, rather
>> than including it.
>>
> I think so. If you strongly prefer the original style, I can rollback to
> the previous version here.

I suppose there's little point changing it back; I know others objected
to the original code:-(

>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
>>>  
>>>  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
>>>  		last_cpu = true;
>>> +	else if (phy_cpu_id == 1)
>>> +		tegra20_cpu_set_resettable_soon();
>>>  
>>>  	spin_unlock(&tegra_lp2_lock);
>>>  	return last_cpu;
>>
>> Shouldn't the code in that else branch have a run-time check for whether
>> it's running on Tegra20? When compiled without Tegra20 support,
>> tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
>> both Tegra20 and Tegra30 are compiled in, isn't that code going to run
>> when it shouldn't; pm.c being a common file.
>
> Because the code didn't hurt Tegra30, so I didn't add a runtime
> detection there. If you have concern, I can add runtime detection there.

The only issue I see that could happen is that
tegra20_cpu_set_resettable_soon() writes to some location to maintain
the CPU resettable state. Since I assume that location isn't used for
that purpose on Tegra30, could this cause some conflict? It seems best
to add the check to make sure there's no issue here.

[PATCH V3 2/5] ARM: tegra20: cpuidle: add powered-down state for secondary CPU

[Stephen Warren]

On 12/20/2012 11:36 PM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:43 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:30 PM, Joseph Lo wrote:
>>> The powered-down state of Tegra20 requires power gating both CPU cores.
>>> When the secondary CPU requests to enter powered-down state, it saves
>>> its own contexts and then enters WFI. The Tegra20 had a limition to
>>> power down both CPU cores. The secondary CPU must waits for CPU0 in
>>> powered-down state too. If the secondary CPU be woken up before CPU0
>>> entering powered-down state, then it needs to restore its CPU states
>>> and waits for next chance.
>>>
>>> Be aware of that, you may see the legacy power state "LP2" in the code
>>> which is exactly the same meaning of "CPU power down".

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

[ code to set up the idle state array]

>> ... although I personally much preferred when all this was just static
>> initialization directly in tegra_idle_driver, rather than all this messy
>> copying. Really, struct cpuidle_driver should point at the array, rather
>> than including it.
>>
> I think so. If you strongly prefer the original style, I can rollback to
> the previous version here.

I suppose there's little point changing it back; I know others objected
to the original code:-(

>>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>>
>>> @@ -173,6 +173,8 @@ bool __cpuinit tegra_set_cpu_in_lp2(int phy_cpu_id)
>>>  
>>>  	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
>>>  		last_cpu = true;
>>> +	else if (phy_cpu_id == 1)
>>> +		tegra20_cpu_set_resettable_soon();
>>>  
>>>  	spin_unlock(&tegra_lp2_lock);
>>>  	return last_cpu;
>>
>> Shouldn't the code in that else branch have a run-time check for whether
>> it's running on Tegra20? When compiled without Tegra20 support,
>> tegra20_cpu_set_resettable_soon() is a dummy static inline, but when
>> both Tegra20 and Tegra30 are compiled in, isn't that code going to run
>> when it shouldn't; pm.c being a common file.
>
> Because the code didn't hurt Tegra30, so I didn't add a runtime
> detection there. If you have concern, I can add runtime detection there.

The only issue I see that could happen is that
tegra20_cpu_set_resettable_soon() writes to some location to maintain
the CPU resettable state. Since I assume that location isn't used for
that purpose on Tegra30, could this cause some conflict? It seems best
to add the check to make sure there's no issue here.

Re: [PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Stephen Warren]

On 12/21/2012 12:10 AM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:54 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:31 PM, Joseph Lo wrote:
>>> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
>>> core to go into this mode before other core. The coupled cpuidle framework
>>> can help to sync the MPCore to coupled state then go into "powered-down"
>>> idle mode together. The driver can just assume the MPCore come into
>>> "powered-down" mode at the same time. No need to take care if the CPU_0
>>> goes into this mode along and only can put it into safe idle mode (WFI).
>>>
>>> The powered-down state of Tegra20 requires power gating both CPU cores.
>>> When the secondary CPU requests to enter powered-down state, it saves
>>> its own contexts and then enters WFI for waiting CPU0 in the same state.
>>> When the CPU0 requests powered-down state, it attempts to put the secondary
>>> CPU into reset to prevent it from waking up. Then power down both CPUs
>>> together and power off the cpu rail.
>>>
>>> Be aware of that, you may see the legacy power state "LP2" in the code
>>> which is exactly the same meaning of "CPU power down".
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
>>
>>> +static void tegra20_wake_reset_cpu_1(void)
>>
>> Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
>> more descriptive name? I assume the function works on CPU1, which is
>> assumed to be in reset, and removes reset from the CPU so it boots.
>>
> Yes.
> Do you mean this function running on CPU1? It runs on CPU0 to wake up
> CPU1. About the function name, it looks more appropriate.

Sorry for bikeshedding even more, perhaps
tegra20_wake_cpu_1_from_reset(); that makes the word-order better English.

>>> @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
>>>  	for_each_possible_cpu(cpu) {
>>>  		dev = &per_cpu(tegra_idle_device, cpu);
>>>  		dev->cpu = cpu;
>>> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>>> +		dev->coupled_cpus = *cpu_online_mask;
>>> +#endif
>>
>> That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
>> enabled for this file to be compiled. So, you can drop the ifdef, and
>> make the code uncondtional.
> 
> No, I found I need this if I want to make the device works on UP case.
> Then I need to do something like below, when we disable SMP.
> 
>> 	config ARCH_TEGRA_2x_SOC 
>> 	bool "Enable support for Tegra20 family" 
>> +	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 
>> 	select ARCH_REQUIRE_GPIOLIB 
>> 	select ARM_ERRATA_720789 
>> 	select ARM_ERRATA_742230 
> 
> This will be updated in next version as well.

OK, if the Kconfig looks like that, you're right - the ifdef in the .c
file is needed.

[PATCH V3 5/5] ARM: tegra20: cpuidle: apply coupled cpuidle for powered-down mode

[Stephen Warren]

On 12/21/2012 12:10 AM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:54 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:31 PM, Joseph Lo wrote:
>>> The "powered-down" cpuidle mode of Tegra20 needs the CPU0 be the last one
>>> core to go into this mode before other core. The coupled cpuidle framework
>>> can help to sync the MPCore to coupled state then go into "powered-down"
>>> idle mode together. The driver can just assume the MPCore come into
>>> "powered-down" mode at the same time. No need to take care if the CPU_0
>>> goes into this mode along and only can put it into safe idle mode (WFI).
>>>
>>> The powered-down state of Tegra20 requires power gating both CPU cores.
>>> When the secondary CPU requests to enter powered-down state, it saves
>>> its own contexts and then enters WFI for waiting CPU0 in the same state.
>>> When the CPU0 requests powered-down state, it attempts to put the secondary
>>> CPU into reset to prevent it from waking up. Then power down both CPUs
>>> together and power off the cpu rail.
>>>
>>> Be aware of that, you may see the legacy power state "LP2" in the code
>>> which is exactly the same meaning of "CPU power down".
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
>>
>>> +static void tegra20_wake_reset_cpu_1(void)
>>
>> Nit/bikeshe: I think tegra20_wake_from_reset_cpu_1() might be a slightly
>> more descriptive name? I assume the function works on CPU1, which is
>> assumed to be in reset, and removes reset from the CPU so it boots.
>>
> Yes.
> Do you mean this function running on CPU1? It runs on CPU0 to wake up
> CPU1. About the function name, it looks more appropriate.

Sorry for bikeshedding even more, perhaps
tegra20_wake_cpu_1_from_reset(); that makes the word-order better English.

>>> @@ -137,6 +241,9 @@ int __init tegra20_cpuidle_init(void)
>>>  	for_each_possible_cpu(cpu) {
>>>  		dev = &per_cpu(tegra_idle_device, cpu);
>>>  		dev->cpu = cpu;
>>> +#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
>>> +		dev->coupled_cpus = *cpu_online_mask;
>>> +#endif
>>
>> That CONFIG option is selected by ARCH_TEGRA_2x_SOC, which must be
>> enabled for this file to be compiled. So, you can drop the ifdef, and
>> make the code uncondtional.
> 
> No, I found I need this if I want to make the device works on UP case.
> Then I need to do something like below, when we disable SMP.
> 
>> 	config ARCH_TEGRA_2x_SOC 
>> 	bool "Enable support for Tegra20 family" 
>> +	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP 
>> 	select ARCH_REQUIRE_GPIOLIB 
>> 	select ARM_ERRATA_720789 
>> 	select ARM_ERRATA_742230 
> 
> This will be updated in next version as well.

OK, if the Kconfig looks like that, you're right - the ifdef in the .c
file is needed.

Re: [PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Stephen Warren]

On 12/20/2012 10:02 PM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:46 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:30 PM, Joseph Lo wrote:
>>> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
>>> functions were used for CPU powered-down state maintenance.
>>
>> I think this isn't adding those functions into tegra_cpu_car_ops, but
>> rather implementing the already existing function pointers for Tegra20,
>> right?
>>
> Yes, exactly.
> 
>> This patch touches the clock driver and Prashant will soon be posting
>> patches that significantly rework the clock driver. Those will conflict.
>> How have you worked with Prashant to resolve the conflicts? Will
>> Prashant's patches be based on top of this series? Please explicitly
>> describe the dependencies and resolution of conflicts when you post the
>> next version. (below the ---, along with the changelog)
>>
> Yes, there are many patch series may have dependency each other
> recently. We also want to know what the sequence you expect to merge the
> patch series to your tree.

To be honest, I don't know what order I'd like to merge things yet.
There are so many conflicting/dependant patches for 3.9 flying around
I'd need to re-read them all and think it through in quite some detail...

The common clock rework is something quite important. I'd certainly like
to get that in early. That most likely implies everything else should be
rebased on top of it.

The best thing would be if all patch authors take a look at Prashant's
clock patches (and all other patches) and work out what has dependencies
on what, and co-ordinate with each-other which order is the easiest to
merge things in. Once 3.8-rc1 is out and I can actually start merging
things, I'd like to see a list of outstanding patches from each person,
with all dependencies explicitly listed. I'll use that information to
decide which order to merge in.

Of course I just realized that Prashant hasn't posted his clock patches
upstream yet, so it's a little difficult for anyone to rebase on top of
them yet, but hopefully that will be resolved very soon now...

[PATCH V3 3/5] ARM: tegra20: clocks: add CPU low-power function into tegra_cpu_car_ops

[Stephen Warren]

On 12/20/2012 10:02 PM, Joseph Lo wrote:
> On Fri, 2012-12-21 at 01:46 +0800, Stephen Warren wrote:
>> On 12/17/2012 07:30 PM, Joseph Lo wrote:
>>> Add suspend, resume and rail_off_ready API into tegra_cpu_car_ops. These
>>> functions were used for CPU powered-down state maintenance.
>>
>> I think this isn't adding those functions into tegra_cpu_car_ops, but
>> rather implementing the already existing function pointers for Tegra20,
>> right?
>>
> Yes, exactly.
> 
>> This patch touches the clock driver and Prashant will soon be posting
>> patches that significantly rework the clock driver. Those will conflict.
>> How have you worked with Prashant to resolve the conflicts? Will
>> Prashant's patches be based on top of this series? Please explicitly
>> describe the dependencies and resolution of conflicts when you post the
>> next version. (below the ---, along with the changelog)
>>
> Yes, there are many patch series may have dependency each other
> recently. We also want to know what the sequence you expect to merge the
> patch series to your tree.

To be honest, I don't know what order I'd like to merge things yet.
There are so many conflicting/dependant patches for 3.9 flying around
I'd need to re-read them all and think it through in quite some detail...

The common clock rework is something quite important. I'd certainly like
to get that in early. That most likely implies everything else should be
rebased on top of it.

The best thing would be if all patch authors take a look at Prashant's
clock patches (and all other patches) and work out what has dependencies
on what, and co-ordinate with each-other which order is the easiest to
merge things in. Once 3.8-rc1 is out and I can actually start merging
things, I'd like to see a list of outstanding patches from each person,
with all dependencies explicitly listed. I'll use that information to
decide which order to merge in.

Of course I just realized that Prashant hasn't posted his clock patches
upstream yet, so it's a little difficult for anyone to rebase on top of
them yet, but hopefully that will be resolved very soon now...

Re: [PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> This adds a "powered-down" state in cpuidle for Tegra20. It requires power
> gating both CPU cores. When the CPU1 requests to enter "powered-down"
> state, it saves its own state and then enters WFI. When the CPU0 requests
> the same state, it attempts to put the CPU1 into reset to prevent it from
> waking up. Then power down both CPUs together and turn off the CPU rail.
> 
> If the CPU1 be woken up before CPU0 entering powered-down state, then it
> needs to restore it's CPU state and waits for next chance.

Joseph,

What's the status of this series? It looks like there are quite a few
comments on V3; I assume you'll post a new version to address those.
Will you base the new version on top of Prashant's common clock cleanup
to remove the conflicts with that?

[PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Stephen Warren]

On 12/17/2012 07:30 PM, Joseph Lo wrote:
> This adds a "powered-down" state in cpuidle for Tegra20. It requires power
> gating both CPU cores. When the CPU1 requests to enter "powered-down"
> state, it saves its own state and then enters WFI. When the CPU0 requests
> the same state, it attempts to put the CPU1 into reset to prevent it from
> waking up. Then power down both CPUs together and turn off the CPU rail.
> 
> If the CPU1 be woken up before CPU0 entering powered-down state, then it
> needs to restore it's CPU state and waits for next chance.

Joseph,

What's the status of this series? It looks like there are quite a few
comments on V3; I assume you'll post a new version to address those.
Will you base the new version on top of Prashant's common clock cleanup
to remove the conflicts with that?

Re: [PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Joseph Lo]

On Thu, 2013-01-03 at 03:05 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > This adds a "powered-down" state in cpuidle for Tegra20. It requires power
> > gating both CPU cores. When the CPU1 requests to enter "powered-down"
> > state, it saves its own state and then enters WFI. When the CPU0 requests
> > the same state, it attempts to put the CPU1 into reset to prevent it from
> > waking up. Then power down both CPUs together and turn off the CPU rail.
> > 
> > If the CPU1 be woken up before CPU0 entering powered-down state, then it
> > needs to restore it's CPU state and waits for next chance.
> 
> Joseph,
> 
> What's the status of this series? It looks like there are quite a few
> comments on V3; I assume you'll post a new version to address those.
> Will you base the new version on top of Prashant's common clock cleanup
> to remove the conflicts with that?
> 

Yes, I will re-base on top of Prashant's CCF rework in the next version.
But there are some issues, I am co-working with Prashant to make sure
every thing is work as usual. Will re-post this series later after
everything be settled down.

Thanks,
Joseph

[PATCH V3 0/5] ARM: tegra20: cpuidle: add power-down state

[Joseph Lo]

On Thu, 2013-01-03 at 03:05 +0800, Stephen Warren wrote:
> On 12/17/2012 07:30 PM, Joseph Lo wrote:
> > This adds a "powered-down" state in cpuidle for Tegra20. It requires power
> > gating both CPU cores. When the CPU1 requests to enter "powered-down"
> > state, it saves its own state and then enters WFI. When the CPU0 requests
> > the same state, it attempts to put the CPU1 into reset to prevent it from
> > waking up. Then power down both CPUs together and turn off the CPU rail.
> > 
> > If the CPU1 be woken up before CPU0 entering powered-down state, then it
> > needs to restore it's CPU state and waits for next chance.
> 
> Joseph,
> 
> What's the status of this series? It looks like there are quite a few
> comments on V3; I assume you'll post a new version to address those.
> Will you base the new version on top of Prashant's common clock cleanup
> to remove the conflicts with that?
> 

Yes, I will re-base on top of Prashant's CCF rework in the next version.
But there are some issues, I am co-working with Prashant to make sure
every thing is work as usual. Will re-post this series later after
everything be settled down.

Thanks,
Joseph