[PATCH 0/4] ARM: tegra: cpuidle: some clean up for CPUidle driver

[PATCH 0/4] ARM: tegra: cpuidle: some clean up for CPUidle driver

[Joseph Lo]

This series has some clean ups for Tegra CPUidle drivers. Also make it
more gereric.

Tested on Seaboard, Cardhu and Dalmore.

Joseph Lo (4):
  ARM: tegra: cpuidle: move the init function behind the suspend init
    function
  ARM: tegra: hook tegra_tear_down_cpu function in the PM suspend init
    function
  ARM: tegra: cpuidle: using IS_ENABLED for multi SoCs management in
    init func
  ARM: tegra: check processor id in tegra_{set,clear}_cpu_in_lp2

 arch/arm/mach-tegra/common.c          |  2 ++
 arch/arm/mach-tegra/cpuidle-tegra20.c | 10 +++-------
 arch/arm/mach-tegra/cpuidle-tegra30.c | 10 +++-------
 arch/arm/mach-tegra/cpuidle.c         | 27 +++++++--------------------
 arch/arm/mach-tegra/cpuidle.h         | 15 +++------------
 arch/arm/mach-tegra/pm.c              | 19 +++++++++++++++----
 arch/arm/mach-tegra/pm.h              |  4 ++--
 7 files changed, 35 insertions(+), 52 deletions(-)

-- 
1.8.3

[PATCH 1/4] ARM: tegra: cpuidle: move the init function behind the suspend init function

[Joseph Lo]

One of the state of CPUidle on Tegra can power gate the CPU and the
vdd_cpu rail. But it depends on some configurations from DT and a common
hook function for different Tegra SoCs to power gate the CPU rail. And
these stuffs are initialized after common Tegra suspend init function. So
we move the CPUidle init behind the suspend init function. And making the
CPUidle driver more generic.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/common.c  |  2 ++
 arch/arm/mach-tegra/cpuidle.c | 14 ++++----------
 arch/arm/mach-tegra/cpuidle.h |  6 ++++++
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 9f852c6..ec5836b 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -29,6 +29,7 @@
 
 #include "board.h"
 #include "common.h"
+#include "cpuidle.h"
 #include "fuse.h"
 #include "iomap.h"
 #include "irq.h"
@@ -108,5 +109,6 @@ void __init tegra_init_early(void)
 void __init tegra_init_late(void)
 {
 	tegra_init_suspend();
+	tegra_cpuidle_init();
 	tegra_powergate_debugfs_init();
 }
diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index 4b744c4..4a7a788 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -27,25 +27,19 @@
 #include "fuse.h"
 #include "cpuidle.h"
 
-static int __init tegra_cpuidle_init(void)
+void __init tegra_cpuidle_init(void)
 {
-	int ret;
-
 	switch (tegra_chip_id) {
 	case TEGRA20:
-		ret = tegra20_cpuidle_init();
+		tegra20_cpuidle_init();
 		break;
 	case TEGRA30:
-		ret = tegra30_cpuidle_init();
+		tegra30_cpuidle_init();
 		break;
 	case TEGRA114:
-		ret = tegra114_cpuidle_init();
+		tegra114_cpuidle_init();
 		break;
 	default:
-		ret = -ENODEV;
 		break;
 	}
-
-	return ret;
 }
-device_initcall(tegra_cpuidle_init);
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index d733f75..9bcf61f 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -35,4 +35,10 @@ int tegra114_cpuidle_init(void);
 static inline int tegra114_cpuidle_init(void) { return -ENODEV; }
 #endif
 
+#ifdef CONFIG_CPU_IDLE
+void tegra_cpuidle_init(void);
+#else
+static inline void tegra_cpuidle_init(void) {}
+#endif
+
 #endif
-- 
1.8.3

[PATCH 2/4] ARM: tegra: hook tegra_tear_down_cpu function in the PM suspend init function

[Joseph Lo]

The tegra_tear_down_cpu was used to cut off the CPU rail for various Tegra
SoCs. Hooking it in the PM suspend init function and making the CPUidle
driver more generic.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 3 ---
 arch/arm/mach-tegra/cpuidle-tegra30.c | 3 ---
 arch/arm/mach-tegra/pm.c              | 9 +++++++++
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 0cdba8d..d667513 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -214,8 +214,5 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 
 int __init tegra20_cpuidle_init(void)
 {
-#ifdef CONFIG_PM_SLEEP
-	tegra_tear_down_cpu = tegra20_tear_down_cpu;
-#endif
 	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
 }
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 3cf9aca..9d9f554 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -146,8 +146,5 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 
 int __init tegra30_cpuidle_init(void)
 {
-#ifdef CONFIG_PM_SLEEP
-	tegra_tear_down_cpu = tegra30_tear_down_cpu;
-#endif
 	return cpuidle_register(&tegra_idle_driver, NULL);
 }
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 45cf52c..779dbb4 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -44,6 +44,14 @@
 static DEFINE_SPINLOCK(tegra_lp2_lock);
 void (*tegra_tear_down_cpu)(void);
 
