OMAP: powerdomains: Make all powerdomain target states as ON at init
Subsystems (5)
PATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
From: Rajendra Nayak <[email protected]>
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.
--- arch/arm/mach-omap2/powerdomain.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/powerdomain.c +11 -1
@@ -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
Signature · 1 line
1.7.4.1
PATCH 1/1 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 <[email protected]>
--- arch/arm/mach-omap2/clockdomain.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
arch/arm/mach-omap2/clockdomain.c +6 -1
@@ -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; }/**
Signature · 1 line
1.7.4.1
Paul Walmsley
maintainerPATCH 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
cc'ing Patrick
Hi Rajendra, Santosh,
some comments here:
Quoted 33 lines · 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_PERwas periodically getting stuck in in-transition state when transitioning from from OSWR to ON.This issue was reported and investigated by Patrick Titiano <[email protected]>
Signed-off-by Santosh Shilimkar <[email protected]>Signed-off-by Rajendra Nayak <[email protected]>Reported-by Patrick Titiano <[email protected]>Cc Kevin Hilman <[email protected]>Cc Benoit Cousson <[email protected]>Cc Paul Walmsley <[email protected]>--- arch/arm/mach-omap2/clockdomain.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)arch/arm/mach-omap2/clockdomain.c +2
-0@@ -718,6 +718,8 @@ int clkdm_sleep(struct clockdomain *clkdm)*/ int clkdm_wakeup (struct clockdomain *clkdm) { +int ret; +if (!clkdm)return -EINVAL ;
arch/arm/mach-omap2/clockdomain.c int clkdm_wakeup(struct clockdomain *clkdm) · line 738
Quoted 6 lines
arch/arm/mach-omap2/clockdomain.c +2
-1@@ -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?
Quoted 8 lines
arch/arm/mach-omap2/clockdomain.c +2
-0+ +return ret; }/** Signature · 1 line
1.7.4.1
- Paul
PATCH 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
Hi Paul,
Quoted 44 lines · Paul Walmsley
cc'ing Patrick
Hi Rajendra, Santosh,
some comments here:
Quoted 36 lines · 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_PERwas periodically getting stuck in in-transition state when transitioning from from OSWR to ON.This issue was reported and investigated by Patrick Titiano<[email protected]>
Signed-off-by Santosh Shilimkar<[email protected]>Signed-off-by Rajendra Nayak<[email protected]>Reported-by Patrick Titiano<[email protected]>Cc Kevin Hilman<[email protected]>Cc Benoit Cousson<[email protected]>Cc Paul Walmsley<[email protected]>--- arch/arm/mach-omap2/clockdomain.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)arch/arm/mach-omap2/clockdomain.c +2
-0@@ -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);
Quoted 8 lines
arch/arm/mach-omap2/clockdomain.c int clkdm_wakeup(struct clockdomain *clkdm) · line 738
Quoted 3 lines
arch/arm/mach-omap2/clockdomain.c +2
-1-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()orpwrdm_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.
Quoted 3 lines
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 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
Paul,
Quoted 86 lines · Santosh Shilimkar
Hi Paul,
Quoted 47 lines · Paul Walmsley
cc'ing Patrick
Hi Rajendra, Santosh,
some comments here:
Quoted 39 lines · 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_PERwas periodically getting stuck in in-transition state when transitioning from from OSWR to ON.This issue was reported and investigated by Patrick Titiano<[email protected]>
Signed-off-by Santosh Shilimkar<[email protected]>Signed-off-by Rajendra Nayak<[email protected]>Reported-by Patrick Titiano<[email protected]>Cc Kevin Hilman<[email protected]>Cc Benoit Cousson<[email protected]>Cc Paul Walmsley<[email protected]>---
arch/arm/mach-omap2/clockdomain.c| 7 ++++++-1 files changed, 6 insertions(+), 1 deletions(-)arch/arm/mach-omap2/clockdomain.c +2
-0@@ -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);Quoted 8 lines
arch/arm/mach-omap2/clockdomain.c int clkdm_wakeup(struct clockdomain *clkdm) · line 738
Quoted 3 lines
arch/arm/mach-omap2/clockdomain.c +2
-1-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()orpwrdm_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.Quoted 3 lines
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 forclkdm_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 leavesclkdm_deny_idle()and I guess we should address this API as well.
Quoted 3 lines
If you agree with above, I can update the patch to address remaining
clkdm_deny_idle()function.
Ping?
Regards
Santosh
Paul Walmsley
maintainerPATCH 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
(cc Tero)
Hi Santosh,
Quoted 19 lines · Santosh
Quoted 18 lines · Santosh Shilimkar
Quoted 10 lines · Paul Walmsley
Quoted 3 lines · Santosh Shilimkar
- 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()orpwrdm_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()
Quoted 5 lines
Quoted 5 lines
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?
Quoted 4 lines
Quoted 4 lines
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.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
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 forclkdm_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.
Quoted 4 lines
Quoted 4 lines
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
Paul Walmsley
maintainerPATCH 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
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 <[email protected]>
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 <[email protected]>
[paul at pwsan.com: updated to apply; added transition wait on clkdm_deny_idle();
remove two superfluous pwrdm_wait_transition() calls]
--- arch/arm/mach-omap2/clockdomain.c | 2 ++ arch/arm/mach-omap2/pm.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-omap2/clockdomain.c +2 -0
@@ -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); }
arch/arm/mach-omap2/pm.c +0 -2
@@ -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;
Signature · 1 line
1.7.5.4
PATCH 1/1 OMAP: clockdomain: Wait for powerdomain to be ON when using clockdomain force wakeup
Quoted 29 lines · 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<[email protected]>
Date: Sat, 13 Aug 2011 08:56:28 +0530
Subject: [PATCH] [PATCH] OMAP: clockdomain: Wait for powerdomain to be ON
when using clockdomain force wakeupWhile 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_PERwas periodically getting stuck in in-transition state when transitioning from from OSWR to ON.This issue was reported and investigated by Patrick Titiano<[email protected]>
Signed-off-by Santosh Shilimkar<[email protected]>Signed-off-by Rajendra Nayak<[email protected]>Reported-by Patrick Titiano<[email protected]>Cc Kevin Hilman<[email protected]>Cc Benoit Cousson<[email protected]>Cc Paul Walmsley<[email protected]>
Quoted 4 lines
[paul at pwsan.com: updated to apply; added transition wait on
clkdm_deny_idle();
remove two superfluouspwrdm_wait_transition()calls]Signed-off-by Paul Walmsley<[email protected]>---
Thanks Paul. Patch looks good to me.
Regards
Santosh
Paul Walmsley
maintainerPATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
In reply to Santosh ShilimkarQuoted 10 lines · Santosh Shilimkar
From: Rajendra Nayak <[email protected]>
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 <[email protected]>
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 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Quoted 11 lines · Paul Walmsley
Quoted 9 lines · Santosh Shilimkar
From: Rajendra Nayak<[email protected]>
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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Rajendra Nayak<[email protected]>Thanks, dropped the second hunk of the patch and queued for 3.1-rc fixes at
git://git.pwsan.com/linux-2.6in 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
Paul Walmsley
maintainerPATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
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 <[email protected]>
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.
[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]
--- arch/arm/mach-omap2/powerdomain.c | 25 ++++++++++++++++--------- 1 files changed, 16 insertions(+), 9 deletions(-)
arch/arm/mach-omap2/powerdomain.c +16 -9
@@ -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 ); }/**
Signature · 1 line
1.7.5.4
PATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
In reply to Santosh ShilimkarHi,
Quoted 21 lines · Santosh Shilimkar
From: Rajendra Nayak <[email protected]>
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 <[email protected]>--- arch/arm/mach-omap2/powerdomain.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)arch/arm/mach-omap2/powerdomain.c +0
-0@@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)list_add (&pwrdm->node, &pwrdm_list);
arch/arm/mach-omap2/powerdomain.c static int _pwrdm_register(struct powerdomain *pwrdm) · line 120
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +9
-0+/* +* 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 ;-)
Signature · 8 lines
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>
Paul Walmsley
maintainerPATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Hi,
Quoted 24 lines · Felipe Balbi
Quoted 7 lines
arch/arm/mach-omap2/powerdomain.c +0
-0@@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)list_add (&pwrdm->node, &pwrdm_list);arch/arm/mach-omap2/powerdomain.c static int _pwrdm_register(struct powerdomain *pwrdm) · line 120
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +9
-0+/* +* 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 ?
Quoted 2 lines
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 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Hi,
Quoted 30 lines · Paul Walmsley
Quoted 24 lines
Quoted 7 lines
arch/arm/mach-omap2/powerdomain.c +0
-0@@ -109,6 +109,16 @@ static int _pwrdm_register(struct powerdomain *pwrdm)list_add (&pwrdm->node, &pwrdm_list);arch/arm/mach-omap2/powerdomain.c static int _pwrdm_register(struct powerdomain *pwrdm) · line 120
Quoted 10 lines
arch/arm/mach-omap2/powerdomain.c +9
-0+/* +* 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 ?
Quoted 2 lines
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.
Quoted 7 lines
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.
Signature · 8 lines
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>
Paul Walmsley
maintainerPATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Hi Felipe,
Quoted 2 lines · Felipe Balbi
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 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Hi,
Quoted 9 lines · Paul Walmsley
Hi Felipe,
Quoted 2 lines · Felipe Balbi
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.
Signature · 8 lines
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>
Paul Walmsley
maintainerPATCH 1/1 OMAP: powerdomains: Make all powerdomain target states as ON at init
Hi,
Quoted 11 lines · Felipe Balbi
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. */
Quoted 1 line
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