[PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver

[PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver

[Joseph Lo]

The CPU power timer set up fucntion was related to PMC register. Now moving
it to PMC driver. And it also help to clean up the PM related code later.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
This series was based on the patch set of suspending support.
---
 arch/arm/mach-tegra/pm.c  | 36 ------------------------------------
 arch/arm/mach-tegra/pmc.c | 38 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/pmc.h |  1 +
 3 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 915e13b..b7cc637 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -23,7 +23,6 @@
 #include <linux/delay.h>
 #include <linux/cpu_pm.h>
 #include <linux/suspend.h>
-#include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/clk/tegra.h>
 
@@ -44,47 +43,12 @@
 #define TEGRA_POWER_CPU_PWRREQ_OE	(1 << 16)  /* CPU pwr req enable */
 
 #define PMC_CTRL		0x0
-#define PMC_CPUPWRGOOD_TIMER	0xc8
-#define PMC_CPUPWROFF_TIMER	0xcc
 
 #ifdef CONFIG_PM_SLEEP
 static DEFINE_SPINLOCK(tegra_lp2_lock);
 static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
-static struct clk *tegra_pclk;
 void (*tegra_tear_down_cpu)(void);
 
-static void set_power_timers(unsigned long us_on, unsigned long us_off)
-{
-	unsigned long long ticks;
-	unsigned long long pclk;
-	unsigned long rate;
-	static unsigned long tegra_last_pclk;
-
-	if (tegra_pclk == NULL) {
-		tegra_pclk = clk_get_sys(NULL, "pclk");
-		WARN_ON(IS_ERR(tegra_pclk));
-	}
-
-	rate = clk_get_rate(tegra_pclk);
-
-	if (WARN_ON_ONCE(rate <= 0))
-		pclk = 100000000;
-	else
-		pclk = rate;
-
-	if ((rate != tegra_last_pclk)) {
-		ticks = (us_on * pclk) + 999999ull;
-		do_div(ticks, 1000000);
-		writel((unsigned long)ticks, pmc + PMC_CPUPWRGOOD_TIMER);
-
-		ticks = (us_off * pclk) + 999999ull;
-		do_div(ticks, 1000000);
-		writel((unsigned long)ticks, pmc + PMC_CPUPWROFF_TIMER);
-		wmb();
-	}
-	tegra_last_pclk = pclk;
-}
-
 /*
  * restore_cpu_complex
  *
diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c
index f0c5435..bb1fe8e 100644
--- a/arch/arm/mach-tegra/pmc.c
+++ b/arch/arm/mach-tegra/pmc.c
@@ -19,6 +19,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/clk.h>
 
 #include "fuse.h"
 #include "pmc.h"
@@ -52,6 +53,9 @@ static DEFINE_SPINLOCK(tegra_powergate_lock);
 static void __iomem *tegra_pmc_base;
 static bool tegra_pmc_invert_interrupt;
 
+#define PMC_CPUPWRGOOD_TIMER	0xc8
+#define PMC_CPUPWROFF_TIMER	0xcc
+
 struct pmc_pm_data {
 	u32 cpu_good_time;	/* CPU power good time in uS */
 	u32 cpu_off_time;	/* CPU power off time in uS */
@@ -158,6 +162,40 @@ int tegra_pmc_cpu_remove_clamping(int cpuid)
 }
 
 #ifdef CONFIG_PM_SLEEP
+static struct clk *tegra_pclk;
+
+void set_power_timers(unsigned long us_on, unsigned long us_off)
+{
+	unsigned long long ticks;
+	unsigned long long pclk;
+	unsigned long rate;
+	static unsigned long tegra_last_pclk;
+
+	if (tegra_pclk == NULL) {
+		tegra_pclk = clk_get_sys(NULL, "pclk");
+		WARN_ON(IS_ERR(tegra_pclk));
+	}
+
+	rate = clk_get_rate(tegra_pclk);
+
+	if (WARN_ON_ONCE(rate <= 0))
+		pclk = 100000000;
+	else
+		pclk = rate;
+
+	if ((rate != tegra_last_pclk)) {
+		ticks = (us_on * pclk) + 999999ull;
+		do_div(ticks, 1000000);
+		tegra_pmc_writel((unsigned long)ticks, PMC_CPUPWRGOOD_TIMER);
+
+		ticks = (us_off * pclk) + 999999ull;
+		do_div(ticks, 1000000);
+		tegra_pmc_writel((unsigned long)ticks, PMC_CPUPWROFF_TIMER);
+		wmb();
+	}
+	tegra_last_pclk = pclk;
+}
+
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void)
 {
 	return pmc_pm_data.suspend_mode;
diff --git a/arch/arm/mach-tegra/pmc.h b/arch/arm/mach-tegra/pmc.h
index 72b2cee..9413a44 100644
--- a/arch/arm/mach-tegra/pmc.h
+++ b/arch/arm/mach-tegra/pmc.h
@@ -27,6 +27,7 @@ enum tegra_suspend_mode {
 };
 
 #ifdef CONFIG_PM_SLEEP
+void set_power_timers(unsigned long us_on, unsigned long us_off);
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
 void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
 u32 tegra_pmc_get_cpu_good_time(void);
-- 
1.8.1.5

[PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function

[Joseph Lo]

The pmc_pm_set function was designed for SoC to configure the related
settings when system going to some low power modes (e.g. platform
suspend or CPU idle powered-down mode). We refactor the function to make
it work on the usage.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
This series was based on the patch set of suspending support.
---
 arch/arm/mach-tegra/pm.c  | 18 ++----------------
 arch/arm/mach-tegra/pmc.c | 38 ++++++++++++++++++--------------------
 arch/arm/mach-tegra/pmc.h |  5 +----
 3 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index b7cc637..2a04dfc 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -40,13 +40,8 @@
 #include "sleep.h"
 #include "pmc.h"
 
-#define TEGRA_POWER_CPU_PWRREQ_OE	(1 << 16)  /* CPU pwr req enable */
-
-#define PMC_CTRL		0x0
-
 #ifdef CONFIG_PM_SLEEP
 static DEFINE_SPINLOCK(tegra_lp2_lock);
-static void __iomem *pmc = IO_ADDRESS(TEGRA_PMC_BASE);
 void (*tegra_tear_down_cpu)(void);
 
 /*
@@ -146,14 +141,7 @@ static int tegra_sleep_cpu(unsigned long v2p)
 
 void tegra_idle_lp2_last(u32 cpu_on_time, u32 cpu_off_time)
 {
-	u32 mode;
-
-	/* Only the last cpu down does the final suspend steps */
-	mode = readl(pmc + PMC_CTRL);
-	mode |= TEGRA_POWER_CPU_PWRREQ_OE;
-	writel(mode, pmc + PMC_CTRL);
-
-	set_power_timers(cpu_on_time, cpu_off_time);
+	tegra_pmc_pm_set(TEGRA_SUSPEND_LP2);
 
 	cpu_cluster_pm_enter();
 	suspend_cpu_complex();
@@ -181,9 +169,7 @@ static int __cpuinit tegra_suspend_enter(suspend_state_t state)
 
 	pr_info("Entering suspend state %s\n", lp_state[mode]);
 
-	tegra_pmc_pm_set();
-	set_power_timers(tegra_pmc_get_cpu_good_time(),
-			 tegra_pmc_get_cpu_off_time());
+	tegra_pmc_pm_set(mode);
 
 	local_fiq_disable();
 
diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c
index bb1fe8e..118a465 100644
--- a/arch/arm/mach-tegra/pmc.c
+++ b/arch/arm/mach-tegra/pmc.c
@@ -20,6 +20,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/clk.h>
+#include <linux/clk-provider.h>
 
 #include "fuse.h"
 #include "pmc.h"
@@ -164,20 +165,12 @@ int tegra_pmc_cpu_remove_clamping(int cpuid)
 #ifdef CONFIG_PM_SLEEP
 static struct clk *tegra_pclk;
 
-void set_power_timers(unsigned long us_on, unsigned long us_off)
+static void set_power_timers(u32 us_on, u32 us_off, unsigned long rate)
 {
 	unsigned long long ticks;
 	unsigned long long pclk;
-	unsigned long rate;
 	static unsigned long tegra_last_pclk;
 
-	if (tegra_pclk == NULL) {
-		tegra_pclk = clk_get_sys(NULL, "pclk");
-		WARN_ON(IS_ERR(tegra_pclk));
-	}
-
-	rate = clk_get_rate(tegra_pclk);
-
 	if (WARN_ON_ONCE(rate <= 0))
 		pclk = 100000000;
 	else
@@ -209,24 +202,26 @@ void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode)
 	pmc_pm_data.suspend_mode = mode;
 }
 
-u32 tegra_pmc_get_cpu_good_time(void)
-{
-	return pmc_pm_data.cpu_good_time;
-}
-
-u32 tegra_pmc_get_cpu_off_time(void)
-{
-	return pmc_pm_data.cpu_off_time;
-}
-
-void tegra_pmc_pm_set(void)
+void tegra_pmc_pm_set(enum tegra_suspend_mode mode)
 {
 	u32 reg;
+	unsigned long rate = 0;
 
 	reg = tegra_pmc_readl(PMC_CTRL);
 	reg |= TEGRA_POWER_CPU_PWRREQ_OE;
 	reg &= ~TEGRA_POWER_EFFECT_LP0;
 
+	switch (mode) {
+	case TEGRA_SUSPEND_LP2:
+		rate = __clk_get_rate(tegra_pclk);
+		break;
+	default:
+		break;
+	}
+
+	set_power_timers(pmc_pm_data.cpu_good_time, pmc_pm_data.cpu_off_time,
+			 rate);
+
 	tegra_pmc_writel(reg, PMC_CTRL);
 }
 
@@ -234,6 +229,9 @@ void tegra_pmc_suspend_init(void)
 {
 	u32 reg;
 
+	tegra_pclk = clk_get_sys(NULL, "pclk");
+	WARN_ON(IS_ERR(tegra_pclk));
+
 	/* Always enable CPU power request; just normal polarity is supported */
 	reg = tegra_pmc_readl(PMC_CTRL);
 	BUG_ON(reg & TEGRA_POWER_CPU_PWRREQ_POLARITY);
diff --git a/arch/arm/mach-tegra/pmc.h b/arch/arm/mach-tegra/pmc.h
index 9413a44..0ee22d9 100644
--- a/arch/arm/mach-tegra/pmc.h
+++ b/arch/arm/mach-tegra/pmc.h
@@ -27,12 +27,9 @@ enum tegra_suspend_mode {
 };
 
 #ifdef CONFIG_PM_SLEEP
-void set_power_timers(unsigned long us_on, unsigned long us_off);
 enum tegra_suspend_mode tegra_pmc_get_suspend_mode(void);
 void tegra_pmc_set_suspend_mode(enum tegra_suspend_mode mode);
-u32 tegra_pmc_get_cpu_good_time(void);
-u32 tegra_pmc_get_cpu_off_time(void);
-void tegra_pmc_pm_set(void);
+void tegra_pmc_pm_set(enum tegra_suspend_mode mode);
 void tegra_pmc_suspend_init(void);
 #endif
 
-- 
1.8.1.5

[PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function

[Stephen Warren]

On 03/13/2013 02:27 AM, Joseph Lo wrote:
> The pmc_pm_set function was designed for SoC to configure the related
> settings when system going to some low power modes (e.g. platform
> suspend or CPU idle powered-down mode). We refactor the function to make
> it work on the usage.

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

> +#include <linux/clk-provider.h>

This code isn't a clock-provider, and hence shouldn't include that
header file.

> +void tegra_pmc_pm_set(enum tegra_suspend_mode mode)

> +	switch (mode) {
> +	case TEGRA_SUSPEND_LP2:
> +		rate = __clk_get_rate(tegra_pclk);

Doesn't regular clk_get_rate() work here?

> @@ -234,6 +229,9 @@ void tegra_pmc_suspend_init(void)
>  {
>  	u32 reg;
>  
> +	tegra_pclk = clk_get_sys(NULL, "pclk");
> +	WARN_ON(IS_ERR(tegra_pclk));

Shouldn't this instead error out and/or disable any system suspend
modes? Otherwise, tegra_pmc_pm_set() is going to call
clk_get_rate(tegra_pclk), and tegra_pclk won't be a valid clock object.

Also, can you use regular clk_get() rather than clk_get_sys()? That'd
need a clocks property in the PMC DT node.

I guess I see now that this series does actually depend on the suspend
series. However, stuff like:

> -u32 tegra_pmc_get_cpu_good_time(void)
> -{
> -	return pmc_pm_data.cpu_good_time;
> -}
> -
> -u32 tegra_pmc_get_cpu_off_time(void)
> -{
> -	return pmc_pm_data.cpu_off_time;
> -}

... was actually added in that series, so if you put this series first,
or merged the two series with these patches first, you could end up
avoiding some churn.

[PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function

[Joseph Lo]

On Thu, 2013-03-14 at 01:52 +0800, Stephen Warren wrote:
> On 03/13/2013 02:27 AM, Joseph Lo wrote:
> > The pmc_pm_set function was designed for SoC to configure the related
> > settings when system going to some low power modes (e.g. platform
> > suspend or CPU idle powered-down mode). We refactor the function to make
> > it work on the usage.
> 
> > diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c
> 
> > +#include <linux/clk-provider.h>
> 
> This code isn't a clock-provider, and hence shouldn't include that
> header file.
> 
> > +void tegra_pmc_pm_set(enum tegra_suspend_mode mode)
> 
> > +	switch (mode) {
> > +	case TEGRA_SUSPEND_LP2:
> > +		rate = __clk_get_rate(tegra_pclk);
> 
> Doesn't regular clk_get_rate() work here?
> 
Actually it works. So I will use clk_get_rate() here and remove
clk-provider.

> > @@ -234,6 +229,9 @@ void tegra_pmc_suspend_init(void)
> >  {
> >  	u32 reg;
> >  
> > +	tegra_pclk = clk_get_sys(NULL, "pclk");
> > +	WARN_ON(IS_ERR(tegra_pclk));
> 
> Shouldn't this instead error out and/or disable any system suspend
> modes? Otherwise, tegra_pmc_pm_set() is going to call
> clk_get_rate(tegra_pclk), and tegra_pclk won't be a valid clock object.
OK.
> 
> Also, can you use regular clk_get() rather than clk_get_sys()? That'd
> need a clocks property in the PMC DT node.
OK.
> I guess I see now that this series does actually depend on the suspend
> series. However, stuff like:
> 
> > -u32 tegra_pmc_get_cpu_good_time(void)
> > -{
> > -	return pmc_pm_data.cpu_good_time;
> > -}
> > -
> > -u32 tegra_pmc_get_cpu_off_time(void)
> > -{
> > -	return pmc_pm_data.cpu_off_time;
> > -}
> 
> ... was actually added in that series, so if you put this series first,
> or merged the two series with these patches first, you could end up
> avoiding some churn.
Let me check how to reorganize them.

Thanks,
Joseph

[PATCH 3/3] ARM: tegra: cpuidle: remove redundant parameters for powered-down mode

[Joseph Lo]

After the previous patch, tegra_idle_lp2_last() no longer uses its
parameters cpu_on_time or cpu_off_time, so remove them.

Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
This series was based on the patch set of suspending support.
---
 arch/arm/mach-tegra/cpuidle-tegra20.c | 6 +-----
 arch/arm/mach-tegra/cpuidle-tegra30.c | 6 +-----
 arch/arm/mach-tegra/pm.c              | 2 +-
 arch/arm/mach-tegra/pm.h              | 2 +-
 4 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 825ced4..8bbbdeb 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -130,10 +130,6 @@ 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();
 
@@ -142,7 +138,7 @@ static bool tegra20_cpu_cluster_power_down(struct cpuidle_device *dev,
 
 	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
 
-	tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+	tegra_idle_lp2_last();
 
 	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
 
diff --git a/arch/arm/mach-tegra/cpuidle-tegra30.c b/arch/arm/mach-tegra/cpuidle-tegra30.c
index 80445ed..c0931c8 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra30.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra30.c
@@ -72,10 +72,6 @@ static bool tegra30_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;
-
 	/* All CPUs entering LP2 is not working.
 	 * Don't let CPU0 enter LP2 when any secondary CPU is online.
 	 */
@@ -86,7 +82,7 @@ static bool tegra30_cpu_cluster_power_down(struct cpuidle_device *dev,
 
 	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
 
-	tegra_idle_lp2_last(cpu_on_time, cpu_off_time);
+	tegra_idle_lp2_last();
 
 	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
 
diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
index 2a04dfc..90b8ddc 100644
--- a/arch/arm/mach-tegra/pm.c
+++ b/arch/arm/mach-tegra/pm.c
@@ -139,7 +139,7 @@ static int tegra_sleep_cpu(unsigned long v2p)
 	return 0;
 }
 
-void tegra_idle_lp2_last(u32 cpu_on_time, u32 cpu_off_time)
+void tegra_idle_lp2_last(void)
 {
 	tegra_pmc_pm_set(TEGRA_SUSPEND_LP2);
 
diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
index e7cbfeb..85e777a 100644
--- a/arch/arm/mach-tegra/pm.h
+++ b/arch/arm/mach-tegra/pm.h
@@ -29,7 +29,7 @@ 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_idle_lp2_last(u32 cpu_on_time, u32 cpu_off_time);
+void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP
-- 
1.8.1.5

[PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver

[Stephen Warren]

On 03/13/2013 02:27 AM, Joseph Lo wrote:
> The CPU power timer set up fucntion was related to PMC register. Now moving
> it to PMC driver. And it also help to clean up the PM related code later.
> 
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> This series was based on the patch set of suspending support.

So the series is based on the suspend series in terms of patch context,
but does it actually /depend/ on that series?

This series feels like cleanup to me, so I'd be tempted to apply it to
Tegra's for-3.10/cleanup branch, whereas I was planning on applying the
suspend series to its own for-3.10/suspend topic branch.

If that won't work, I guess I can apply this series on top of the
suspend series in the suspend topic branch.

[PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver

[Joseph Lo]

On Thu, 2013-03-14 at 01:40 +0800, Stephen Warren wrote:
> On 03/13/2013 02:27 AM, Joseph Lo wrote:
> > The CPU power timer set up fucntion was related to PMC register. Now moving
> > it to PMC driver. And it also help to clean up the PM related code later.
> > 
> > Signed-off-by: Joseph Lo <josephl@nvidia.com>
> > ---
> > This series was based on the patch set of suspending support.
> 
> So the series is based on the suspend series in terms of patch context,
> but does it actually /depend/ on that series?
For functionality, it didn't. But the other two, yes.
> 
> This series feels like cleanup to me, so I'd be tempted to apply it to
> Tegra's for-3.10/cleanup branch, whereas I was planning on applying the
> suspend series to its own for-3.10/suspend topic branch.
> 
> If that won't work, I guess I can apply this series on top of the
> suspend series in the suspend topic branch.
Thanks for take care.