[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

Program all powerdomain target state as ON; This is to
prevent domains from hitting low power states (if bootloader
has target states set to something other than ON) and potentially
even losing context while PM is not fully initilized.
The PM late init code can then program the desired target
state for all the power domains.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index e0490bc..e61866c 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 
 	list_add(&pwrdm->node, &pwrdm_list);
 
+	/*
+	* Program all powerdomain target state as ON; This is to
+	* prevent domains from hitting low power states (if bootloader
+	* has target states set to something other than ON) and potentially
+	* even losing context while PM is not fully initilized.
+	* The PM late init code can then program the desired target
+	* state for all the power domains.
+	*/
+	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
+
 	/* Initialize the powerdomain's state counter */
 	for (i = 0; i < PWRDM_MAX_PWRSTS; i++)
 		pwrdm->state_counter[i] = 0;
@@ -218,7 +228,7 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 /**
  * pwrdm_init - set up the powerdomain layer
  * @pwrdm_list: array of struct powerdomain pointers to register
- * @custom_funcs: func pointers for arch specific implementations
+ * @custom_funcs: func pointers for arch specfic implementations
  *
  * Loop through the array of powerdomains @pwrdm_list, registering all
  * that are available on the current CPU. If pwrdm_list is supplied
-- 
1.7.4.1

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

Program all powerdomain target state as ON; This is to
prevent domains from hitting low power states (if bootloader
has target states set to something other than ON) and potentially
even losing context while PM is not fully initilized.
The PM late init code can then program the desired target
state for all the power domains.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/powerdomain.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index e0490bc..e61866c 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
 
 	list_add(&pwrdm->node, &pwrdm_list);
 
+	/*
+	* Program all powerdomain target state as ON; This is to
+	* prevent domains from hitting low power states (if bootloader
+	* has target states set to something other than ON) and potentially
+	* even losing context while PM is not fully initilized.
+	* The PM late init code can then program the desired target
+	* state for all the power domains.
+	*/
+	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
+
 	/* Initialize the powerdomain's state counter */
 	for (i = 0; i < PWRDM_MAX_PWRSTS; i++)
 		pwrdm->state_counter[i] = 0;
@@ -218,7 +228,7 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 /**
  * pwrdm_init - set up the powerdomain layer
  * @pwrdm_list: array of struct powerdomain pointers to register
- * @custom_funcs: func pointers for arch specific implementations
+ * @custom_funcs: func pointers for arch specfic implementations
  *
  * Loop through the array of powerdomains @pwrdm_list, registering all
  * that are available on the current CPU. If pwrdm_list is supplied
-- 
1.7.4.1

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh Shilimkar]

While using clockdomain force wakeup method, not waiting for powerdomain
to be effectively ON may end up locking the clockdomain FSM until a
next wakeup event occurs.

One such issue was seen on OMAP4430, where L4_PER was periodically
getting stuck in in-transition state when transitioning from from OSWR to ON.

This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Patrick Titiano <p-titiano@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index b98a972..583cc3d 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
  */
 int clkdm_wakeup(struct clockdomain *clkdm)
 {
+	int ret;
+
 	if (!clkdm)
 		return -EINVAL;
 
@@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
 
 	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
 
-	return arch_clkdm->clkdm_wakeup(clkdm);
+	ret = arch_clkdm->clkdm_wakeup(clkdm);
+	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
+
+	return ret;
 }
 
 /**
-- 
1.7.4.1

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh Shilimkar]

While using clockdomain force wakeup method, not waiting for powerdomain
to be effectively ON may end up locking the clockdomain FSM until a
next wakeup event occurs.

One such issue was seen on OMAP4430, where L4_PER was periodically
getting stuck in in-transition state when transitioning from from OSWR to ON.

This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Patrick Titiano <p-titiano@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index b98a972..583cc3d 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
  */
 int clkdm_wakeup(struct clockdomain *clkdm)
 {
+	int ret;
+
 	if (!clkdm)
 		return -EINVAL;
 
@@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
 
 	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
 
-	return arch_clkdm->clkdm_wakeup(clkdm);
+	ret = arch_clkdm->clkdm_wakeup(clkdm);
+	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
+
+	return ret;
 }
 
 /**
-- 
1.7.4.1

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

On Wed, 13 Jul 2011, Santosh Shilimkar wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> Program all powerdomain target state as ON; This is to
> prevent domains from hitting low power states (if bootloader
> has target states set to something other than ON) and potentially
> even losing context while PM is not fully initilized.
> The PM late init code can then program the desired target
> state for all the power domains.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>

Thanks, dropped the second hunk of the patch and queued for 3.1-rc fixes 
at git://git.pwsan.com/linux-2.6 in the 'pwrdm_clkdm_fixes_3.1rc' branch.

Santosh, looks like this is missing a Signed-off-by: or similar from you.  
Do you want me to add it?


- Paul

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

On Wed, 13 Jul 2011, Santosh Shilimkar wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> Program all powerdomain target state as ON; This is to
> prevent domains from hitting low power states (if bootloader
> has target states set to something other than ON) and potentially
> even losing context while PM is not fully initilized.
> The PM late init code can then program the desired target
> state for all the power domains.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>

Thanks, dropped the second hunk of the patch and queued for 3.1-rc fixes 
at git://git.pwsan.com/linux-2.6 in the 'pwrdm_clkdm_fixes_3.1rc' branch.

Santosh, looks like this is missing a Signed-off-by: or similar from you.  
Do you want me to add it?


- Paul

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]

Hi,

On Wed, Jul 13, 2011 at 08:56:27AM +0530, Santosh Shilimkar wrote:
> From: Rajendra Nayak <rnayak@ti.com>
> 
> Program all powerdomain target state as ON; This is to
> prevent domains from hitting low power states (if bootloader
> has target states set to something other than ON) and potentially
> even losing context while PM is not fully initilized.
> The PM late init code can then program the desired target
> state for all the power domains.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/powerdomain.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index e0490bc..e61866c 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
>  
>  	list_add(&pwrdm->node, &pwrdm_list);
>  
> +	/*
> +	* Program all powerdomain target state as ON; This is to
> +	* prevent domains from hitting low power states (if bootloader
> +	* has target states set to something other than ON) and potentially
> +	* even losing context while PM is not fully initilized.
> +	* The PM late init code can then program the desired target
> +	* state for all the power domains.
> +	*/
> +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);

Just out of curiosity, I was wondering if it really makes sense to power
up all power domains during boot just to avoid loosing context. Doesn't
hwmod/omap_device soft-reset all IPs during initialization ? If that's
really the case, shouldn't we then choose which powerdomains are
strictly necessary for boot and only power those up ?

Sorry if this is a non-sensical question, but I was curious about it
;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

Hi,