+static void tegra_tear_down_cpu_init(void)
+{
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && tegra_chip_id == TEGRA20)
+		tegra_tear_down_cpu = tegra20_tear_down_cpu;
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) && tegra_chip_id == TEGRA30)
+		tegra_tear_down_cpu = tegra30_tear_down_cpu;
+}
+
 /*
  * restore_cpu_complex
  *
@@ -224,6 +232,7 @@ void __init tegra_init_suspend(void)
 	if (tegra_pmc_get_suspend_mode() == TEGRA_SUSPEND_NONE)
 		return;
 
+	tegra_tear_down_cpu_init();
 	tegra_pmc_suspend_init();
 
 	suspend_set_ops(&tegra_suspend_ops);
-- 
1.8.3

[PATCH 2/4] ARM: tegra: hook tegra_tear_down_cpu function in the PM suspend init function

[Stephen Warren]

On 06/03/2013 05:17 AM, Joseph Lo wrote:
> The tegra_tear_down_cpu was used to cut off the CPU rail for various Tegra
> SoCs. Hooking it in the PM suspend init function and making the CPUidle
> driver more generic.

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

> +static void tegra_tear_down_cpu_init(void)
> +{
> +	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && tegra_chip_id == TEGRA20)
> +		tegra_tear_down_cpu = tegra20_tear_down_cpu;
> +	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) && tegra_chip_id == TEGRA30)
> +		tegra_tear_down_cpu = tegra30_tear_down_cpu;
> +}

I think you should use a switch statement there instead:

switch (tegra_chip_id) {
case TEGRA20:
	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
		tegra_tear_down_cpu = tegra20_tear_down_cpu;
	break;
...

Similar for patch 3/4. This makes it a lot more obvious that it's
selecting an option based on the runtime SoC, rather than just a bunch
of if conditions that each may or may-not be doing similar things.

[PATCH 3/4] ARM: tegra: cpuidle: using IS_ENABLED for multi SoCs management in init func

[Joseph Lo]

Clean up the Tegra CPUidle init function by using IS_ENABLED for multi
SoCs management in the init function.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/cpuidle.c | 13 +++----------
 arch/arm/mach-tegra/cpuidle.h | 17 +----------------
 2 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle.c b/arch/arm/mach-tegra/cpuidle.c
index 4a7a788..55e88c8 100644
--- a/arch/arm/mach-tegra/cpuidle.c
+++ b/arch/arm/mach-tegra/cpuidle.c
@@ -29,17 +29,10 @@
 
 void __init tegra_cpuidle_init(void)
 {
-	switch (tegra_chip_id) {
-	case TEGRA20:
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) && tegra_chip_id == TEGRA20)
 		tegra20_cpuidle_init();
-		break;
-	case TEGRA30:
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) && tegra_chip_id == TEGRA30)
 		tegra30_cpuidle_init();
-		break;
-	case TEGRA114:
+	if (IS_ENABLED(CONFIG_ARCH_TEGRA_114_SOC) && tegra_chip_id == TEGRA114)
 		tegra114_cpuidle_init();
-		break;
-	default:
-		break;
-	}
 }
diff --git a/arch/arm/mach-tegra/cpuidle.h b/arch/arm/mach-tegra/cpuidle.h
index 9bcf61f..9ec2c1a 100644
--- a/arch/arm/mach-tegra/cpuidle.h
+++ b/arch/arm/mach-tegra/cpuidle.h
@@ -17,25 +17,10 @@
 #ifndef __MACH_TEGRA_CPUIDLE_H
 #define __MACH_TEGRA_CPUIDLE_H
 
-#ifdef CONFIG_ARCH_TEGRA_2x_SOC
+#ifdef CONFIG_CPU_IDLE
 int tegra20_cpuidle_init(void);
-#else
-static inline int tegra20_cpuidle_init(void) { return -ENODEV; }
-#endif
-
-#ifdef CONFIG_ARCH_TEGRA_3x_SOC
 int tegra30_cpuidle_init(void);
-#else
-static inline int tegra30_cpuidle_init(void) { return -ENODEV; }
-#endif
-
-#ifdef CONFIG_ARCH_TEGRA_114_SOC
 int tegra114_cpuidle_init(void);
-#else
-static inline int tegra114_cpuidle_init(void) { return -ENODEV; }
-#endif
-
-#ifdef CONFIG_CPU_IDLE
 void tegra_cpuidle_init(void);
 #else
 static inline void tegra_cpuidle_init(void) {}
-- 
1.8.3

[PATCH 4/4] ARM: tegra: check processor id in tegra_{set, clear}_cpu_in_lp2

[Joseph Lo]

The tegra_set_cpu_in_lp2 was used in the Tegra reset handler to know the
CPU were coming up with which state then redirect to the appropriate
handler. So we need to pass the processor id every time when we use the
function. Now moving the code into the function.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
 arch/arm/mach-tegra/cpuidle-tegra20.c |  7 +++----
 arch/arm/mach-tegra/cpuidle-tegra30.c |  7 +++----
 arch/arm/mach-tegra/pm.c              | 10 ++++++----
 arch/arm/mach-tegra/pm.h              |  4 ++--
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index d667513..706aa42 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -177,7 +177,6 @@ static int 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())
@@ -193,16 +192,16 @@ static int tegra20_idle_lp2_coupled(struct cpuidle_device *dev,
 
 	local_fiq_disable();
 
-	tegra_set_cpu_in_lp2(cpu);
+	tegra_set_cpu_in_lp2();
 	cpu_pm_enter();
 
-	if (cpu == 0)
+	if (dev->cpu == 0)
 		entered_lp2 = tegra20_cpu_cluster_power_down(dev, drv, index);
 	else
 		entered_lp2 = tegra20_idle_enter_lp2_cpu_1(dev, drv, index);
 
 	cpu_pm_exit();
-	tegra_clear_cpu_in_lp2(cpu);
+	tegra_clear_cpu_in_lp2();
 
 	local_fiq_enable();
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 9d9f554..ed2a2a7 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -114,16 +114,15 @@ static int tegra30_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;
 	bool last_cpu;
 
 	local_fiq_disable();
 
-	last_cpu = tegra_set_cpu_in_lp2(cpu);
+	last_cpu = tegra_set_cpu_in_lp2();
 	cpu_pm_enter();
 
-	if (cpu == 0) {
+	if (dev->cpu == 0) {
 		if (last_cpu)
 			entered_lp2 = tegra30_cpu_cluster_power_down(dev, drv,
 								     index);
@@ -134,7 +133,7 @@ static int tegra30_idle_lp2(struct cpuidle_device *dev,
 	}
 
 	cpu_pm_exit();
-	tegra_clear_cpu_in_lp2(cpu);
+	tegra_clear_cpu_in_lp2();
 
 	local_fiq_enable();
 
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 779dbb4..8f8678b 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -99,8 +99,9 @@ static void suspend_cpu_complex(void)
 	flowctrl_cpu_suspend_enter(cpu);
 }
 
-void tegra_clear_cpu_in_lp2(int phy_cpu_id)
+void tegra_clear_cpu_in_lp2(void)
 {
+	int phy_cpu_id = cpu_logical_map(smp_processor_id());
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;
 
 	spin_lock(&tegra_lp2_lock);
@@ -111,8 +112,9 @@ void tegra_clear_cpu_in_lp2(int phy_cpu_id)
 	spin_unlock(&tegra_lp2_lock);
 }
 
-bool tegra_set_cpu_in_lp2(int phy_cpu_id)
+bool tegra_set_cpu_in_lp2(void)
 {
+	int phy_cpu_id = cpu_logical_map(smp_processor_id());
 	bool last_cpu = false;
 	cpumask_t *cpu_lp2_mask = tegra_cpu_lp2_mask;
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;
@@ -200,7 +202,7 @@ static int __cpuinit tegra_suspend_enter(suspend_state_t state)
 	suspend_cpu_complex();
 	switch (mode) {
 	case TEGRA_SUSPEND_LP2:
-		tegra_set_cpu_in_lp2(0);
+		tegra_set_cpu_in_lp2();
 		break;
 	default:
 		break;
@@ -210,7 +212,7 @@ static int __cpuinit tegra_suspend_enter(suspend_state_t state)
 
 	switch (mode) {
 	case TEGRA_SUSPEND_LP2:
-		tegra_clear_cpu_in_lp2(0);
+		tegra_clear_cpu_in_lp2();
 		break;
 	default:
 		break;
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index 778a4aa..94c4b9d 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -28,8 +28,8 @@ extern unsigned long l2x0_saved_regs_addr;
 void save_cpu_arch_register(void);
 void restore_cpu_arch_register(void);
 
-void tegra_clear_cpu_in_lp2(int phy_cpu_id);
-bool tegra_set_cpu_in_lp2(int phy_cpu_id);
+void tegra_clear_cpu_in_lp2(void);
+bool tegra_set_cpu_in_lp2(void);
 
 void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
-- 
1.8.3

[PATCH 4/4] ARM: tegra: check processor id in tegra_{set, clear}_cpu_in_lp2

[Stephen Warren]

On 06/03/2013 05:17 AM, Joseph Lo wrote:
> The tegra_set_cpu_in_lp2 was used in the Tegra reset handler to know the
> CPU were coming up with which state then redirect to the appropriate
> handler. So we need to pass the processor id every time when we use the
> function. Now moving the code into the function.

That description makes very little sense. To me, the second sentence
actually implies the opposite code change to the content of this patch.
I think the following would be better:

ARM: tegra: don't pass CPU ID to tegra_{set,clear}_cpu_in_lp2

tegra_{set,clear}_cpu_in_lp2 can easily determine which CPU ID they are
running on; there is no need to pass the CPU ID into those functions.
So, remove their CPU ID function parameter.