Kevin Hilman had written, on 11/19/2010 11:08 AM, the following:
Nishanth Menon <nm@xxxxxx> writes:
From: Eduardo Valentin <eduardo.valentin@xxxxxxxxx>
Deny MPU idle before save secure ram and allow it after save secure RAM.
We want to deny MPU going to low power state because, there is a short
time window where a wakeup event would happen around the time the MPU
is going to idle. Since the first thing ROM code does after WFI is to
read INTCPS register, we could reach a situation where the INTCPS is
in idle, but the read is sent to it. That would produce a data abord
during the save of secure ram, which will hang the system. we need
to prevent clock transitions as well during this timeframe.
This changelog needs to be a bit clearer about exacly why MPU would be
going to a low power state during a secure-mode call. IIUC, it's
because the ROM code might do a WFI. Since it's completely
non-intuitive (and broken, at least to me) that the ROM code would do a
WFI, this should be stated explicitly in the changelog, and probably in
the code too.
yep, ROM code does use WFI in this path - I unfortunately cannot go into
the details on why it does it :( - will modify the commit to be explicit
and state that it does use WFI in this path.
Also, this approach only solves this problem for this particular
secure-mode call. Presumably there are other secure-mode calls that
might WFI as well, and will have the same problem. As I'm not familiar
with secure ROM or PPAs, so I don't know if this is true or not. If it
is, then somethen more general should be done.
After a long review, the impacted section is this logic alone.
Also, do we care about other powerdomains (besides MPU) going idle
during a secure mode call?
not for this case.
Because of those issues, some other proposals have been floated for this
problem. In particular, explicitly setting some of the powerdomain next
states (at least MPU & CORE) to ON when we're not in the normal idle
path so that would also prevent this problem.
We need to do save secureram for hitting OFF - depending on the PPA - we
may need to do save secureram in every iteration, the criteria needed
here to workaround the "limitation" we have in ROM Code is to deny
idling the clocks as well for this path as done here.
I guess we need some more details on which secure mode calls can trigger
this problem. If this is an isolated case, I'm OK with this fix. If
it's more general, I'd like to see a more general fix.
As mentioned above, This is an isolated case based on our analysis.
Kevin
[nm@xxxxxx: rebased to 2.6.37-rc2, used omap2_clkdm_deny_idle for
clock prevention]
Signed-off-by: Nishanth Menon <nm@xxxxxx>
Signed-off-by: Eduardo Valentin <eduardo.valentin@xxxxxxxxx>
---
arch/arm/mach-omap2/pm34xx.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 7877f74..f520b38 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -194,15 +194,19 @@ int __init omap3_secure_copy_data_set(struct omap3_secure_copy_data *data)
static void omap3_save_secure_ram_context(u32 target_mpu_state)
{
if (!secure_ram_saved && omap_type() != OMAP2_DEVICE_TYPE_GP) {
+ struct clockdomain *clkd = mpu_pwrdm->pwrdm_clkdms[0];
+
/*
* MPU next state must be set to POWER_ON temporarily,
* otherwise the WFI executed inside the ROM code
* will hang the system.
*/
pwrdm_set_next_pwrst(mpu_pwrdm, PWRDM_POWER_ON);
+ omap2_clkdm_deny_idle(clkd);
secure_ram_save_status = _omap_save_secure_sram((u32 *)
(omap3_secure_ram_storage));
pwrdm_set_next_pwrst(mpu_pwrdm, target_mpu_state);
+ omap2_clkdm_allow_idle(clkd);
if (!secure_copy_data.save_every_cycle)
secure_ram_saved = 1;
}
--
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