On Wed, Jul 13, 2011 at 08:56:27AM +0530, Santosh Shilimkar wrote:
> From: Rajendra Nayak <rnayak@ti.com>
> 
> Program all powerdomain target state as ON; This is to
> prevent domains from hitting low power states (if bootloader
> has target states set to something other than ON) and potentially
> even losing context while PM is not fully initilized.
> The PM late init code can then program the desired target
> state for all the power domains.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
>  arch/arm/mach-omap2/powerdomain.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index e0490bc..e61866c 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
>  
>  	list_add(&pwrdm->node, &pwrdm_list);
>  
> +	/*
> +	* Program all powerdomain target state as ON; This is to
> +	* prevent domains from hitting low power states (if bootloader
> +	* has target states set to something other than ON) and potentially
> +	* even losing context while PM is not fully initilized.
> +	* The PM late init code can then program the desired target
> +	* state for all the power domains.
> +	*/
> +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);

Just out of curiosity, I was wondering if it really makes sense to power
up all power domains during boot just to avoid loosing context. Doesn't
hwmod/omap_device soft-reset all IPs during initialization ? If that's
really the case, shouldn't we then choose which powerdomains are
strictly necessary for boot and only power those up ?

Sorry if this is a non-sensical question, but I was curious about it
;-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110715/e9cd6215/attachment.sig>

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

cc'ing Patrick

Hi Rajendra, Santosh,

some comments here:

On Wed, 13 Jul 2011, Santosh Shilimkar wrote:

> While using clockdomain force wakeup method, not waiting for powerdomain
> to be effectively ON may end up locking the clockdomain FSM until a
> next wakeup event occurs.
> 
> One such issue was seen on OMAP4430, where L4_PER was periodically
> getting stuck in in-transition state when transitioning from from OSWR to ON.
> 
> This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Reported-by: Patrick Titiano <p-titiano@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
>  arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index b98a972..583cc3d 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>   */
>  int clkdm_wakeup(struct clockdomain *clkdm)
>  {
> +	int ret;
> +
>  	if (!clkdm)
>  		return -EINVAL;
>  
> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>  
>  	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>  
> -	return arch_clkdm->clkdm_wakeup(clkdm);
> +	ret = arch_clkdm->clkdm_wakeup(clkdm);
> +	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);

Seems like this should just call pwrdm_state_switch() or 
pwrdm_clkdm_state_switch()?  (This second function looks superfluous, we 
should probably get rid of it.)

Shouldn't this be added to all of 
clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?

> +
> +	return ret;
>  }
>  
>  /**
> -- 
> 1.7.4.1
> 


- Paul

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

cc'ing Patrick

Hi Rajendra, Santosh,

some comments here:

On Wed, 13 Jul 2011, Santosh Shilimkar wrote:

> While using clockdomain force wakeup method, not waiting for powerdomain
> to be effectively ON may end up locking the clockdomain FSM until a
> next wakeup event occurs.
> 
> One such issue was seen on OMAP4430, where L4_PER was periodically
> getting stuck in in-transition state when transitioning from from OSWR to ON.
> 
> This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>
> 
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Reported-by: Patrick Titiano <p-titiano@ti.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> ---
>  arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
> index b98a972..583cc3d 100644
> --- a/arch/arm/mach-omap2/clockdomain.c
> +++ b/arch/arm/mach-omap2/clockdomain.c
> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>   */
>  int clkdm_wakeup(struct clockdomain *clkdm)
>  {
> +	int ret;
> +
>  	if (!clkdm)
>  		return -EINVAL;
>  
> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>  
>  	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>  
> -	return arch_clkdm->clkdm_wakeup(clkdm);
> +	ret = arch_clkdm->clkdm_wakeup(clkdm);
> +	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);

Seems like this should just call pwrdm_state_switch() or 
pwrdm_clkdm_state_switch()?  (This second function looks superfluous, we 
should probably get rid of it.)

Shouldn't this be added to all of 
clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?

> +
> +	return ret;
>  }
>  
>  /**
> -- 
> 1.7.4.1
> 


- Paul

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> > diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> > index e0490bc..e61866c 100644
> > --- a/arch/arm/mach-omap2/powerdomain.c
> > +++ b/arch/arm/mach-omap2/powerdomain.c
> > @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
> >  
> >  	list_add(&pwrdm->node, &pwrdm_list);
> >  
> > +	/*
> > +	* Program all powerdomain target state as ON; This is to
> > +	* prevent domains from hitting low power states (if bootloader
> > +	* has target states set to something other than ON) and potentially
> > +	* even losing context while PM is not fully initilized.
> > +	* The PM late init code can then program the desired target
> > +	* state for all the power domains.
> > +	*/
> > +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
> 
> Just out of curiosity, I was wondering if it really makes sense to power
> up all power domains during boot just to avoid loosing context. Doesn't
> hwmod/omap_device soft-reset all IPs during initialization ? If that's
> really the case, shouldn't we then choose which powerdomains are
> strictly necessary for boot and only power those up ?
> 
> Sorry if this is a non-sensical question, but I was curious about it
> ;-)

This patch only sets the powerdomain's next power state to ON.  It doesn't 
affect the current power state of the powerdomain.

Let's say that the bootloader, previous OS (in the kexec case), or ROM 
code programs the next power state of some powerdomains to OFF.  Let's 
also say that the kernel that is booted has PM disabled.  The moment a 
powerdomain's clockdomains go inactive, the powerdomain will then switch 
off and all devices in that powerdomain will lose context.  On a 
non-PM-enabled kernel, that will be unexpected and will probably cause the 
system to crash.  This patch prevents that from happening.


- Paul

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> > diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> > index e0490bc..e61866c 100644
> > --- a/arch/arm/mach-omap2/powerdomain.c
> > +++ b/arch/arm/mach-omap2/powerdomain.c
> > @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
> >  
> >  	list_add(&pwrdm->node, &pwrdm_list);
> >  
> > +	/*
> > +	* Program all powerdomain target state as ON; This is to
> > +	* prevent domains from hitting low power states (if bootloader
> > +	* has target states set to something other than ON) and potentially
> > +	* even losing context while PM is not fully initilized.
> > +	* The PM late init code can then program the desired target
> > +	* state for all the power domains.
> > +	*/
> > +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
> 
> Just out of curiosity, I was wondering if it really makes sense to power
> up all power domains during boot just to avoid loosing context. Doesn't
> hwmod/omap_device soft-reset all IPs during initialization ? If that's
> really the case, shouldn't we then choose which powerdomains are
> strictly necessary for boot and only power those up ?
> 
> Sorry if this is a non-sensical question, but I was curious about it
> ;-)

This patch only sets the powerdomain's next power state to ON.  It doesn't 
affect the current power state of the powerdomain.

