ARM: tegra114: add support for system suspend

Quoted 7 lines · Joseph Lo

Adding a flag for tegra_disable_clean_inv_dcache to flush cache as LoUIS or ALL. After this patch, the v7_flush_dcache_louis is used for CPU hotplug and CPU suspend in CPU power down (e.g. CPU idle power-down mode) case. And the v7_flush_dcache_all is used for CPU cluster power down (e.g. suspend to LP2 mode).

arch/arm/mach-tegra/sleep.h +0 -0

arch/arm/mach-tegra/sleep.h line 46

Quoted 3 lines

arch/arm/mach-tegra/sleep.h +3 -0

+/* flag of tegra_disable_clean_inv_dcache to do LoUIS or all */
+#define TEGRA_FLUSH_CACHE_LOUIS	0x10
+#define TEGRA_FLUSH_CACHE_ALL	0x20

Is there a reason those two values aren't just 0 and 1; they only seem to be used internally to switch between two function calls, and not passed to those calls or written to HW.

arch/arm/mach-tegra/pmc.c void tegra_pmc_pm_set(enum tegra_suspend_mode mode) · line 207

Quoted 8 lines · Joseph Lo

When the last CPU core in suspend, the CPU power rail can be turned off by setting flags to flow controller. Then the flow controller will inform PMC to turn off the CPU rail when the last CPU goes into suspend.

arch/arm/mach-tegra/pmc.c +2 -0

 
+	/* Turn off CRAIL */
+	if (tegra_chip_id == TEGRA114) {

Presumably this new code is needed on Tegra114 and chips after it, so not needing it is the exception? If so, I'd suggest the following instead, so the code doesn't have to be modified for later chips:

switch (tegra_chip_id) {
case TEGRA20:
case TEGRA30:

	break;
default:
	the code you added
	break;
}

Hooking tegra_tear_down_cpu for Tegra114 for supporting cluster power down when CPU cluster suspneded in LP2.

Signed-off-by Joseph Lo <[email protected]>

---
 arch/arm/mach-tegra/pm.c | 1 +
 1 file changed, 1 insertion(+)

arch/arm/mach-tegra/pm.c +0 -0

index 94e69be..a0668a2 100644

* PGP Signed by an unknown key

Quoted 8 lines · Joseph Lo

Hooking tegra_tear_down_cpu for Tegra114 for supporting cluster power down when CPU cluster suspneded in LP2.

Signed-off-by Joseph Lo <[email protected]>

---
 arch/arm/mach-tegra/pm.c | 1 +
 1 file changed, 1 insertion(+)

arch/arm/mach-tegra/pm.c static void tegra_tear_down_cpu_init(void) · line 57

Quoted 11 lines

arch/arm/mach-tegra/pm.c +1 -0

index 94e69be..a0668a2 100644
@@ -52,6 +52,7 @@ static void tegra_tear_down_cpu_init(void)
 			tegra_tear_down_cpu = tegra20_tear_down_cpu;
 		break;
 	case TEGRA30:
+	case TEGRA114:
 		if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC))
 			tegra_tear_down_cpu = tegra30_tear_down_cpu;

This is getting a little weird. Suppose I want to build a Tegra114 only kernel. With the above code it means the tegra_tear_down_cpu won't be hooked because ARCH_TEGRA_3x_SOC isn't selected.

Perhaps tegra30_tear_down_cpu() should be built unconditionally so that it's always available?

There are other alternatives like adding another Kconfig symbol which doesn't cover all of Tegra30 but only code shared with Tegra114 (and possible future chips) or building sleep-tegra30.S if either one of the ARCH_TEGRA_3x_SOC or ARCH_TEGRA_114_SOC symbols is selected.

Quoted 8 lines · Joseph Lo

Hooking tegra_tear_down_cpu for Tegra114 for supporting cluster power down when CPU cluster suspneded in LP2.

Signed-off-by Joseph Lo <[email protected]> ---

arch/arm/mach-tegra/pm.c | 1 + 1 file changed, 1 insertion(+)

