Re: [PATCH 08/13] OMAP3: PM: Deny MPU idle while saving secure RAM

[Nishanth Menon <nm@ti.com>]

Kevin Hilman had written, on 11/19/2010 02:39 PM, the following:
[...]
> In addtion, the patch from Santosh needs to better describe what other
> problems it is solving, since it is clearly not fixing this particular
> secure mode entry.  Therefore, there must be others that are also doing
> WFI.   That being said, instead of such a generic fix as is done by
> Santosh's patch, maybe we need a common secure-mode entry point which
> does the necessary ROM code prep.
Ideally speaking - save_secure_ram can hit latencies which are pretty 
bad.. eventually this logic should be moved outside the current 
boundaries in some manner - unfortunately, I cant at the moment think of 
a sane mechanism to do that given various proprietary and 
not-mainlined-but-public security drivers for OMAP3 out there :(. IMHO, 
the responsibility of secure storage should be with secure drivers, but, 
at the moment touching that topic is opening up a pandora's box :(

> 
>> This specific patch controls the clock domain from auto idling around
>> the secure ram save. Apologies on the confusion - but if the [1] patch
>> is fixing it, you can help me understand how it does it.
> 
> Now that I understand the clockdomain part, I'm seeing the problem
> differently.  (side note: A better written changelog could have avoided
> this confusion by being clear that it was *clockdomain* idle that was
> being added here and that it was in addition to the existing powerdomain
> settings.)
> 
> Technically, $SUBJECT patch could have replaced the set_next_pwrst with
> the clkdm_deny_idle.  IOW, setting the pwrdm next state to is redundant
> if you clkdm_deny_idle.
> 
> I think this is the key to the confusion:
> 
> 1) clkdm_deny_idle() implies the powerdomain stays on
> 2) setting powerdomain to on, does NOT imply clkdm_deny_idle()
> 
> Another way of saying it is that setting a powerdomain to on does not
> prevent it from going inactive.  It only prevents retention or off-mode.
Agreed and I apologize for the confusion caused by the commit message - 
will it be sufficient for the purpose of this series to change the 
commit log to better describe the patch? - I will leave the power domain 
control to Santosh's /Tero's series instead.

Is this acceptable option?

[...]

-- 
Regards,
Nishanth Menon