Let's say that the bootloader, previous OS (in the kexec case), or ROM 
code programs the next power state of some powerdomains to OFF.  Let's 
also say that the kernel that is booted has PM disabled.  The moment a 
powerdomain's clockdomains go inactive, the powerdomain will then switch 
off and all devices in that powerdomain will lose context.  On a 
non-PM-enabled kernel, that will be unexpected and will probably cause the 
system to crash.  This patch prevents that from happening.


- Paul

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2130 bytes --]

Hi,

On Fri, Jul 15, 2011 at 02:10:46AM -0600, Paul Walmsley wrote:
> > > diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> > > index e0490bc..e61866c 100644
> > > --- a/arch/arm/mach-omap2/powerdomain.c
> > > +++ b/arch/arm/mach-omap2/powerdomain.c
> > > @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
> > >  
> > >  	list_add(&pwrdm->node, &pwrdm_list);
> > >  
> > > +	/*
> > > +	* Program all powerdomain target state as ON; This is to
> > > +	* prevent domains from hitting low power states (if bootloader
> > > +	* has target states set to something other than ON) and potentially
> > > +	* even losing context while PM is not fully initilized.
> > > +	* The PM late init code can then program the desired target
> > > +	* state for all the power domains.
> > > +	*/
> > > +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
> > 
> > Just out of curiosity, I was wondering if it really makes sense to power
> > up all power domains during boot just to avoid loosing context. Doesn't
> > hwmod/omap_device soft-reset all IPs during initialization ? If that's
> > really the case, shouldn't we then choose which powerdomains are
> > strictly necessary for boot and only power those up ?
> > 
> > Sorry if this is a non-sensical question, but I was curious about it
> > ;-)
> 
> This patch only sets the powerdomain's next power state to ON.  It doesn't 
> affect the current power state of the powerdomain.
> 
> Let's say that the bootloader, previous OS (in the kexec case), or ROM 
> code programs the next power state of some powerdomains to OFF.  Let's 
> also say that the kernel that is booted has PM disabled.  The moment a 
> powerdomain's clockdomains go inactive, the powerdomain will then switch 
> off and all devices in that powerdomain will lose context.  On a 
> non-PM-enabled kernel, that will be unexpected and will probably cause the 
> system to crash.  This patch prevents that from happening.

I see... thanks for clarifying. The comment above the code wasn't really
clear about it to me.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

Hi,

On Fri, Jul 15, 2011 at 02:10:46AM -0600, Paul Walmsley wrote:
> > > diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> > > index e0490bc..e61866c 100644
> > > --- a/arch/arm/mach-omap2/powerdomain.c
> > > +++ b/arch/arm/mach-omap2/powerdomain.c
> > > @@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)
> > >  
> > >  	list_add(&pwrdm->node, &pwrdm_list);
> > >  
> > > +	/*
> > > +	* Program all powerdomain target state as ON; This is to
> > > +	* prevent domains from hitting low power states (if bootloader
> > > +	* has target states set to something other than ON) and potentially
> > > +	* even losing context while PM is not fully initilized.
> > > +	* The PM late init code can then program the desired target
> > > +	* state for all the power domains.
> > > +	*/
> > > +	pwrdm_set_next_pwrst(pwrdm, PWRDM_POWER_ON);
> > 
> > Just out of curiosity, I was wondering if it really makes sense to power
> > up all power domains during boot just to avoid loosing context. Doesn't
> > hwmod/omap_device soft-reset all IPs during initialization ? If that's
> > really the case, shouldn't we then choose which powerdomains are
> > strictly necessary for boot and only power those up ?
> > 
> > Sorry if this is a non-sensical question, but I was curious about it
> > ;-)
> 
> This patch only sets the powerdomain's next power state to ON.  It doesn't 
> affect the current power state of the powerdomain.
> 
> Let's say that the bootloader, previous OS (in the kexec case), or ROM 
> code programs the next power state of some powerdomains to OFF.  Let's 
> also say that the kernel that is booted has PM disabled.  The moment a 
> powerdomain's clockdomains go inactive, the powerdomain will then switch 
> off and all devices in that powerdomain will lose context.  On a 
> non-PM-enabled kernel, that will be unexpected and will probably cause the 
> system to crash.  This patch prevents that from happening.

I see... thanks for clarifying. The comment above the code wasn't really
clear about it to me.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110715/29a18d47/attachment.sig>

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi Felipe,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> I see... thanks for clarifying. The comment above the code wasn't really
> clear about it to me.

No worries.  If you propose a clarification, I'll stick that into the 
patch.


- Paul

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi Felipe,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> I see... thanks for clarifying. The comment above the code wasn't really
> clear about it to me.

No worries.  If you propose a clarification, I'll stick that into the 
patch.


- Paul

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Hi,

On Fri, Jul 15, 2011 at 02:23:16AM -0600, Paul Walmsley wrote:
> Hi Felipe,
> 
> On Fri, 15 Jul 2011, Felipe Balbi wrote:
> 
> > I see... thanks for clarifying. The comment above the code wasn't really
> > clear about it to me.
> 
> No worries.  If you propose a clarification, I'll stick that into the 
> patch.

How about something like:

/*
 * Program target state of all power domains to ON. This is to prevent
 * power domains from hitting low power states during boot up and
 * potentially causing accesses to the address space of an IP while it
 * is disabled.
 *
 * PM late init code will make sure of disabling all unused IPs later.
 */

not sure if it's a lot better though.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Felipe Balbi]

Hi,

On Fri, Jul 15, 2011 at 02:23:16AM -0600, Paul Walmsley wrote:
> Hi Felipe,
> 
> On Fri, 15 Jul 2011, Felipe Balbi wrote:
> 
> > I see... thanks for clarifying. The comment above the code wasn't really
> > clear about it to me.
> 
> No worries.  If you propose a clarification, I'll stick that into the 
> patch.

How about something like:

/*
 * Program target state of all power domains to ON. This is to prevent
 * power domains from hitting low power states during boot up and
 * potentially causing accesses to the address space of an IP while it
 * is disabled.
 *
 * PM late init code will make sure of disabling all unused IPs later.
 */

not sure if it's a lot better though.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110715/9b5e456c/attachment.sig>

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Santosh Shilimkar]

On 7/15/2011 12:54 AM, Paul Walmsley wrote:
> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>
>> From: Rajendra Nayak<rnayak@ti.com>
>>
>> Program all powerdomain target state as ON; This is to
>> prevent domains from hitting low power states (if bootloader
>> has target states set to something other than ON) and potentially
>> even losing context while PM is not fully initilized.
>> The PM late init code can then program the desired target
>> state for all the power domains.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>
> Thanks, dropped the second hunk of the patch and queued for 3.1-rc fixes
> at git://git.pwsan.com/linux-2.6 in the 'pwrdm_clkdm_fixes_3.1rc' branch.
>
> Santosh, looks like this is missing a Signed-off-by: or similar from you.
> Do you want me to add it?
>
Sure Paul. I missed to add it somehow while posting it.