arch/arm/mach-tegra/pm.c +0 -0

index 94e69be..a0668a2 100644 --- a/arch/arm/mach-tegra/pm.c +++

Quoted 5 lines

b/arch/arm/mach-tegra/pm.c @@ -52,6 +52,7 @@ static void tegra_tear_down_cpu_init(void) tegra_tear_down_cpu =
tegra20_tear_down_cpu; break; case TEGRA30: + case TEGRA114: if
(IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC)) tegra_tear_down_cpu = tegra30_tear_down_cpu;

This is getting a little weird. Suppose I want to build a Tegra114 only kernel. With the above code it means the tegra_tear_down_cpu won't be hooked because ARCH_TEGRA_3x_SOC isn't selected.

Perhaps tegra30_tear_down_cpu() should be built unconditionally so that it's always available? I suspect that something similar will need to be done for future chips too, further complicating matters.

There are other alternatives like adding another Kconfig symbol which doesn't cover all of Tegra30 but only code shared with Tegra114 (and possible future chips) or building sleep-tegra30.S if either one of the ARCH_TEGRA_3x_SOC or ARCH_TEGRA_114_SOC symbols is selected.

Quoted 24 lines · Thierry Reding

Quoted 8 lines · Joseph Lo

Hooking tegra_tear_down_cpu for Tegra114 for supporting cluster power down when CPU cluster suspneded in LP2.

Signed-off-by Joseph Lo <[email protected]> ---

arch/arm/mach-tegra/pm.c | 1 + 1 file changed, 1 insertion(+)

arch/arm/mach-tegra/pm.c +0 -0

index 94e69be..a0668a2 100644 --- a/arch/arm/mach-tegra/pm.c +++

Quoted 5 lines

b/arch/arm/mach-tegra/pm.c @@ -52,6 +52,7 @@ static void tegra_tear_down_cpu_init(void) tegra_tear_down_cpu =
tegra20_tear_down_cpu; break; case TEGRA30: + case TEGRA114: if
(IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC)) tegra_tear_down_cpu = tegra30_tear_down_cpu;

This is getting a little weird. Suppose I want to build a Tegra114 only kernel. With the above code it means the tegra_tear_down_cpu won't be hooked because ARCH_TEGRA_3x_SOC isn't selected.

Perhaps tegra30_tear_down_cpu() should be built unconditionally so that it's always available? I suspect that something similar will need to be done for future chips too, further complicating matters.

Quoted 5 lines

There are other alternatives like adding another Kconfig symbol which doesn't cover all of Tegra30 but only code shared with Tegra114 (and possible future chips) or building sleep-tegra30.S if either one of the ARCH_TEGRA_3x_SOC or ARCH_TEGRA_114_SOC symbols is selected.

To be honest, I wonder if we should just get rid of ARCH_TEGRA_*_SOC, and build everything if ARCH_TEGRA is defined. tegra_defconfig enables all ARCH_TEGRA_*_SOC anyway, and I'm afraid I don't test other configurations very often, and I assume that distros will enable everything...

Adding suspend/resume function for tegra_cpu_car_ops. We only save and restore the setting of the clock of CoreSight. Other clocks still need to be taken care by clock driver.

Quoted 3 lines · Joseph Lo

Adding suspend/resume function for tegra_cpu_car_ops. We only save and restore the setting of the clock of CoreSight. Other clocks still need to be taken care by clock driver.

Mike, can I get an ack to take this through the Tegra tree with the rest of this series, for 3.12. Thanks.

