Kevin Hilman had written, on 11/19/2010 03:06 PM, the following:
Nishanth Menon <nm@xxxxxx> writes:
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 :(
Hmm, so the complexity and mess is pushed into the OMAP PM core...
/me no likey
/me neither :(
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?
That is a minimum requirement, but...
Based on the rest of this series, I am not at all comfortable with
managing this directly in the idle path. The latencies you mentioned
above are only part of the reason. I have been trying to remove this
Keep in mind that the latency is incurred by the default settings in
this series *only* for the very first off mode currently.
kind of device idle/PM management from the core idle path and I am not
enthused about adding stuff back.
I would much rather see a separate, secure-mode driver, which for
starters only manages secure RAM. It doesn't have to manage all of
security stuff, but it will make a clearer (and cleaner)
separation between the idle path and secure RAM management. If
implemented as a driver, it could be much more intelligent about
its save/restore and can behave just like any other driver that has to
manage context save/restore. If the concern is about trying to have a
general purpose "secure driver", then just call it a secure RAM driver
or something to be clear it has a small, targetted scope.
There are few other issues with this approach. secure ram save by itself
is just a function. it's trigger should ideally be not just one security
driver IMHO - there is AES, SHAM, and other ones that will need to
implement runtime pm, context save and restore hooks -> E.g. Dimitry's
series[1] is trying to introduce an opensource security driver solution
for OMAP - this is just a start - it will be some time before these
drivers are ready and merged to mainline followed by power management
enablement - do we want to keep omap3 broken while a fix is available
till then?
[1]
http://www.google.com/search?q=Dmitry+Kasatkin+site%3Apatchwork.kernel.org&hl=en&num=10&lr=&ft=i&cr=&safe=images
--
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