Regards
Santosh

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Santosh Shilimkar]

On 7/15/2011 12:54 AM, Paul Walmsley wrote:
> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>
>> From: Rajendra Nayak<rnayak@ti.com>
>>
>> Program all powerdomain target state as ON; This is to
>> prevent domains from hitting low power states (if bootloader
>> has target states set to something other than ON) and potentially
>> even losing context while PM is not fully initilized.
>> The PM late init code can then program the desired target
>> state for all the power domains.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>
> Thanks, dropped the second hunk of the patch and queued for 3.1-rc fixes
> at git://git.pwsan.com/linux-2.6 in the 'pwrdm_clkdm_fixes_3.1rc' branch.
>
> Santosh, looks like this is missing a Signed-off-by: or similar from you.
> Do you want me to add it?
>
Sure Paul. I missed to add it somehow while posting it.

Regards
Santosh

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh Shilimkar]

Hi Paul,

On 7/15/2011 1:03 AM, Paul Walmsley wrote:
> cc'ing Patrick
>
> Hi Rajendra, Santosh,
>
> some comments here:
>
> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>
>> While using clockdomain force wakeup method, not waiting for powerdomain
>> to be effectively ON may end up locking the clockdomain FSM until a
>> next wakeup event occurs.
>>
>> One such issue was seen on OMAP4430, where L4_PER was periodically
>> getting stuck in in-transition state when transitioning from from OSWR to ON.
>>
>> This issue was reported and investigated by Patrick Titiano<p-titiano@ti.com>
>>
>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> Reported-by: Patrick Titiano<p-titiano@ti.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>> Cc: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> ---
>>   arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
>>   1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
>> index b98a972..583cc3d 100644
>> --- a/arch/arm/mach-omap2/clockdomain.c
>> +++ b/arch/arm/mach-omap2/clockdomain.c
>> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>>    */
>>   int clkdm_wakeup(struct clockdomain *clkdm)
>>   {
>> +	int ret;
>> +
>>   	if (!clkdm)
>>   		return -EINVAL;
>>
>> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>>
>>   	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>>
>> -	return arch_clkdm->clkdm_wakeup(clkdm);
>> +	ret = arch_clkdm->clkdm_wakeup(clkdm);
>> +	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
>
> Seems like this should just call pwrdm_state_switch() or
> pwrdm_clkdm_state_switch()?  (This second function looks superfluous, we
> should probably get rid of it.)
>
This comment was expected since initially we thought of using
pwrdm_clkdm_state_switch

pwrdm_clkdm_state_switch()
	|--> pwrdm_wait_transition()
	|--> pwrdm_state_switch()

What we need is only first function to ensure that we don't
proceed when PD is in middle of transition.

The second function is actually just doing those debug counter
updates and we wanted to avoid that since it's a live path.
Rajendra observed some huge latencies while doing the
profiling and the suspect was "pwrdm_state_switch()" which
actually keeps adding overhead because of those counter
updates.

> Shouldn't this be added to all of
> clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
>
clkdm_allow_idle() already has the power-domain wait.
This patch adds it for clkdm_wakeup() function.
clkdm_sleep(), this shouldn't be applicable since power
domain sleep transition is depdent on other clockdomains
in the PD and it's a lazy operation.
That leaves clkdm_deny_idle() and I guess we should
address this API as well.

If you agree with above, I can update the patch to address
remaining clkdm_deny_idle() function.

Regards
Santosh
Regards
Santosh

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh Shilimkar]

Hi Paul,

On 7/15/2011 1:03 AM, Paul Walmsley wrote:
> cc'ing Patrick
>
> Hi Rajendra, Santosh,
>
> some comments here:
>
> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>
>> While using clockdomain force wakeup method, not waiting for powerdomain
>> to be effectively ON may end up locking the clockdomain FSM until a
>> next wakeup event occurs.
>>
>> One such issue was seen on OMAP4430, where L4_PER was periodically
>> getting stuck in in-transition state when transitioning from from OSWR to ON.
>>
>> This issue was reported and investigated by Patrick Titiano<p-titiano@ti.com>
>>
>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> Reported-by: Patrick Titiano<p-titiano@ti.com>
>> Cc: Kevin Hilman<khilman@ti.com>
>> Cc: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> ---
>>   arch/arm/mach-omap2/clockdomain.c |    7 ++++++-
>>   1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
>> index b98a972..583cc3d 100644
>> --- a/arch/arm/mach-omap2/clockdomain.c
>> +++ b/arch/arm/mach-omap2/clockdomain.c
>> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>>    */
>>   int clkdm_wakeup(struct clockdomain *clkdm)
>>   {
>> +	int ret;
>> +
>>   	if (!clkdm)
>>   		return -EINVAL;
>>
>> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>>
>>   	pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>>
>> -	return arch_clkdm->clkdm_wakeup(clkdm);
>> +	ret = arch_clkdm->clkdm_wakeup(clkdm);
>> +	ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
>
> Seems like this should just call pwrdm_state_switch() or
> pwrdm_clkdm_state_switch()?  (This second function looks superfluous, we
> should probably get rid of it.)
>
This comment was expected since initially we thought of using
pwrdm_clkdm_state_switch

pwrdm_clkdm_state_switch()
	|--> pwrdm_wait_transition()
	|--> pwrdm_state_switch()

What we need is only first function to ensure that we don't
proceed when PD is in middle of transition.

The second function is actually just doing those debug counter
updates and we wanted to avoid that since it's a live path.
Rajendra observed some huge latencies while doing the
profiling and the suspect was "pwrdm_state_switch()" which
actually keeps adding overhead because of those counter
updates.

> Shouldn't this be added to all of
> clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
>
clkdm_allow_idle() already has the power-domain wait.
This patch adds it for clkdm_wakeup() function.
clkdm_sleep(), this shouldn't be applicable since power
domain sleep transition is depdent on other clockdomains
in the PD and it's a lazy operation.
That leaves clkdm_deny_idle() and I guess we should
address this API as well.

If you agree with above, I can update the patch to address
remaining clkdm_deny_idle() function.

Regards
Santosh
Regards
Santosh

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh]

Paul,

