Re: [PATCH 06/12] ARM: OMAP44xx: PM: convert to use the functional power states API

2013-01-04 Thread Tero Kristo 
Hi Paul,

On Sun, 2012-12-09 at 10:53 -0700, Paul Walmsley wrote:

clip

 @@ -40,20 +39,19 @@ static LIST_HEAD(pwrst_list);
  static int omap4_pm_suspend(void)
  {
   struct power_state *pwrst;
 - int state, ret = 0;
 + int prev_fpwrst;
 + int ret = 0;
   u32 cpu_id = smp_processor_id();
  
 + /* XXX Seems like these two loops could be combined into one loop? */
 +

They can be combined yes.

clip

 @@ -113,10 +113,10 @@ static int __init pwrdms_setup(struct powerdomain 
 *pwrdm, void *unused)
   return -ENOMEM;
  
   pwrst-pwrdm = pwrdm;
 - pwrst-next_state = PWRDM_POWER_RET;
 + pwrst-next_fpwrst = PWRDM_FUNC_PWRST_CSWR;
   list_add(pwrst-node, pwrst_list);
  
 - return omap_set_pwrdm_state(pwrst-pwrdm, pwrst-next_state);
 + return WARN_ON(pwrdm_set_fpwrst(pwrst-pwrdm, pwrst-next_fpwrst));

This causes a regression on omap4, as several power domains can't idle
after this anymore (they get programmed to ON state due to CSWR not
being supported on them.) Following patch fixes this problem (applies on
top of the whole set):

=

From: Tero Kristo t-kri...@ti.com
Date: Wed, 2 Jan 2013 18:05:42 +0200
Subject: [PATCH] ARM: OMAP4: PM: fix the power state setup

After the functional power state code changes, OMAP4 PM init code
programs
powerdomains now to ON state if the domain does not support CSWR. This
breaks
suspend and should be avoided.

Fixed by adding a loop to the pwrdms_setup code, that for each domain
selects
the next fpwrst based on following order: CSWR, OSWR, OFF, ON. This
allows
all the domains to enter supported low power states, instead of always
selecting ON.

Signed-off-by: Tero Kristo t-kri...@ti.com
---
 arch/arm/mach-omap2/pm44xx.c |   22 +-
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index 8775ee5..67f05fe 100644
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -88,6 +88,13 @@ static int omap4_pm_suspend(void)
 static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
 {
struct power_state *pwrst;
+   const u8 fpwrst_states[] = {
+   PWRDM_FUNC_PWRST_CSWR,
+   PWRDM_FUNC_PWRST_OSWR,
+   PWRDM_FUNC_PWRST_OFF,
+   PWRDM_FUNC_PWRST_ON,
+   };
+   int i;
 
if (!pwrdm-pwrsts)
return 0;
@@ -106,12 +113,17 @@ static int __init pwrdms_setup(struct powerdomain
*pwrdm, void *unused)
 
pwrst-pwrdm = pwrdm;
/*
-* XXX This should be replaced by explicit lists of
-* powerdomains with specific powerstates to set
+* We attempt to program the fpwrst of a domain in the following
+* order: CSWR, OSWR, OFF, ON. This is needed as some domains
+* don't support retention, and instead should go to off in low
+* power modes. If everything else fails, the code falls back
+* to ON state.
 */
-   pwrst-next_fpwrst = PWRDM_FUNC_PWRST_CSWR;
-   if (!pwrdm_supports_fpwrst(pwrdm, pwrst-next_fpwrst))
-   pwrst-next_fpwrst = PWRDM_FUNC_PWRST_ON;
+   for (i = 0; i  ARRAY_SIZE(fpwrst_states); i++) {
+   pwrst-next_fpwrst = fpwrst_states[i];
+   if (pwrdm_supports_fpwrst(pwrdm, pwrst-next_fpwrst))
+   break;
+   }
list_add(pwrst-node, pwrst_list);
 
return WARN_ON(pwrdm_set_fpwrst(pwrst-pwrdm, pwrst-next_fpwrst));
-- 
1.7.4.1



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/12] ARM: OMAP44xx: PM: convert to use the functional power states API

2012-12-12 Thread Jean Pihet 
Paul,

On Sun, Dec 9, 2012 at 6:53 PM, Paul Walmsley p...@pwsan.com wrote:
 From: Jean Pihet jean.pi...@newoldbits.com

 Use the functional power states as the API to control power
 domains:
 - use the PWRDM_FUNC_PWRST_* and PWRDM_LOGIC_MEM_PWRST_*
   macros for the power states and logic settings,
 - the function pwrdm_set_next_fpwrst, which controls
   the power domains next power and logic settings, shall
   be used instead of pwrdm_set_next_pwrst to program the
   power domains next states,
 - the function pwrdm_set_fpwrst, which programs the power
   domains power and logic settings, shall be used instead
   of omap_set_pwrdm_state.

 Signed-off-by: Jean Pihet j-pi...@ti.com
 [p...@pwsan.com: split the original patch into OMAP2/3/4 variants;
  warn if sets fail; various other changes]
 Signed-off-by: Paul Walmsley p...@pwsan.com
 ---
  arch/arm/mach-omap2/common.h  |7 +--
  arch/arm/mach-omap2/cpuidle44xx.c |   32 +
  arch/arm/mach-omap2/omap-hotplug.c|2 -
  arch/arm/mach-omap2/omap-mpuss-lowpower.c |   69 
 +
  arch/arm/mach-omap2/pm44xx.c  |   42 +-
  5 files changed, 79 insertions(+), 73 deletions(-)

 diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
 index c57eeea..5ad846a 100644
 --- a/arch/arm/mach-omap2/common.h
 +++ b/arch/arm/mach-omap2/common.h
 @@ -235,14 +235,13 @@ extern void omap5_secondary_startup(void);

  #if defined(CONFIG_SMP)  defined(CONFIG_PM)
  extern int omap4_mpuss_init(void);
 -extern int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state);
 +extern int omap4_mpuss_enter_lowpower(unsigned int cpu, u8 fpwrst);
  extern int omap4_finish_suspend(unsigned long cpu_state);
  extern void omap4_cpu_resume(void);
 -extern int omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state);
 +extern int omap4_mpuss_hotplug_cpu(unsigned int cpu, u8 fpwrst);
  extern u32 omap4_mpuss_read_prev_context_state(void);
  #else
 -static inline int omap4_enter_lowpower(unsigned int cpu,
 -   unsigned int power_state)
 +static inline int omap4_mpuss_enter_lowpower(unsigned int cpu, u8 fpwrst)
  {
 cpu_do_idle();
 return 0;
 diff --git a/arch/arm/mach-omap2/cpuidle44xx.c 
 b/arch/arm/mach-omap2/cpuidle44xx.c
 index d639aef..2cb5332 100644
 --- a/arch/arm/mach-omap2/cpuidle44xx.c
 +++ b/arch/arm/mach-omap2/cpuidle44xx.c
 @@ -25,26 +25,22 @@

  /* Machine specific information */
  struct omap4_idle_statedata {
 -   u32 cpu_state;
 -   u32 mpu_logic_state;
 -   u32 mpu_state;
 +   u8 cpu_pwrst;
 +   u8 mpu_pwrst;

This should be cpu_fpwrst and mpu_fwprst for consistency.

  };

  static struct omap4_idle_statedata omap4_idle_data[] = {
 {
 -   .cpu_state = PWRDM_POWER_ON,
 -   .mpu_state = PWRDM_POWER_ON,
 -   .mpu_logic_state = PWRDM_POWER_RET,
 +   .cpu_pwrst = PWRDM_FUNC_PWRST_ON,
 +   .mpu_pwrst = PWRDM_FUNC_PWRST_ON,
 },
 {
 -   .cpu_state = PWRDM_POWER_OFF,
 -   .mpu_state = PWRDM_POWER_RET,
 -   .mpu_logic_state = PWRDM_POWER_RET,
 +   .cpu_pwrst = PWRDM_FUNC_PWRST_OFF,
 +   .mpu_pwrst = PWRDM_FUNC_PWRST_CSWR,
 },
 {
 -   .cpu_state = PWRDM_POWER_OFF,
 -   .mpu_state = PWRDM_POWER_RET,
 -   .mpu_logic_state = PWRDM_POWER_OFF,
 +   .cpu_pwrst = PWRDM_FUNC_PWRST_OFF,
 +   .mpu_pwrst = PWRDM_FUNC_PWRST_OSWR,
 },
  };

 @@ -93,7 +89,7 @@ static int omap4_enter_idle_coupled(struct cpuidle_device 
 *dev,
  * out of coherency and in OFF mode.
  */
 if (dev-cpu == 0  cpumask_test_cpu(1, cpu_online_mask)) {
 -   while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF) {
 +   while (pwrdm_read_fpwrst(cpu_pd[1]) != PWRDM_FUNC_PWRST_OFF) {
 cpu_relax();

 /*
 @@ -118,19 +114,17 @@ static int omap4_enter_idle_coupled(struct 
 cpuidle_device *dev,
 cpu_pm_enter();

 if (dev-cpu == 0) {
 -   pwrdm_set_logic_retst(mpu_pd, cx-mpu_logic_state);
 -   omap_set_pwrdm_state(mpu_pd, cx-mpu_state);
 +   WARN_ON(pwrdm_set_fpwrst(mpu_pd, cx-mpu_pwrst));

 /*
  * Call idle CPU cluster PM enter notifier chain
  * to save GIC and wakeupgen context.
  */
 -   if ((cx-mpu_state == PWRDM_POWER_RET) 
 -   (cx-mpu_logic_state == PWRDM_POWER_OFF))
 -   cpu_cluster_pm_enter();
 +   if (cx-mpu_pwrst == PWRDM_FUNC_PWRST_OSWR)
 +   cpu_cluster_pm_enter();
 }

 -   omap4_enter_lowpower(dev-cpu, cx-cpu_state);
 +   omap4_mpuss_enter_lowpower(dev-cpu, cx-cpu_pwrst);

[PATCH 06/12] ARM: OMAP44xx: PM: convert to use the functional power states API

2012-12-09 Thread Paul Walmsley 
From: Jean Pihet jean.pi...@newoldbits.com

Use the functional power states as the API to control power
domains:
- use the PWRDM_FUNC_PWRST_* and PWRDM_LOGIC_MEM_PWRST_*
  macros for the power states and logic settings,
- the function pwrdm_set_next_fpwrst, which controls
  the power domains next power and logic settings, shall
  be used instead of pwrdm_set_next_pwrst to program the
  power domains next states,
- the function pwrdm_set_fpwrst, which programs the power
  domains power and logic settings, shall be used instead
  of omap_set_pwrdm_state.

Signed-off-by: Jean Pihet j-pi...@ti.com
[p...@pwsan.com: split the original patch into OMAP2/3/4 variants;
 warn if sets fail; various other changes]
Signed-off-by: Paul Walmsley p...@pwsan.com
---
 arch/arm/mach-omap2/common.h  |7 +--
 arch/arm/mach-omap2/cpuidle44xx.c |   32 +
 arch/arm/mach-omap2/omap-hotplug.c|2 -
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |   69 +
 arch/arm/mach-omap2/pm44xx.c  |   42 +-
 5 files changed, 79 insertions(+), 73 deletions(-)

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index c57eeea..5ad846a 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -235,14 +235,13 @@ extern void omap5_secondary_startup(void);
 
 #if defined(CONFIG_SMP)  defined(CONFIG_PM)
 extern int omap4_mpuss_init(void);
-extern int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state);
+extern int omap4_mpuss_enter_lowpower(unsigned int cpu, u8 fpwrst);
 extern int omap4_finish_suspend(unsigned long cpu_state);
 extern void omap4_cpu_resume(void);
-extern int omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state);
+extern int omap4_mpuss_hotplug_cpu(unsigned int cpu, u8 fpwrst);
 extern u32 omap4_mpuss_read_prev_context_state(void);
 #else
-static inline int omap4_enter_lowpower(unsigned int cpu,
-   unsigned int power_state)
+static inline int omap4_mpuss_enter_lowpower(unsigned int cpu, u8 fpwrst)
 {
cpu_do_idle();
return 0;
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c 
b/arch/arm/mach-omap2/cpuidle44xx.c
index d639aef..2cb5332 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -25,26 +25,22 @@
 
 /* Machine specific information */
 struct omap4_idle_statedata {
-   u32 cpu_state;
-   u32 mpu_logic_state;
-   u32 mpu_state;
+   u8 cpu_pwrst;
+   u8 mpu_pwrst;
 };
 
 static struct omap4_idle_statedata omap4_idle_data[] = {
{
-   .cpu_state = PWRDM_POWER_ON,
-   .mpu_state = PWRDM_POWER_ON,
-   .mpu_logic_state = PWRDM_POWER_RET,
+   .cpu_pwrst = PWRDM_FUNC_PWRST_ON,
+   .mpu_pwrst = PWRDM_FUNC_PWRST_ON,
},
{
-   .cpu_state = PWRDM_POWER_OFF,
-   .mpu_state = PWRDM_POWER_RET,
-   .mpu_logic_state = PWRDM_POWER_RET,
+   .cpu_pwrst = PWRDM_FUNC_PWRST_OFF,
+   .mpu_pwrst = PWRDM_FUNC_PWRST_CSWR,
},
{
-   .cpu_state = PWRDM_POWER_OFF,
-   .mpu_state = PWRDM_POWER_RET,
-   .mpu_logic_state = PWRDM_POWER_OFF,
+   .cpu_pwrst = PWRDM_FUNC_PWRST_OFF,
+   .mpu_pwrst = PWRDM_FUNC_PWRST_OSWR,
},
 };
 
@@ -93,7 +89,7 @@ static int omap4_enter_idle_coupled(struct cpuidle_device 
*dev,
 * out of coherency and in OFF mode.
 */
if (dev-cpu == 0  cpumask_test_cpu(1, cpu_online_mask)) {
-   while (pwrdm_read_pwrst(cpu_pd[1]) != PWRDM_POWER_OFF) {
+   while (pwrdm_read_fpwrst(cpu_pd[1]) != PWRDM_FUNC_PWRST_OFF) {
cpu_relax();
 
/*
@@ -118,19 +114,17 @@ static int omap4_enter_idle_coupled(struct cpuidle_device 
*dev,
cpu_pm_enter();
 
if (dev-cpu == 0) {
-   pwrdm_set_logic_retst(mpu_pd, cx-mpu_logic_state);
-   omap_set_pwrdm_state(mpu_pd, cx-mpu_state);
+   WARN_ON(pwrdm_set_fpwrst(mpu_pd, cx-mpu_pwrst));
 
/*
 * Call idle CPU cluster PM enter notifier chain
 * to save GIC and wakeupgen context.
 */
-   if ((cx-mpu_state == PWRDM_POWER_RET) 
-   (cx-mpu_logic_state == PWRDM_POWER_OFF))
-   cpu_cluster_pm_enter();
+   if (cx-mpu_pwrst == PWRDM_FUNC_PWRST_OSWR)
+   cpu_cluster_pm_enter();
}
 
-   omap4_enter_lowpower(dev-cpu, cx-cpu_state);
+   omap4_mpuss_enter_lowpower(dev-cpu, cx-cpu_pwrst);
cpu_done[dev-cpu] = true;
 
/* Wakeup CPU1 only if it is not offlined */
diff --git a/arch/arm/mach-omap2/omap-hotplug.c 
b/arch/arm/mach-omap2/omap-hotplug.c
index e712d17..d38b12d 100644
--- a/arch/arm/mach-omap2/omap-hotplug.c
+++