(Nit: There's a typo in the patch subject here too.)

Quoted 1 line

drivers/clk/tegra/clk-tegra114.c +0 -0

drivers/clk/tegra/clk-tegra114.c static void tegra114_disable_cpu_clock(u32 cpu) · line 2122

Quoted 7 lines

drivers/clk/tegra/clk-tegra114.c +7 -0

+#ifdef CONFIG_PM_SLEEP
+static void tegra114_cpu_clock_suspend(void)
+{
+	/* switch coresite to clk_m, save off original source */
+	tegra114_cpu_clk_sctx.clk_csite_src =
+				readl(clk_base + CLK_SOURCE_CSITE);
+	writel(3<<30, clk_base + CLK_SOURCE_CSITE);

Don't you need spaces around the << to satisfy checkpatch/coding style?

Adding suspend/resume function for tegra_cpu_car_ops. We only save and restore the setting of the clock of CoreSight. Other clocks still need to be taken care by clock driver.

Cc Mike Turquette <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

---
 drivers/clk/tegra/clk-tegra114.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

drivers/clk/tegra/clk-tegra114.c +26 -0

index dbd0428..cd51d75 100644
@@ -258,6 +258,12 @@
 #define PMC_PLLM_WB0_OVERRIDE 0x1dc
 #define PMC_PLLM_WB0_OVERRIDE_2 0x2b0
 
+#ifdef CONFIG_PM_SLEEP
+static struct cpu_clk_suspend_context {
+       u32 clk_csite_src;
+} tegra114_cpu_clk_sctx;
+#endif
+
 static int periph_clk_enb_refcnt[CLK_OUT_ENB_NUM * 32];
 
 static void __iomem *clk_base;

@@ -2107,9 +2113,29 @@ static void tegra114_disable_cpu_clock(u32 cpu)
        /* flow controller would take care in the power sequence. */
 }
 
+#ifdef CONFIG_PM_SLEEP
+static void tegra114_cpu_clock_suspend(void)
+{
+       /* switch coresite to clk_m, save off original source */
+       tegra114_cpu_clk_sctx.clk_csite_src =
+                               readl(clk_base + CLK_SOURCE_CSITE);
+       writel(3<<30, clk_base + CLK_SOURCE_CSITE);
+}
+
+static void tegra114_cpu_clock_resume(void)
+{
+       writel(tegra114_cpu_clk_sctx.clk_csite_src,
+                                       clk_base + CLK_SOURCE_CSITE);
+}
+#endif
+
 static struct tegra_cpu_car_ops tegra114_cpu_car_ops = {
        .wait_for_reset = tegra114_wait_cpu_in_reset,
        .disable_clock  = tegra114_disable_cpu_clock,
+#ifdef CONFIG_PM_SLEEP
+       .suspend        = tegra114_cpu_clock_suspend,
+       .resume         = tegra114_cpu_clock_resume,
+#endif
 };
 
 static const struct of_device_id pmc_match[] __initconst = {

Signature · 1 line

1.8.3.1

Quoting Joseph Lo (2013-06-25 02:27:52)

Quoted 6 lines

Adding suspend/resume function for tegra_cpu_car_ops. We only save and restore the setting of the clock of CoreSight. Other clocks still need to be taken care by clock driver.

Cc Mike Turquette <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

Change looks OK to me, besides Stephen's comments. I was waiting for a V2 to hit the list but I never saw it. Any plans?

Quoted 8 lines · Mike Turquette

Quoting Joseph Lo (2013-06-25 02:27:52)

Quoted 6 lines

Adding suspend/resume function for tegra_cpu_car_ops. We only save and restore the setting of the clock of CoreSight. Other clocks still need to be taken care by clock driver.

Cc Mike Turquette <[email protected]>

Signed-off-by Joseph Lo <[email protected]>

Quoted 3 lines

Change looks OK to me, besides Stephen's comments. I was waiting for a V2 to hit the list but I never saw it. Any plans?

Hi Mike,

The V2 was sent on 2013/7/3.

Thanks,
Joseph

* PGP Signed by an unknown key

There's a typo in the subject, it should be prefixed with "ARM: tegra:".

There's a typo in the subject, it should be prefixed with "ARM:
tegra:".

The Tegra114 can support suspend function now, removing the limitation.

Quoted 1 line · Joseph Lo

The Tegra114 can support suspend function now, removing the limitation.

OK, so this is the first point at which system suspend is enabled on Tegra114. However, some of the changes in earlier patches sounded like the might affect, or be required by, cpuidle too. Is that the case? If so, can you assure me that bisectability of Tegra114 cpuidle is maintained across this whole series?