On Saturday 16 July 2011 01:18 PM, Santosh Shilimkar wrote:
> Hi Paul,
>
> On 7/15/2011 1:03 AM, Paul Walmsley wrote:
>> cc'ing Patrick
>>
>> Hi Rajendra, Santosh,
>>
>> some comments here:
>>
>> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>>
>>> While using clockdomain force wakeup method, not waiting for powerdomain
>>> to be effectively ON may end up locking the clockdomain FSM until a
>>> next wakeup event occurs.
>>>
>>> One such issue was seen on OMAP4430, where L4_PER was periodically
>>> getting stuck in in-transition state when transitioning from from
>>> OSWR to ON.
>>>
>>> This issue was reported and investigated by Patrick
>>> Titiano<p-titiano@ti.com>
>>>
>>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>>> Reported-by: Patrick Titiano<p-titiano@ti.com>
>>> Cc: Kevin Hilman<khilman@ti.com>
>>> Cc: Benoit Cousson<b-cousson@ti.com>
>>> Cc: Paul Walmsley<paul@pwsan.com>
>>> ---
>>> arch/arm/mach-omap2/clockdomain.c | 7 ++++++-
>>> 1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/clockdomain.c
>>> b/arch/arm/mach-omap2/clockdomain.c
>>> index b98a972..583cc3d 100644
>>> --- a/arch/arm/mach-omap2/clockdomain.c
>>> +++ b/arch/arm/mach-omap2/clockdomain.c
>>> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>>> */
>>> int clkdm_wakeup(struct clockdomain *clkdm)
>>> {
>>> + int ret;
>>> +
>>> if (!clkdm)
>>> return -EINVAL;
>>>
>>> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>>>
>>> pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>>>
>>> - return arch_clkdm->clkdm_wakeup(clkdm);
>>> + ret = arch_clkdm->clkdm_wakeup(clkdm);
>>> + ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
>>
>> Seems like this should just call pwrdm_state_switch() or
>> pwrdm_clkdm_state_switch()? (This second function looks superfluous, we
>> should probably get rid of it.)
>>
> This comment was expected since initially we thought of using
> pwrdm_clkdm_state_switch
>
> pwrdm_clkdm_state_switch()
> |--> pwrdm_wait_transition()
> |--> pwrdm_state_switch()
>
> What we need is only first function to ensure that we don't
> proceed when PD is in middle of transition.
>
> The second function is actually just doing those debug counter
> updates and we wanted to avoid that since it's a live path.
> Rajendra observed some huge latencies while doing the
> profiling and the suspect was "pwrdm_state_switch()" which
> actually keeps adding overhead because of those counter
> updates.
>
>> Shouldn't this be added to all of
>> clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
>>
> clkdm_allow_idle() already has the power-domain wait.
> This patch adds it for clkdm_wakeup() function.
> clkdm_sleep(), this shouldn't be applicable since power
> domain sleep transition is depdent on other clockdomains
> in the PD and it's a lazy operation.
> That leaves clkdm_deny_idle() and I guess we should
> address this API as well.
>
> If you agree with above, I can update the patch to address
> remaining clkdm_deny_idle() function.
>
Ping?

Regards
Santosh

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh]

Paul,

