[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Benoit Cousson]

Hi Paul,

This is a series of fixes on OMAP3/4 in setup apis,
in the suspend framework and in powerdomain modelling
for OMAP4.

The series is based on your power domain series and is available here:
git://gitorious.org/omap-pm/linux.git for_2.6.38/power

Tested on 4430sdp + ES2.0 with omap2plus_defconfig.
Tested on 3430sdp - validate OFF in suspend, with omap3_pm_defconfig
from Kevin's tree.
Tested on 2430sdp after fixing the break on linux-omap master.
See http://www.spinics.net/lists/linux-omap/msg42050.html

Regards,
Rajendra, Santosh & Benoit


Rajendra Nayak (3):
  OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
  OMAP4: PM: Do not assume clkdm supports hw transitions
  OMAP4: powerdomain: l4per pwrdm does not support OFF

Santosh Shilimkar (2):
  OMAP4: powerdomain: Remove L3INIT_PD OFF state
  OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

 arch/arm/mach-omap2/clock44xx_data.c        |    3 +++
 arch/arm/mach-omap2/pm.c                    |   16 +++++++++++++---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    4 ++--
 3 files changed, 18 insertions(+), 5 deletions(-)

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Benoit Cousson]

Hi Paul,

This is a series of fixes on OMAP3/4 in setup apis,
in the suspend framework and in powerdomain modelling
for OMAP4.

The series is based on your power domain series and is available here:
git://gitorious.org/omap-pm/linux.git for_2.6.38/power

Tested on 4430sdp + ES2.0 with omap2plus_defconfig.
Tested on 3430sdp - validate OFF in suspend, with omap3_pm_defconfig
from Kevin's tree.
Tested on 2430sdp after fixing the break on linux-omap master.
See http://www.spinics.net/lists/linux-omap/msg42050.html

Regards,
Rajendra, Santosh & Benoit


Rajendra Nayak (3):
  OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
  OMAP4: PM: Do not assume clkdm supports hw transitions
  OMAP4: powerdomain: l4per pwrdm does not support OFF

Santosh Shilimkar (2):
  OMAP4: powerdomain: Remove L3INIT_PD OFF state
  OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

 arch/arm/mach-omap2/clock44xx_data.c        |    3 +++
 arch/arm/mach-omap2/pm.c                    |   16 +++++++++++++---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    4 ++--
 3 files changed, 18 insertions(+), 5 deletions(-)

[PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

For pwrdm's which support lowperstatechange, do not try waking
up the domain to put it back to deeper sleep state.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/pm.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index cf1c4c9..dc68044 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -114,6 +114,14 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		return ret;
 
 	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
+		if ((pwrdm_read_pwrst(pwrdm) > state) &&
+			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+				ret = pwrdm_set_next_pwrst(pwrdm, state);
+				pwrdm_set_lowpwrstchange(pwrdm);
+				pwrdm_wait_transition(pwrdm);
+				pwrdm_state_switch(pwrdm);
+				return ret;
+		}
 		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
 		sleep_switch = 1;
 		pwrdm_wait_transition(pwrdm);
-- 
1.7.0.4

[PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

For pwrdm's which support lowperstatechange, do not try waking
up the domain to put it back to deeper sleep state.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/pm.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index cf1c4c9..dc68044 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -114,6 +114,14 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		return ret;
 
 	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
+		if ((pwrdm_read_pwrst(pwrdm) > state) &&
+			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+				ret = pwrdm_set_next_pwrst(pwrdm, state);
+				pwrdm_set_lowpwrstchange(pwrdm);
+				pwrdm_wait_transition(pwrdm);
+				pwrdm_state_switch(pwrdm);
+				return ret;
+		}
 		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
 		sleep_switch = 1;
 		pwrdm_wait_transition(pwrdm);
-- 
1.7.0.4

[PATCH 2/5] OMAP4: PM: Do not assume clkdm supports hw transitions

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

omap_set_pwrdm_state today assumes a clkdm supports hw_auto
transitions and hence leaves some which do not support this
in sw wkup state preventing low power transitions.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/pm.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index dc68044..a2a70e1 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -91,8 +91,7 @@ static void omap2_init_processor_devices(void)
 
 /*
  * This sets pwrdm state (other than mpu & core. Currently only ON &
- * RET are supported. Function is assuming that clkdm doesn't have
- * hw_sup mode enabled.
+ * RET are supported.
  */
 int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 {
@@ -135,7 +134,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	}
 
 	if (sleep_switch) {
-		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
+			omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+		else
+			omap2_clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
 		pwrdm_wait_transition(pwrdm);
 		pwrdm_state_switch(pwrdm);
 	}
-- 
1.7.0.4

[PATCH 2/5] OMAP4: PM: Do not assume clkdm supports hw transitions

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

omap_set_pwrdm_state today assumes a clkdm supports hw_auto
transitions and hence leaves some which do not support this
in sw wkup state preventing low power transitions.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/pm.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index dc68044..a2a70e1 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -91,8 +91,7 @@ static void omap2_init_processor_devices(void)
 
 /*
  * This sets pwrdm state (other than mpu & core. Currently only ON &
- * RET are supported. Function is assuming that clkdm doesn't have
- * hw_sup mode enabled.
+ * RET are supported.
  */
 int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 {
@@ -135,7 +134,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	}
 
 	if (sleep_switch) {
-		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
+			omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+		else
+			omap2_clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
 		pwrdm_wait_transition(pwrdm);
 		pwrdm_state_switch(pwrdm);
 	}
-- 
1.7.0.4

[PATCH 3/5] OMAP4: powerdomain: l4per pwrdm does not support OFF

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

The l4per power domain in ES2.0 does support only RET and ON states.
The previous ES1.0 HW database was wrong and thus fixed on ES2.
Change the pwrsts field to reflect that.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomains44xx_data.c b/arch/arm/mach-omap2/powerdomains44xx_data.c
index 5fdf485..cec9c09 100644
--- a/arch/arm/mach-omap2/powerdomains44xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains44xx_data.c
@@ -285,7 +285,7 @@ static struct powerdomain l4per_44xx_pwrdm = {
 	.prcm_offs	  = OMAP4430_PRM_L4PER_INST,
 	.prcm_partition	  = OMAP4430_PRM_PARTITION,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_RET_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 2,
 	.pwrsts_mem_ret	= {
-- 
1.7.0.4

[PATCH 3/5] OMAP4: powerdomain: l4per pwrdm does not support OFF

[Benoit Cousson]

From: Rajendra Nayak <rnayak@ti.com>

The l4per power domain in ES2.0 does support only RET and ON states.
The previous ES1.0 HW database was wrong and thus fixed on ES2.
Change the pwrsts field to reflect that.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomains44xx_data.c b/arch/arm/mach-omap2/powerdomains44xx_data.c
index 5fdf485..cec9c09 100644
--- a/arch/arm/mach-omap2/powerdomains44xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains44xx_data.c
@@ -285,7 +285,7 @@ static struct powerdomain l4per_44xx_pwrdm = {
 	.prcm_offs	  = OMAP4430_PRM_L4PER_INST,
 	.prcm_partition	  = OMAP4430_PRM_PARTITION,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_RET_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 2,
 	.pwrsts_mem_ret	= {
-- 
1.7.0.4

[PATCH 4/5] OMAP4: powerdomain: Remove L3INIT_PD OFF state

[Benoit Cousson]

From: Santosh Shilimkar <santosh.shilimkar@ti.com>

On OMAP4, there is an issue when L3INIT transitions to OFF mode without
device OFF. The SAR restore mechanism will not get triggered without
wakeup from device OFF and hence the USB host and USB TLL context
will not be restored.

Hardware team recommended to remove the OFF state support for L3INIT_PD
since there is no power impact. It will be removed on next OMAP revision
(OMAP4440 and beyond).

Hence this patch removed the OFF state from L3INIT_PD. The deepest
state supported on L3INIT_PD is OSWR just like CORE_PD and PER_PD

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
[b-cousson@ti.com: update the changelog with next OMAP info]
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomains44xx_data.c b/arch/arm/mach-omap2/powerdomains44xx_data.c
index cec9c09..26d7641 100644
--- a/arch/arm/mach-omap2/powerdomains44xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains44xx_data.c
@@ -267,7 +267,7 @@ static struct powerdomain l3init_44xx_pwrdm = {
 	.prcm_offs	  = OMAP4430_PRM_L3INIT_INST,
 	.prcm_partition	  = OMAP4430_PRM_PARTITION,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_RET_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	= {
-- 
1.7.0.4

[PATCH 4/5] OMAP4: powerdomain: Remove L3INIT_PD OFF state

[Benoit Cousson]

From: Santosh Shilimkar <santosh.shilimkar@ti.com>

On OMAP4, there is an issue when L3INIT transitions to OFF mode without
device OFF. The SAR restore mechanism will not get triggered without
wakeup from device OFF and hence the USB host and USB TLL context
will not be restored.

Hardware team recommended to remove the OFF state support for L3INIT_PD
since there is no power impact. It will be removed on next OMAP revision
(OMAP4440 and beyond).

Hence this patch removed the OFF state from L3INIT_PD. The deepest
state supported on L3INIT_PD is OSWR just like CORE_PD and PER_PD

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
[b-cousson at ti.com: update the changelog with next OMAP info]
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/powerdomains44xx_data.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomains44xx_data.c b/arch/arm/mach-omap2/powerdomains44xx_data.c
index cec9c09..26d7641 100644
--- a/arch/arm/mach-omap2/powerdomains44xx_data.c
+++ b/arch/arm/mach-omap2/powerdomains44xx_data.c
@@ -267,7 +267,7 @@ static struct powerdomain l3init_44xx_pwrdm = {
 	.prcm_offs	  = OMAP4430_PRM_L3INIT_INST,
 	.prcm_partition	  = OMAP4430_PRM_PARTITION,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_RET_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	= {
-- 
1.7.0.4

[PATCH 5/5] OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

[Benoit Cousson]

From: Santosh Shilimkar <santosh.shilimkar@ti.com>

L3INSTR clock domain is read only register and its reset value is
HW_AUTO. The modules withing this clock domain needs to be kept under
hardware control.

MODULEMODE:
- 0x0: Module is disable by software. Any INTRCONN access to module
  results in an error, except if resulting from a module wakeup
  (asynchronous wakeup).
- 0x1: Module is managed automatically by hardware according to
  clock domain transition. A clock domain sleep transition put
  module into idle. A wakeup domain transition put it back
  into function. If CLKTRCTRL=3, any INTRCONN access to module
  is always granted. Module clocks may be gated according to
  the clock domain state.

This patch keeps CM_L3INSTR_L3_3_CLKCTRL, CM_L3INSTR_L3_INSTR_CLKCTRL
and CM_L3INSTR_INTRCONN_WP1_CLKCTRL module mode under hardware control
by using ENABLE_ON_INIT flag.

Without this the OMAP4 device OFF mode SAR restore phase aborts during
interconnect register restore phase. This can be also handled by doing
explicit a clock enable and disable in the low power code since there
is no direct module associated with it. But that seems not necessary
since the clock domain is under HW control.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/clock44xx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 76e900b..a72f6ab 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -1755,6 +1755,7 @@ static struct clk l3_instr_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_L3_INSTR_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
@@ -1765,6 +1766,7 @@ static struct clk l3_main_3_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_L3_3_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
@@ -2069,6 +2071,7 @@ static struct clk ocp_wp_noc_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_OCP_WP1_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
-- 
1.7.0.4

[PATCH 5/5] OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

[Benoit Cousson]

From: Santosh Shilimkar <santosh.shilimkar@ti.com>

L3INSTR clock domain is read only register and its reset value is
HW_AUTO. The modules withing this clock domain needs to be kept under
hardware control.

MODULEMODE:
- 0x0: Module is disable by software. Any INTRCONN access to module
  results in an error, except if resulting from a module wakeup
  (asynchronous wakeup).
- 0x1: Module is managed automatically by hardware according to
  clock domain transition. A clock domain sleep transition put
  module into idle. A wakeup domain transition put it back
  into function. If CLKTRCTRL=3, any INTRCONN access to module
  is always granted. Module clocks may be gated according to
  the clock domain state.

This patch keeps CM_L3INSTR_L3_3_CLKCTRL, CM_L3INSTR_L3_INSTR_CLKCTRL
and CM_L3INSTR_INTRCONN_WP1_CLKCTRL module mode under hardware control
by using ENABLE_ON_INIT flag.

Without this the OMAP4 device OFF mode SAR restore phase aborts during
interconnect register restore phase. This can be also handled by doing
explicit a clock enable and disable in the low power code since there
is no direct module associated with it. But that seems not necessary
since the clock domain is under HW control.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/clock44xx_data.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 76e900b..a72f6ab 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -1755,6 +1755,7 @@ static struct clk l3_instr_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_L3_INSTR_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
@@ -1765,6 +1766,7 @@ static struct clk l3_main_3_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_L3_3_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
@@ -2069,6 +2071,7 @@ static struct clk ocp_wp_noc_ick = {
 	.enable_reg	= OMAP4430_CM_L3INSTR_OCP_WP1_CLKCTRL,
 	.enable_bit	= OMAP4430_MODULEMODE_HWCTRL,
 	.clkdm_name	= "l3_instr_clkdm",
+	.flags		= ENABLE_ON_INIT,
 	.parent		= &l3_div_ck,
 	.recalc		= &followparent_recalc,
 };
-- 
1.7.0.4

Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 470 bytes --]

Hi Benoît, Kevin,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> This is a series of fixes on OMAP3/4 in setup apis,
> in the suspend framework and in powerdomain modelling
> for OMAP4.

...

> Rajendra Nayak (3):
>   OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
>   OMAP4: PM: Do not assume clkdm supports hw transitions

The above two patches should be run by Kevin for his ack (cc'ed on this 
message).  Kevin, care to ack these?



- Paul

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

Hi Beno?t, Kevin,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> This is a series of fixes on OMAP3/4 in setup apis,
> in the suspend framework and in powerdomain modelling
> for OMAP4.

...

> Rajendra Nayak (3):
>   OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
>   OMAP4: PM: Do not assume clkdm supports hw transitions

The above two patches should be run by Kevin for his ack (cc'ed on this 
message).  Kevin, care to ack these?



- Paul

Re: [PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> From: Rajendra Nayak <rnayak@ti.com>
>
> For pwrdm's which support lowperstatechange, do not try waking
> up the domain to put it back to deeper sleep state.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>

Re: $SUBJECT, the rather than using the cryptic lowpwrstchange name from the 
pwrdm API, just use the more readable "low-power state change"

Also, readability/flow, I have some questions/comments...

> ---
>  arch/arm/mach-omap2/pm.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index cf1c4c9..dc68044 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -114,6 +114,14 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  		return ret;
>  
>  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> +				ret = pwrdm_set_next_pwrst(pwrdm, state);
> +				pwrdm_set_lowpwrstchange(pwrdm);
> +				pwrdm_wait_transition(pwrdm);
> +				pwrdm_state_switch(pwrdm);
> +				return ret;

Personally, I'd prefer if this function flowed through better instead of
the early return in order to emphasize the common code.

Rather than the return here, can you just set the low-power state change
bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
clause?

Or, does the next state have to be set before the low-power state change
bit?

Basically, what I'm getting at is this should be a single function with
common flow.  The conditional code based on low-power state change
should be isolated instead of having a special path.

Kevin

> +		}
>  		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>  		sleep_switch = 1;
>  		pwrdm_wait_transition(pwrdm);

[PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> From: Rajendra Nayak <rnayak@ti.com>
>
> For pwrdm's which support lowperstatechange, do not try waking
> up the domain to put it back to deeper sleep state.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>

Re: $SUBJECT, the rather than using the cryptic lowpwrstchange name from the 
pwrdm API, just use the more readable "low-power state change"

Also, readability/flow, I have some questions/comments...

> ---
>  arch/arm/mach-omap2/pm.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index cf1c4c9..dc68044 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -114,6 +114,14 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  		return ret;
>  
>  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> +				ret = pwrdm_set_next_pwrst(pwrdm, state);
> +				pwrdm_set_lowpwrstchange(pwrdm);
> +				pwrdm_wait_transition(pwrdm);
> +				pwrdm_state_switch(pwrdm);
> +				return ret;

Personally, I'd prefer if this function flowed through better instead of
the early return in order to emphasize the common code.

Rather than the return here, can you just set the low-power state change
bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
clause?

Or, does the next state have to be set before the low-power state change
bit?

Basically, what I'm getting at is this should be a single function with
common flow.  The conditional code based on low-power state change
should be isolated instead of having a special path.

Kevin

> +		}
>  		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>  		sleep_switch = 1;
>  		pwrdm_wait_transition(pwrdm);

Re: [PATCH 2/5] OMAP4: PM: Do not assume clkdm supports hw transitions

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> From: Rajendra Nayak <rnayak@ti.com>
>
> omap_set_pwrdm_state today assumes a clkdm supports hw_auto
> transitions and hence leaves some which do not support this
> in sw wkup state preventing low power transitions.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

> ---
>  arch/arm/mach-omap2/pm.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index dc68044..a2a70e1 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -91,8 +91,7 @@ static void omap2_init_processor_devices(void)
>  
>  /*
>   * This sets pwrdm state (other than mpu & core. Currently only ON &
> - * RET are supported. Function is assuming that clkdm doesn't have
> - * hw_sup mode enabled.
> + * RET are supported.
>   */
>  int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  {
> @@ -135,7 +134,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	}
>  
>  	if (sleep_switch) {
> -		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
> +			omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +		else
> +			omap2_clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
>  		pwrdm_wait_transition(pwrdm);
>  		pwrdm_state_switch(pwrdm);
>  	}

[PATCH 2/5] OMAP4: PM: Do not assume clkdm supports hw transitions

[Kevin Hilman]

Benoit Cousson <b-cousson@ti.com> writes:

> From: Rajendra Nayak <rnayak@ti.com>
>
> omap_set_pwrdm_state today assumes a clkdm supports hw_auto
> transitions and hence leaves some which do not support this
> in sw wkup state preventing low power transitions.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>

Acked-by: Kevin Hilman <khilman@deeprootsystems.com>

> ---
>  arch/arm/mach-omap2/pm.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
> index dc68044..a2a70e1 100644
> --- a/arch/arm/mach-omap2/pm.c
> +++ b/arch/arm/mach-omap2/pm.c
> @@ -91,8 +91,7 @@ static void omap2_init_processor_devices(void)
>  
>  /*
>   * This sets pwrdm state (other than mpu & core. Currently only ON &
> - * RET are supported. Function is assuming that clkdm doesn't have
> - * hw_sup mode enabled.
> + * RET are supported.
>   */
>  int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  {
> @@ -135,7 +134,10 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	}
>  
>  	if (sleep_switch) {
> -		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
> +			omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> +		else
> +			omap2_clkdm_sleep(pwrdm->pwrdm_clkdms[0]);
>  		pwrdm_wait_transition(pwrdm);
>  		pwrdm_state_switch(pwrdm);
>  	}

Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Benoît, Kevin,
>
> On Mon, 13 Dec 2010, Benoit Cousson wrote:
>
>> This is a series of fixes on OMAP3/4 in setup apis,
>> in the suspend framework and in powerdomain modelling
>> for OMAP4.
>
> ...
>
>> Rajendra Nayak (3):
>>   OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
>>   OMAP4: PM: Do not assume clkdm supports hw transitions
>
> The above two patches should be run by Kevin for his ack (cc'ed on this 
> message).  Kevin, care to ack these?

I had some comments on the first, and ack'd the second.

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

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Beno?t, Kevin,
>
> On Mon, 13 Dec 2010, Benoit Cousson wrote:
>
>> This is a series of fixes on OMAP3/4 in setup apis,
>> in the suspend framework and in powerdomain modelling
>> for OMAP4.
>
> ...
>
>> Rajendra Nayak (3):
>>   OMAP4: PM: Use the lowpwrstatechange feature on OMAP4
>>   OMAP4: PM: Do not assume clkdm supports hw transitions
>
> The above two patches should be run by Kevin for his ack (cc'ed on this 
> message).  Kevin, care to ack these?

I had some comments on the first, and ack'd the second.

Kevin

RE: [PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Rajendra Nayak]

Hi Kevin,

<snip>..

> >  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> > +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> > +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> > +				ret = pwrdm_set_next_pwrst(pwrdm, state);
> > +				pwrdm_set_lowpwrstchange(pwrdm);
> > +				pwrdm_wait_transition(pwrdm);
> > +				pwrdm_state_switch(pwrdm);
> > +				return ret;
>
> Personally, I'd prefer if this function flowed through better instead of
> the early return in order to emphasize the common code.
>
> Rather than the return here, can you just set the low-power state change
> bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
> clause?
>
> Or, does the next state have to be set before the low-power state change
> bit?

Yes, that sequencing is needed.

>
> Basically, what I'm getting at is this should be a single function with
> common flow.  The conditional code based on low-power state change
> should be isolated instead of having a special path.

I get your point. See if the below approach looks better.
If it looks fine, I'll do some more testing (currently only tested
on OMAP4430sdp) and repost the 2 patches.

>From 5d206ba908071edafae6c044bd3ef6ad8a9c32e7 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak <rnayak@ti.com>
Date: Thu, 25 Nov 2010 14:26:51 +0530
Subject: [PATCH 1/5] OMAP4: PM: Use the low-power state change feature on
OMAP4

For pwrdm's which support LOWPOWERSTATECHANGE, do not try waking
up the domain to put it back to deeper sleep state.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Acked-by: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/pm.c |   28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Index: linux/arch/arm/mach-omap2/pm.c
===================================================================
--- linux.orig/arch/arm/mach-omap2/pm.c	2010-12-15 17:29:42.361228780
+0530
+++ linux/arch/arm/mach-omap2/pm.c	2010-12-15 20:19:48.321228780
+0530
@@ -89,6 +89,10 @@
 	}
 }

+/* Types of sleep_switch used in omap_set_pwrdm_state */
+#define FORCEWAKEUP_SWITCH	0
+#define LOWPOWERSTATE_SWITCH	1
+
 /*
  * This sets pwrdm state (other than mpu & core. Currently only ON &
  * RET are supported. Function is assuming that clkdm doesn't have
@@ -114,9 +118,14 @@
 		return ret;

 	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
-		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
-		sleep_switch = 1;
-		pwrdm_wait_transition(pwrdm);
+		if ((pwrdm_read_pwrst(pwrdm) > state) &&
+			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
+			sleep_switch = LOWPOWERSTATE_SWITCH;
+		} else {
+			omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
+			pwrdm_wait_transition(pwrdm);
+			sleep_switch = FORCEWAKEUP_SWITCH;
+		}
 	}

 	ret = pwrdm_set_next_pwrst(pwrdm, state);
@@ -126,12 +135,19 @@
 		goto err;
 	}

-	if (sleep_switch) {
+	switch (sleep_switch) {
+	case FORCEWAKEUP_SWITCH:
 		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
-		pwrdm_wait_transition(pwrdm);
-		pwrdm_state_switch(pwrdm);
+		break;
+	case LOWPOWERSTATE_SWITCH:
+		pwrdm_set_lowpwrstchange(pwrdm);
+		break;
+	default:
+		return ret;
 	}

+	pwrdm_wait_transition(pwrdm);
+	pwrdm_state_switch(pwrdm);
 err:
 	return ret;
 }


>
> Kevin
>
> > +		}
> >  		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> >  		sleep_switch = 1;
> >  		pwrdm_wait_transition(pwrdm);

[PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Rajendra Nayak]

Hi Kevin,

<snip>..

> >  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> > +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> > +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> > +				ret = pwrdm_set_next_pwrst(pwrdm, state);
> > +				pwrdm_set_lowpwrstchange(pwrdm);
> > +				pwrdm_wait_transition(pwrdm);
> > +				pwrdm_state_switch(pwrdm);
> > +				return ret;
>
> Personally, I'd prefer if this function flowed through better instead of
> the early return in order to emphasize the common code.
>
> Rather than the return here, can you just set the low-power state change
> bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
> clause?
>
> Or, does the next state have to be set before the low-power state change
> bit?

Yes, that sequencing is needed.

>
> Basically, what I'm getting at is this should be a single function with
> common flow.  The conditional code based on low-power state change
> should be isolated instead of having a special path.

I get your point. See if the below approach looks better.
If it looks fine, I'll do some more testing (currently only tested
on OMAP4430sdp) and repost the 2 patches.

Re: [PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Kevin Hilman]

Rajendra Nayak <rnayak@ti.com> writes:

> Hi Kevin,
>
> <snip>..
>
>> >  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
>> > +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
>> > +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
>> > +				ret = pwrdm_set_next_pwrst(pwrdm, state);
>> > +				pwrdm_set_lowpwrstchange(pwrdm);
>> > +				pwrdm_wait_transition(pwrdm);
>> > +				pwrdm_state_switch(pwrdm);
>> > +				return ret;
>>
>> Personally, I'd prefer if this function flowed through better instead of
>> the early return in order to emphasize the common code.
>>
>> Rather than the return here, can you just set the low-power state change
>> bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
>> clause?
>>
>> Or, does the next state have to be set before the low-power state change
>> bit?
>
> Yes, that sequencing is needed.
>
>>
>> Basically, what I'm getting at is this should be a single function with
>> common flow.  The conditional code based on low-power state change
>> should be isolated instead of having a special path.
>
> I get your point. See if the below approach looks better.
> If it looks fine, I'll do some more testing (currently only tested
> on OMAP4430sdp) and repost the 2 patches.

Yes, this approach leads to better readability IMO.

Thanks,

Kevin

> From 5d206ba908071edafae6c044bd3ef6ad8a9c32e7 Mon Sep 17 00:00:00 2001
> From: Rajendra Nayak <rnayak@ti.com>
> Date: Thu, 25 Nov 2010 14:26:51 +0530
> Subject: [PATCH 1/5] OMAP4: PM: Use the low-power state change feature on
> OMAP4
>
> For pwrdm's which support LOWPOWERSTATECHANGE, do not try waking
> up the domain to put it back to deeper sleep state.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |   28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> Index: linux/arch/arm/mach-omap2/pm.c
> ===================================================================
> --- linux.orig/arch/arm/mach-omap2/pm.c	2010-12-15 17:29:42.361228780
> +0530
> +++ linux/arch/arm/mach-omap2/pm.c	2010-12-15 20:19:48.321228780
> +0530
> @@ -89,6 +89,10 @@
>  	}
>  }
>
> +/* Types of sleep_switch used in omap_set_pwrdm_state */
> +#define FORCEWAKEUP_SWITCH	0
> +#define LOWPOWERSTATE_SWITCH	1
> +
>  /*
>   * This sets pwrdm state (other than mpu & core. Currently only ON &
>   * RET are supported. Function is assuming that clkdm doesn't have
> @@ -114,9 +118,14 @@
>  		return ret;
>
>  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> -		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> -		sleep_switch = 1;
> -		pwrdm_wait_transition(pwrdm);
> +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> +			sleep_switch = LOWPOWERSTATE_SWITCH;
> +		} else {
> +			omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> +			pwrdm_wait_transition(pwrdm);
> +			sleep_switch = FORCEWAKEUP_SWITCH;
> +		}
>  	}
>
>  	ret = pwrdm_set_next_pwrst(pwrdm, state);
> @@ -126,12 +135,19 @@
>  		goto err;
>  	}
>
> -	if (sleep_switch) {
> +	switch (sleep_switch) {
> +	case FORCEWAKEUP_SWITCH:
>  		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> -		pwrdm_wait_transition(pwrdm);
> -		pwrdm_state_switch(pwrdm);
> +		break;
> +	case LOWPOWERSTATE_SWITCH:
> +		pwrdm_set_lowpwrstchange(pwrdm);
> +		break;
> +	default:
> +		return ret;
>  	}
>
> +	pwrdm_wait_transition(pwrdm);
> +	pwrdm_state_switch(pwrdm);
>  err:
>  	return ret;
>  }
>
>
>>
>> Kevin
>>
>> > +		}
>> >  		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>> >  		sleep_switch = 1;
>> >  		pwrdm_wait_transition(pwrdm);

[PATCH 1/5] OMAP4: PM: Use the lowpwrstatechange feature on OMAP4

[Kevin Hilman]

Rajendra Nayak <rnayak@ti.com> writes:

> Hi Kevin,
>
> <snip>..
>
>> >  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
>> > +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
>> > +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
>> > +				ret = pwrdm_set_next_pwrst(pwrdm, state);
>> > +				pwrdm_set_lowpwrstchange(pwrdm);
>> > +				pwrdm_wait_transition(pwrdm);
>> > +				pwrdm_state_switch(pwrdm);
>> > +				return ret;
>>
>> Personally, I'd prefer if this function flowed through better instead of
>> the early return in order to emphasize the common code.
>>
>> Rather than the return here, can you just set the low-power state change
>> bit here (and put the clkdm_wakeup + sleep_switch = 1 into the else
>> clause?
>>
>> Or, does the next state have to be set before the low-power state change
>> bit?
>
> Yes, that sequencing is needed.
>
>>
>> Basically, what I'm getting at is this should be a single function with
>> common flow.  The conditional code based on low-power state change
>> should be isolated instead of having a special path.
>
> I get your point. See if the below approach looks better.
> If it looks fine, I'll do some more testing (currently only tested
> on OMAP4430sdp) and repost the 2 patches.

Yes, this approach leads to better readability IMO.

Thanks,

Kevin

> From 5d206ba908071edafae6c044bd3ef6ad8a9c32e7 Mon Sep 17 00:00:00 2001
> From: Rajendra Nayak <rnayak@ti.com>
> Date: Thu, 25 Nov 2010 14:26:51 +0530
> Subject: [PATCH 1/5] OMAP4: PM: Use the low-power state change feature on
> OMAP4
>
> For pwrdm's which support LOWPOWERSTATECHANGE, do not try waking
> up the domain to put it back to deeper sleep state.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Acked-by: Benoit Cousson <b-cousson@ti.com>
> ---
>  arch/arm/mach-omap2/pm.c |   28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> Index: linux/arch/arm/mach-omap2/pm.c
> ===================================================================
> --- linux.orig/arch/arm/mach-omap2/pm.c	2010-12-15 17:29:42.361228780
> +0530
> +++ linux/arch/arm/mach-omap2/pm.c	2010-12-15 20:19:48.321228780
> +0530
> @@ -89,6 +89,10 @@
>  	}
>  }
>
> +/* Types of sleep_switch used in omap_set_pwrdm_state */
> +#define FORCEWAKEUP_SWITCH	0
> +#define LOWPOWERSTATE_SWITCH	1
> +
>  /*
>   * This sets pwrdm state (other than mpu & core. Currently only ON &
>   * RET are supported. Function is assuming that clkdm doesn't have
> @@ -114,9 +118,14 @@
>  		return ret;
>
>  	if (pwrdm_read_pwrst(pwrdm) < PWRDM_POWER_ON) {
> -		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> -		sleep_switch = 1;
> -		pwrdm_wait_transition(pwrdm);
> +		if ((pwrdm_read_pwrst(pwrdm) > state) &&
> +			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
> +			sleep_switch = LOWPOWERSTATE_SWITCH;
> +		} else {
> +			omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
> +			pwrdm_wait_transition(pwrdm);
> +			sleep_switch = FORCEWAKEUP_SWITCH;
> +		}
>  	}
>
>  	ret = pwrdm_set_next_pwrst(pwrdm, state);
> @@ -126,12 +135,19 @@
>  		goto err;
>  	}
>
> -	if (sleep_switch) {
> +	switch (sleep_switch) {
> +	case FORCEWAKEUP_SWITCH:
>  		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
> -		pwrdm_wait_transition(pwrdm);
> -		pwrdm_state_switch(pwrdm);
> +		break;
> +	case LOWPOWERSTATE_SWITCH:
> +		pwrdm_set_lowpwrstchange(pwrdm);
> +		break;
> +	default:
> +		return ret;
>  	}
>
> +	pwrdm_wait_transition(pwrdm);
> +	pwrdm_state_switch(pwrdm);
>  err:
>  	return ret;
>  }
>
>
>>
>> Kevin
>>
>> > +		}
>> >  		omap2_clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>> >  		sleep_switch = 1;
>> >  		pwrdm_wait_transition(pwrdm);

Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 930 bytes --]

Hi Rajendra, Santosh, Benoît, Kevin,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> This is a series of fixes on OMAP3/4 in setup apis,
> in the suspend framework and in powerdomain modelling
> for OMAP4.
> 
> The series is based on your power domain series and is available here:
> git://gitorious.org/omap-pm/linux.git for_2.6.38/power
> 
> Tested on 4430sdp + ES2.0 with omap2plus_defconfig.
> Tested on 3430sdp - validate OFF in suspend, with omap3_pm_defconfig
> from Kevin's tree.
> Tested on 2430sdp after fixing the break on linux-omap master.
> See http://www.spinics.net/lists/linux-omap/msg42050.html

Thanks, queued for 2.6.38 in the 'omap4_pm_clk_pwrdm_a_2.6.38' branch of 
git://git.pwsan.com/linux-2.6.  I used the v2 versions of the first and 
second patches.

Also added to the integration branch at tag 
'integration-2.6.38-20101221-009' of git://git.pwsan.com/linux-integration


- Paul

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

Hi Rajendra, Santosh, Beno?t, Kevin,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> This is a series of fixes on OMAP3/4 in setup apis,
> in the suspend framework and in powerdomain modelling
> for OMAP4.
> 
> The series is based on your power domain series and is available here:
> git://gitorious.org/omap-pm/linux.git for_2.6.38/power
> 
> Tested on 4430sdp + ES2.0 with omap2plus_defconfig.
> Tested on 3430sdp - validate OFF in suspend, with omap3_pm_defconfig
> from Kevin's tree.
> Tested on 2430sdp after fixing the break on linux-omap master.
> See http://www.spinics.net/lists/linux-omap/msg42050.html

Thanks, queued for 2.6.38 in the 'omap4_pm_clk_pwrdm_a_2.6.38' branch of 
git://git.pwsan.com/linux-2.6.  I used the v2 versions of the first and 
second patches.

Also added to the integration branch at tag 
'integration-2.6.38-20101221-009' of git://git.pwsan.com/linux-integration


- Paul

Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

Hi Santosh,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> Santosh Shilimkar (2):
>   OMAP4: powerdomain: Remove L3INIT_PD OFF state
>   OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

by the way, nice commit messages on these!


- Paul

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

Hi Santosh,

On Mon, 13 Dec 2010, Benoit Cousson wrote:

> Santosh Shilimkar (2):
>   OMAP4: powerdomain: Remove L3INIT_PD OFF state
>   OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW control

by the way, nice commit messages on these!


- Paul

Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

On Tue, 21 Dec 2010, Paul Walmsley wrote:

> Also added to the integration branch at tag 
> 'integration-2.6.38-20101221-009' of git://git.pwsan.com/linux-integration

Correction: these wound up under the tag 'integration-2.6.38-20101221-010' 
instead.


- Paul

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Paul Walmsley]

On Tue, 21 Dec 2010, Paul Walmsley wrote:

> Also added to the integration branch at tag 
> 'integration-2.6.38-20101221-009' of git://git.pwsan.com/linux-integration

Correction: these wound up under the tag 'integration-2.6.38-20101221-010' 
instead.


- Paul

RE: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Santosh Shilimkar]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Wednesday, December 22, 2010 9:14 AM
> To: santosh.shilimkar@ti.com
> Cc: Benoit Cousson; rnayak@ti.com; linux-omap@vger.kernel.org;
linux-arm-
> kernel@lists.infradead.org; khilman@deeprootsystems.com
> Subject: Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain
>
> Hi Santosh,
>
> On Mon, 13 Dec 2010, Benoit Cousson wrote:
>
> > Santosh Shilimkar (2):
> >   OMAP4: powerdomain: Remove L3INIT_PD OFF state
> >   OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW
> control
>
> by the way, nice commit messages on these!
>
Thanks Paul!!

Kevin, Benoit and you are inspiration here.

Regards,
Santosh

[PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain

[Santosh Shilimkar]

> -----Original Message-----
> From: Paul Walmsley [mailto:paul at pwsan.com]
> Sent: Wednesday, December 22, 2010 9:14 AM
> To: santosh.shilimkar at ti.com
> Cc: Benoit Cousson; rnayak at ti.com; linux-omap at vger.kernel.org;
linux-arm-
> kernel at lists.infradead.org; khilman at deeprootsystems.com
> Subject: Re: [PATCH 0/5] OMAP3&4: Fixes in setup/suspend/powerdomain
>
> Hi Santosh,
>
> On Mon, 13 Dec 2010, Benoit Cousson wrote:
>
> > Santosh Shilimkar (2):
> >   OMAP4: powerdomain: Remove L3INIT_PD OFF state
> >   OMAP4: clock data: Keep L3INSTR clock domain modulemode under HW
> control
>
> by the way, nice commit messages on these!
>
Thanks Paul!!

Kevin, Benoit and you are inspiration here.

Regards,
Santosh