Vishwanath Sripathy had written, on 12/13/2010 08:48 AM, the following:
-----Original Message-----
From: Nishanth Menon [mailto:nm@xxxxxx]
Sent: Monday, December 13, 2010 8:14 PM
To: Vishwanath Sripathy
Cc: linux-omap; Eduardo Valentin; Kevin Hilman; Tony Lindgren
Subject: RE: [PATCH 5/5 v3] OMAP3630: PM: Erratum i583: disable
coreoff if < ES1.2
-----Original Message-----
Vishwanath Sripathy had written, on 12/13/2010 08:25 AM, the
following:
[...]
+ if
(IS_PM34XX_ERRATUM(SDRC_WAKEUP_ERRATUM_i583)
&&
+ (core_next_state ==
PWRDM_POWER_OFF))
{
+ pwrdm_set_next_pwrst(core_pwrdm,
PWRDM_POWER_RET);
+ core_next_state = PWRDM_POWER_RET;
+ }
Since next_state in pwrst_list (for core) is not updated, this
is
throwing
up an error "Powerdomain (core_pwrdm) didn't enter target
state
0"
when
you off mode is enabled for ES1.1 or lesser (in suspend path).
It's
not
really true that Core has not entered target state. It has
entered
Retention state which is the actual target state set in
omap_sram_idle.
However it did not enter the state that was passed by
omap3_pm_suspend. Is
this expected behavior?
we could go both ways on this - this patch will(as you noticed)
indicate
that the transition failed on <ES1.2, or we could make it
entirely
transparent(by modifying the the pwrst_list - claim we achieved
off,
while not really hitting off - I personally dont think that is
correct.
The point I am making is that you cannot distinguish between
genuine
off
/retention failure since this message is thrown for both pass and
fail.
May be an additional trace message indicating that system
entered
retention instead of off (for ES<1.2) will be useful.
hmm... good point there.
two issues here:
a) omap3_pm_suspend should probably state which state was
achieved
as
well in the error message (trivial fix).
b) how do we notify users that it was due to
SDRC_WAKEUP_ERRATUM_i583
that core-off was denied. -> do this in omap3_pm_suspend(when
user
attempts actual OFF) OR omap3_pm_off_mode_enable(when user
attempts to
enable OFF mode)?
Any suggestions to allow the same uImage boot on all silicon +
allow
cpu_idle + suspend paths not to spew pr_info messages(aka cant
add
prints in sram_idle)?
I vote for denying off mode for Core (for ES<1.2) in
omap3_pm_off_mode_enable and throw up a message saying that
Core off
is
not enabled. Then we will not get this failure message in suspend
path
since pwrst_list will have the right state.
Keep in mind - if we disable it in omap3_pm_off_mode_enable - we
will
deny OFF wholesale if I understand the logic right- not just core-off
-
I kind of think that is extreme.
I take that back, we could do something like the following instead:
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-
omap2/pm34xx.c
index ba3c0d6..74842f1 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -933,7 +933,14 @@ void omap3_pm_off_mode_enable(int enable)
list_for_each_entry(pwrst, &pwrst_list, node) {
pwrst->next_state = state;
- omap_set_pwrdm_state(pwrst->pwrdm, state);
+ if (IS_PM34XX_ERRATUM(SDRC_WAKEUP_ERRATUM_i583)
&&
+ pwrst->pwrdm == core_pwrdm) {
+ omap_set_pwrdm_state(pwrst->pwrdm,
PWRDM_POWER_RET);
You need to update pwrst->next_state as well.
Otherwise suspend will still throw the same error. I just sent the code in
other email.
yep. except, I think we dont need to do string compare. the following
looks any better?
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index ba3c0d6..da12a56 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -932,8 +932,15 @@ void omap3_pm_off_mode_enable(int enable)
#endif
list_for_each_entry(pwrst, &pwrst_list, node) {
- pwrst->next_state = state;
- omap_set_pwrdm_state(pwrst->pwrdm, state);
+ if (IS_PM34XX_ERRATUM(SDRC_WAKEUP_ERRATUM_i583) &&
+ pwrst->pwrdm == core_pwrdm) {
+ pwrst->next_state = PWRDM_POWER_RET;
+ pr_err("%s: cannot enable Core OFF due to i583\n",
+ __func__);
+ } else {
+ pwrst->next_state = state;
+ }
+ omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
}
}
--
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