On Saturday 16 July 2011 01:18 PM, Santosh Shilimkar wrote:
> Hi Paul,
>
> On 7/15/2011 1:03 AM, Paul Walmsley wrote:
>> cc'ing Patrick
>>
>> Hi Rajendra, Santosh,
>>
>> some comments here:
>>
>> On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
>>
>>> While using clockdomain force wakeup method, not waiting for powerdomain
>>> to be effectively ON may end up locking the clockdomain FSM until a
>>> next wakeup event occurs.
>>>
>>> One such issue was seen on OMAP4430, where L4_PER was periodically
>>> getting stuck in in-transition state when transitioning from from
>>> OSWR to ON.
>>>
>>> This issue was reported and investigated by Patrick
>>> Titiano<p-titiano@ti.com>
>>>
>>> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
>>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>>> Reported-by: Patrick Titiano<p-titiano@ti.com>
>>> Cc: Kevin Hilman<khilman@ti.com>
>>> Cc: Benoit Cousson<b-cousson@ti.com>
>>> Cc: Paul Walmsley<paul@pwsan.com>
>>> ---
>>> arch/arm/mach-omap2/clockdomain.c | 7 ++++++-
>>> 1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/clockdomain.c
>>> b/arch/arm/mach-omap2/clockdomain.c
>>> index b98a972..583cc3d 100644
>>> --- a/arch/arm/mach-omap2/clockdomain.c
>>> +++ b/arch/arm/mach-omap2/clockdomain.c
>>> @@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)
>>> */
>>> int clkdm_wakeup(struct clockdomain *clkdm)
>>> {
>>> + int ret;
>>> +
>>> if (!clkdm)
>>> return -EINVAL;
>>>
>>> @@ -732,7 +734,10 @@ int clkdm_wakeup(struct clockdomain *clkdm)
>>>
>>> pr_debug("clockdomain: forcing wakeup on %s\n", clkdm->name);
>>>
>>> - return arch_clkdm->clkdm_wakeup(clkdm);
>>> + ret = arch_clkdm->clkdm_wakeup(clkdm);
>>> + ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
>>
>> Seems like this should just call pwrdm_state_switch() or
>> pwrdm_clkdm_state_switch()? (This second function looks superfluous, we
>> should probably get rid of it.)
>>
> This comment was expected since initially we thought of using
> pwrdm_clkdm_state_switch
>
> pwrdm_clkdm_state_switch()
> |--> pwrdm_wait_transition()
> |--> pwrdm_state_switch()
>
> What we need is only first function to ensure that we don't
> proceed when PD is in middle of transition.
>
> The second function is actually just doing those debug counter
> updates and we wanted to avoid that since it's a live path.
> Rajendra observed some huge latencies while doing the
> profiling and the suspect was "pwrdm_state_switch()" which
> actually keeps adding overhead because of those counter
> updates.
>
>> Shouldn't this be added to all of
>> clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
>>
> clkdm_allow_idle() already has the power-domain wait.
> This patch adds it for clkdm_wakeup() function.
> clkdm_sleep(), this shouldn't be applicable since power
> domain sleep transition is depdent on other clockdomains
> in the PD and it's a lazy operation.
> That leaves clkdm_deny_idle() and I guess we should
> address this API as well.
>
> If you agree with above, I can update the patch to address
> remaining clkdm_deny_idle() function.
>
Ping?

Regards
Santosh

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

(cc Tero)

Hi Santosh,

On Thu, 11 Aug 2011, Santosh wrote:

> On Saturday 16 July 2011 01:18 PM, Santosh Shilimkar wrote:
> > On 7/15/2011 1:03 AM, Paul Walmsley wrote:
> > > On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
> > > 
> > > > - return arch_clkdm->clkdm_wakeup(clkdm);
> > > > + ret = arch_clkdm->clkdm_wakeup(clkdm);
> > > > + ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
> > > 
> > > Seems like this should just call pwrdm_state_switch() or
> > > pwrdm_clkdm_state_switch()? (This second function looks superfluous, we
> > > should probably get rid of it.)
> > > 
> > This comment was expected since initially we thought of using
> > pwrdm_clkdm_state_switch
> > 
> > pwrdm_clkdm_state_switch()
> > |--> pwrdm_wait_transition()
> > |--> pwrdm_state_switch()
> > 
> > What we need is only first function to ensure that we don't
> > proceed when PD is in middle of transition.
> > 
> > The second function is actually just doing those debug counter
> > updates and we wanted to avoid that since it's a live path.

Those counters are now also used by pwrdm_get_context_loss_count() which 
ultimately is exposed to device drivers as
omap_pm_get_dev_context_loss_count().

If clkdm_wakeup() causes the powerdomain to wake up from a state that 
caused context to be lost, don't we need to update the counters due to 
that dependency?

> > Rajendra observed some huge latencies while doing the
> > profiling and the suspect was "pwrdm_state_switch()" which
> > actually keeps adding overhead because of those counter
> > updates.

Probably we'd better deal with the latency issue in a separate patch.

If I recall correctly, the high latency operation in the counters is the 
PREPWSTST register read.  That could probably be optimized out under many 
circumstances.  The counter functionality could be made dependent on 
CONFIG_PM_DEBUG or something similar.

> > > Shouldn't this be added to all of
> > > clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
> > > 
> > clkdm_allow_idle() already has the power-domain wait.
> > This patch adds it for clkdm_wakeup() function.
> > clkdm_sleep(), this shouldn't be applicable since power
> > domain sleep transition is depdent on other clockdomains
> > in the PD and it's a lazy operation.

That's a good question.  Could you ping the PRCM designers on that issue 
to see if there's any risk that not waiting for a clockdomain force-sleep 
might cause some FSM issue?  We should probably handle this in a separate 
patch anyway so it doesn't retard this patch.

> > That leaves clkdm_deny_idle() and I guess we should
> > address this API as well.
> > 
> > If you agree with above, I can update the patch to address

I'd propose:

1. changing the patch to use pwrdm_state_switch()

2. adding that to clkdm_deny_idle() also

What do you think?


- Paul

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

(cc Tero)

Hi Santosh,

On Thu, 11 Aug 2011, Santosh wrote:

> On Saturday 16 July 2011 01:18 PM, Santosh Shilimkar wrote:
> > On 7/15/2011 1:03 AM, Paul Walmsley wrote:
> > > On Wed, 13 Jul 2011, Santosh Shilimkar wrote:
> > > 
> > > > - return arch_clkdm->clkdm_wakeup(clkdm);
> > > > + ret = arch_clkdm->clkdm_wakeup(clkdm);
> > > > + ret |= pwrdm_wait_transition(clkdm->pwrdm.ptr);
> > > 
> > > Seems like this should just call pwrdm_state_switch() or
> > > pwrdm_clkdm_state_switch()? (This second function looks superfluous, we
> > > should probably get rid of it.)
> > > 
> > This comment was expected since initially we thought of using
> > pwrdm_clkdm_state_switch
> > 
> > pwrdm_clkdm_state_switch()
> > |--> pwrdm_wait_transition()
> > |--> pwrdm_state_switch()
> > 
> > What we need is only first function to ensure that we don't
> > proceed when PD is in middle of transition.
> > 
> > The second function is actually just doing those debug counter
> > updates and we wanted to avoid that since it's a live path.

Those counters are now also used by pwrdm_get_context_loss_count() which 
ultimately is exposed to device drivers as
omap_pm_get_dev_context_loss_count().

If clkdm_wakeup() causes the powerdomain to wake up from a state that 
caused context to be lost, don't we need to update the counters due to 
that dependency?

> > Rajendra observed some huge latencies while doing the
> > profiling and the suspect was "pwrdm_state_switch()" which
> > actually keeps adding overhead because of those counter
> > updates.

Probably we'd better deal with the latency issue in a separate patch.

If I recall correctly, the high latency operation in the counters is the 
PREPWSTST register read.  That could probably be optimized out under many 
circumstances.  The counter functionality could be made dependent on 
CONFIG_PM_DEBUG or something similar.

> > > Shouldn't this be added to all of
> > > clkdm_{wakeup,sleep,allow_idle,deny_idle}() if it isn't there already?
> > > 
> > clkdm_allow_idle() already has the power-domain wait.
> > This patch adds it for clkdm_wakeup() function.
> > clkdm_sleep(), this shouldn't be applicable since power
> > domain sleep transition is depdent on other clockdomains
> > in the PD and it's a lazy operation.

That's a good question.  Could you ping the PRCM designers on that issue 
to see if there's any risk that not waiting for a clockdomain force-sleep 
might cause some FSM issue?  We should probably handle this in a separate 
patch anyway so it doesn't retard this patch.

> > That leaves clkdm_deny_idle() and I guess we should
> > address this API as well.
> > 
> > If you agree with above, I can update the patch to address

I'd propose:

1. changing the patch to use pwrdm_state_switch()

2. adding that to clkdm_deny_idle() also

What do you think?


- Paul

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

Hi everyone

I've updated this patch per my earlier comments and also to apply.  
Please let me know if you have any issues with it ASAP.


- Paul

From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Sat, 13 Aug 2011 08:56:28 +0530
Subject: [PATCH] [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON
 when using clockdomain force wakeup

While using clockdomain force wakeup method, not waiting for powerdomain
to be effectively ON may end up locking the clockdomain FSM until a
next wakeup event occurs.

One such issue was seen on OMAP4430, where L4_PER was periodically
getting stuck in in-transition state when transitioning from from OSWR to ON.

This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Patrick Titiano <p-titiano@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
[paul@pwsan.com: updated to apply; added transition wait on clkdm_deny_idle();
 remove two superfluous pwrdm_wait_transition() calls]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clockdomain.c |    2 ++
 arch/arm/mach-omap2/pm.c          |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index ab7db08..8f08906 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -747,6 +747,7 @@ int clkdm_wakeup(struct clockdomain *clkdm)
 	spin_lock_irqsave(&clkdm->lock, flags);
 	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
 	ret = arch_clkdm->clkdm_wakeup(clkdm);
+	ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
 	spin_unlock_irqrestore(&clkdm->lock, flags);
 	return ret;
 }
@@ -818,6 +819,7 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
 	spin_lock_irqsave(&clkdm->lock, flags);
 	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
 	arch_clkdm->clkdm_deny_idle(clkdm);
+	pwrdm_state_switch(clkdm->pwrdm.ptr);
 	spin_unlock_irqrestore(&clkdm->lock, flags);
 }
 
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 3feb359..472bf22 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -130,7 +130,6 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		} else {
 			hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
 			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
-			pwrdm_wait_transition(pwrdm);
 			sleep_switch = FORCEWAKEUP_SWITCH;
 		}
 	}
