diff mbox

[v3,4/8] OMAP2+: PM: idle clkdms only if already in idle

Message ID 1309191103-8817-5-git-send-email-b-cousson@ti.com (mailing list archive)
State New, archived
Delegated to: Benoit Cousson
Headers show

Commit Message

Benoit Cousson June 27, 2011, 4:11 p.m. UTC 
From: Rajendra Nayak <rnayak@ti.com>

The omap_set_pwrdm_state function forces clockdomains
to idle, without checking the existing idle state
programmed, instead based solely on the HW capability
of the clockdomain to support idle.
This is wrong and the clockdomains should be idled
post a state_switch *only* if idle transitions on the
clockdomain were already enabled.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/pm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Todd Poynor June 28, 2011, 1 a.m. UTC | #1 
On Mon, Jun 27, 2011 at 06:11:39PM +0200, Benoit Cousson wrote:
> From: Rajendra Nayak <rnayak@ti.com>
> 
> The omap_set_pwrdm_state function forces clockdomains
> to idle, without checking the existing idle state
> programmed, instead based solely on the HW capability
> of the clockdomain to support idle.
> This is wrong and the clockdomains should be idled
> post a state_switch *only* if idle transitions on the
> clockdomain were already enabled.
...
>  		} else {
> +			hwsup = clkdm_allows_idle(pwrdm->pwrdm_clkdms[0]);
>  			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
>  			pwrdm_wait_transition(pwrdm);
>  			sleep_switch = FORCEWAKEUP_SWITCH;
> @@ -142,7 +144,7 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  
>  	switch (sleep_switch) {
>  	case FORCEWAKEUP_SWITCH:
> -		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
> +		if (hwsup)
>  			clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>  		else
>  			clkdm_sleep(pwrdm->pwrdm_clkdms[0]);

Since it seems these power domain state transitions could be executed
concurrently with other operations on the clock domain that is first
in the list, is there still a chance for a race, such that it may no
longer be appropriate to put the clock domain in h/w-supervised idle
mode after the power domain transition is complete?  If so, perhaps an
atomic wakeup refcount could flag when it is OK to allow idle on the
clock domain.


Todd
--
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
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index d48813f..ce1a9f3 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -108,6 +108,7 @@  int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	u32 cur_state;
 	int sleep_switch = -1;
 	int ret = 0;
+	int hwsup = 0;
 
 	if (pwrdm == NULL || IS_ERR(pwrdm))
 		return -EINVAL;
@@ -127,6 +128,7 @@  int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 			(pwrdm->flags & PWRDM_HAS_LOWPOWERSTATECHANGE)) {
 			sleep_switch = LOWPOWERSTATE_SWITCH;
 		} else {
+			hwsup = clkdm_allows_idle(pwrdm->pwrdm_clkdms[0]);
 			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
 			pwrdm_wait_transition(pwrdm);
 			sleep_switch = FORCEWAKEUP_SWITCH;
@@ -142,7 +144,7 @@  int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 
 	switch (sleep_switch) {
 	case FORCEWAKEUP_SWITCH:
-		if (pwrdm->pwrdm_clkdms[0]->flags & CLKDM_CAN_ENABLE_AUTO)
+		if (hwsup)
 			clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
 		else
 			clkdm_sleep(pwrdm->pwrdm_clkdms[0]);