@@ -156,7 +155,6 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		return ret;
 	}
 
-	pwrdm_wait_transition(pwrdm);
 	pwrdm_state_switch(pwrdm);
 err:
 	return ret;
-- 
1.7.5.4

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Paul Walmsley]

Hi everyone

I've updated this patch per my earlier comments and also to apply.  
Please let me know if you have any issues with it ASAP.


- Paul

From: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date: Sat, 13 Aug 2011 08:56:28 +0530
Subject: [PATCH] [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON
 when using clockdomain force wakeup

While using clockdomain force wakeup method, not waiting for powerdomain
to be effectively ON may end up locking the clockdomain FSM until a
next wakeup event occurs.

One such issue was seen on OMAP4430, where L4_PER was periodically
getting stuck in in-transition state when transitioning from from OSWR to ON.

This issue was reported and investigated by Patrick Titiano <p-titiano@ti.com>

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Patrick Titiano <p-titiano@ti.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
[paul at pwsan.com: updated to apply; added transition wait on clkdm_deny_idle();
 remove two superfluous pwrdm_wait_transition() calls]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clockdomain.c |    2 ++
 arch/arm/mach-omap2/pm.c          |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomain.c b/arch/arm/mach-omap2/clockdomain.c
index ab7db08..8f08906 100644
--- a/arch/arm/mach-omap2/clockdomain.c
+++ b/arch/arm/mach-omap2/clockdomain.c
@@ -747,6 +747,7 @@ int clkdm_wakeup(struct clockdomain *clkdm)
 	spin_lock_irqsave(&clkdm->lock, flags);
 	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
 	ret = arch_clkdm->clkdm_wakeup(clkdm);
+	ret |= pwrdm_state_switch(clkdm->pwrdm.ptr);
 	spin_unlock_irqrestore(&clkdm->lock, flags);
 	return ret;
 }
@@ -818,6 +819,7 @@ void clkdm_deny_idle(struct clockdomain *clkdm)
 	spin_lock_irqsave(&clkdm->lock, flags);
 	clkdm->_flags &= ~_CLKDM_FLAG_HWSUP_ENABLED;
 	arch_clkdm->clkdm_deny_idle(clkdm);
+	pwrdm_state_switch(clkdm->pwrdm.ptr);
 	spin_unlock_irqrestore(&clkdm->lock, flags);
 }
 
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 3feb359..472bf22 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -130,7 +130,6 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		} else {
 			hwsup = clkdm_in_hwsup(pwrdm->pwrdm_clkdms[0]);
 			clkdm_wakeup(pwrdm->pwrdm_clkdms[0]);
-			pwrdm_wait_transition(pwrdm);
 			sleep_switch = FORCEWAKEUP_SWITCH;
 		}
 	}
@@ -156,7 +155,6 @@ int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 		return ret;
 	}
 
-	pwrdm_wait_transition(pwrdm);
 	pwrdm_state_switch(pwrdm);
 err:
 	return ret;
-- 
1.7.5.4

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi everyone

I've tweaked this patch a little bit.  Please let me know if you have any 
issues with it ASAP,

thanks,

- Paul

From: Rajendra Nayak <rnayak@ti.com>
Date: Tue, 16 Aug 2011 15:22:58 -0600
Subject: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON
 at init

Program all powerdomain target state as ON; this is to prevent domains
from hitting low power states (if bootloader has target states set to
something other than ON) and potentially even losing context while PM
is not fully initialized, which can cause the system to crash.  The PM
late init code can then program the desired target state for all the
power domains.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
[paul@pwsan.com: dropped comment typo hunk; fixed comment indent and moved
 to kerneldoc; moved code to pwrdm_init(); changed pwrdm_init() argument name
 to prevent clash; cleaned up patch description]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/powerdomain.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 9af0847..ef71fdd 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -195,28 +195,35 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 
 /**
  * pwrdm_init - set up the powerdomain layer
- * @pwrdm_list: array of struct powerdomain pointers to register
+ * @pwrdms: array of struct powerdomain pointers to register
  * @custom_funcs: func pointers for arch specific implementations
  *
- * Loop through the array of powerdomains @pwrdm_list, registering all
- * that are available on the current CPU. If pwrdm_list is supplied
- * and not null, all of the referenced powerdomains will be
- * registered.  No return value.  XXX pwrdm_list is not really a
- * "list"; it is an array.  Rename appropriately.
+ * Loop through the array of powerdomains @pwrdms, registering all
+ * that are available on the current CPU.  Also, program all
+ * powerdomain target state as ON; this is to prevent domains from
+ * hitting low power states (if bootloader has target states set to
+ * something other than ON) and potentially even losing context while
+ * PM is not fully initialized.  The PM late init code can then program
+ * the desired target state for all the power domains.  No return
+ * value.
  */
-void pwrdm_init(struct powerdomain **pwrdm_list, struct pwrdm_ops *custom_funcs)
+void pwrdm_init(struct powerdomain **pwrdms, struct pwrdm_ops *custom_funcs)
 {
 	struct powerdomain **p = NULL;
+	struct powerdomain *temp_p;
 
 	if (!custom_funcs)
 		WARN(1, "powerdomain: No custom pwrdm functions registered\n");
 	else
 		arch_pwrdm = custom_funcs;
 
-	if (pwrdm_list) {
-		for (p = pwrdm_list; *p; p++)
+	if (pwrdms) {
+		for (p = pwrdms; *p; p++)
 			_pwrdm_register(*p);
 	}
+
+	list_for_each_entry(temp_p, &pwrdm_list, node)
+		pwrdm_set_next_pwrst(temp_p, PWRDM_POWER_ON);
 }
 
 /**
-- 
1.7.5.4

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi everyone

I've tweaked this patch a little bit.  Please let me know if you have any 
issues with it ASAP,

thanks,

- Paul

From: Rajendra Nayak <rnayak@ti.com>
Date: Tue, 16 Aug 2011 15:22:58 -0600
Subject: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON
 at init

Program all powerdomain target state as ON; this is to prevent domains
from hitting low power states (if bootloader has target states set to
something other than ON) and potentially even losing context while PM
is not fully initialized, which can cause the system to crash.  The PM
late init code can then program the desired target state for all the
power domains.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
[paul at pwsan.com: dropped comment typo hunk; fixed comment indent and moved
 to kerneldoc; moved code to pwrdm_init(); changed pwrdm_init() argument name
 to prevent clash; cleaned up patch description]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/powerdomain.c |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 9af0847..ef71fdd 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -195,28 +195,35 @@ static int _pwrdm_post_transition_cb(struct powerdomain *pwrdm, void *unused)
 
 /**
  * pwrdm_init - set up the powerdomain layer
- * @pwrdm_list: array of struct powerdomain pointers to register
+ * @pwrdms: array of struct powerdomain pointers to register
  * @custom_funcs: func pointers for arch specific implementations
  *
- * Loop through the array of powerdomains @pwrdm_list, registering all
- * that are available on the current CPU. If pwrdm_list is supplied
- * and not null, all of the referenced powerdomains will be
- * registered.  No return value.  XXX pwrdm_list is not really a
- * "list"; it is an array.  Rename appropriately.
+ * Loop through the array of powerdomains @pwrdms, registering all
+ * that are available on the current CPU.  Also, program all
+ * powerdomain target state as ON; this is to prevent domains from
+ * hitting low power states (if bootloader has target states set to
+ * something other than ON) and potentially even losing context while
+ * PM is not fully initialized.  The PM late init code can then program
+ * the desired target state for all the power domains.  No return
+ * value.
  */
-void pwrdm_init(struct powerdomain **pwrdm_list, struct pwrdm_ops *custom_funcs)
+void pwrdm_init(struct powerdomain **pwrdms, struct pwrdm_ops *custom_funcs)
 {
 	struct powerdomain **p = NULL;
+	struct powerdomain *temp_p;
 
 	if (!custom_funcs)
 		WARN(1, "powerdomain: No custom pwrdm functions registered\n");
 	else
 		arch_pwrdm = custom_funcs;
 
-	if (pwrdm_list) {
-		for (p = pwrdm_list; *p; p++)
+	if (pwrdms) {
+		for (p = pwrdms; *p; p++)
 			_pwrdm_register(*p);
 	}
+
+	list_for_each_entry(temp_p, &pwrdm_list, node)
+		pwrdm_set_next_pwrst(temp_p, PWRDM_POWER_ON);
 }
 
 /**
-- 
1.7.5.4

Re: [PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> How about something like:
> 
> /*
>  * Program target state of all power domains to ON. This is to prevent
>  * power domains from hitting low power states during boot up and
>  * potentially causing accesses to the address space of an IP while it
>  * is disabled.
>  *
>  * PM late init code will make sure of disabling all unused IPs later.
>  */
> 
> not sure if it's a lot better though.

It's not directly related to accesses to the IP block's address space.  
The problem happens when the driver (or the hwmod code) for a particular 
device causes the last clock in a clockdomain to be disabled.  This allows 
the clockdomain to go INACTIVE.

At that point, if all of the clockdomains in the powerdomain are INACTIVE, 
the powerdomain itself will switch to its "next power state". If the 
bootloader had programmed the next power state to be OFF, for example, 
then all devices in that powerdomain will lose context. 

If this happens before the PM code initializes, or if the PM code is not 
compiled in, then this will cause some strange behavior or crashes, since 
the kernel won't be expecting devices to lose their context at that point.


- Paul

[PATCH] OMAP: powerdomains: Make all powerdomain target states as ON at init

[Paul Walmsley]

Hi,

On Fri, 15 Jul 2011, Felipe Balbi wrote:

> How about something like:
> 
> /*
>  * Program target state of all power domains to ON. This is to prevent
>  * power domains from hitting low power states during boot up and
>  * potentially causing accesses to the address space of an IP while it
>  * is disabled.
>  *
>  * PM late init code will make sure of disabling all unused IPs later.
>  */
> 
> not sure if it's a lot better though.

It's not directly related to accesses to the IP block's address space.  
The problem happens when the driver (or the hwmod code) for a particular 
device causes the last clock in a clockdomain to be disabled.  This allows 
the clockdomain to go INACTIVE.

At that point, if all of the clockdomains in the powerdomain are INACTIVE, 
the powerdomain itself will switch to its "next power state". If the 
bootloader had programmed the next power state to be OFF, for example, 
then all devices in that powerdomain will lose context. 

If this happens before the PM code initializes, or if the PM code is not 
compiled in, then this will cause some strange behavior or crashes, since 
the kernel won't be expecting devices to lose their context at that point.


- Paul

Re: [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh]

On Wednesday 17 August 2011 07:17 AM, Paul Walmsley wrote:
>
> Hi everyone
>
> I've updated this patch per my earlier comments and also to apply.
> Please let me know if you have any issues with it ASAP.
>
>
> - Paul
>
> From: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Date: Sat, 13 Aug 2011 08:56:28 +0530
> Subject: [PATCH] [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON
>   when using clockdomain force wakeup
>
> While using clockdomain force wakeup method, not waiting for powerdomain
> to be effectively ON may end up locking the clockdomain FSM until a
> next wakeup event occurs.
>
> One such issue was seen on OMAP4430, where L4_PER was periodically
> getting stuck in in-transition state when transitioning from from OSWR to ON.
>
> This issue was reported and investigated by Patrick Titiano<p-titiano@ti.com>
>
> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
> Reported-by: Patrick Titiano<p-titiano@ti.com>
> Cc: Kevin Hilman<khilman@ti.com>
> Cc: Benoit Cousson<b-cousson@ti.com>
> Cc: Paul Walmsley<paul@pwsan.com>
> [paul@pwsan.com: updated to apply; added transition wait on clkdm_deny_idle();
>   remove two superfluous pwrdm_wait_transition() calls]
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> ---
Thanks Paul. Patch looks good to me.

Regards
Santosh

[PATCH] OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup

[Santosh]

On Wednesday 17 August 2011 07:17 AM, Paul Walmsley wrote:
>
> Hi everyone
>
> I've updated this patch per my earlier comments and also to apply.
> Please let me know if you have any issues with it ASAP.
>
>
> - Paul
>
> From: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Date: Sat, 13 Aug 2011 08:56:28 +0530
> Subject: [PATCH] [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON
>   when using clockdomain force wakeup
>
> While using clockdomain force wakeup method, not waiting for powerdomain
> to be effectively ON may end up locking the clockdomain FSM until a
> next wakeup event occurs.
>
> One such issue was seen on OMAP4430, where L4_PER was periodically
> getting stuck in in-transition state when transitioning from from OSWR to ON.
>
> This issue was reported and investigated by Patrick Titiano<p-titiano@ti.com>
>
> Signed-off-by: Santosh Shilimkar<santosh.shilimkar@ti.com>
> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
> Reported-by: Patrick Titiano<p-titiano@ti.com>
> Cc: Kevin Hilman<khilman@ti.com>
> Cc: Benoit Cousson<b-cousson@ti.com>
> Cc: Paul Walmsley<paul@pwsan.com>
> [paul at pwsan.com: updated to apply; added transition wait on clkdm_deny_idle();
>   remove two superfluous pwrdm_wait_transition() calls]
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> ---
Thanks Paul. Patch looks good to me.

Regards
Santosh