On Thu, May 17, 2012 at 4:06 AM, Kevin Hilman [off-list ref] wrote:

+Jean for functional power states

Tero Kristo [off-list ref] writes:

quoted

This patch adds device off support to OMAP4 device type.

Description is rather thin for a patch that is doing so much.

quoted

OFF mode is disabled by default,

why?

quoted

however, there are two ways to enable OFF mode:
a) In the board file, call omap4_pm_off_mode_enable(1)
b) Enable OFF mode using the debugfs entry
echo "1">/sys/kernel/debug/pm_debug/enable_off_mode
(conversely echo '0' will disable it as well).

This part needs to be a separate patch.

But, as stated in the core retention series, I'd like to move away from
these global flags all together.

The way we manage the disabling of certain states (like off) is already
clumsy for OMAP3, and it's getting worse with OMAP4. ?Basically, I think
this feature needs to be supported by using constraints on functional
power states. ? Having checks all over the place is getting unwieldy and
not attractive to maintain.

The combination of constraints and functional power states should make
this much more manageable. ? Until we have that, I'd prefer to keep
the debugfs enable/disable stuff as separate patches at the end of the
series used only for testing.

quoted

Signed-off-by: Santosh Shilimkar <redacted>
[t-kristo at ti.com: largely re-structured the code]

then the sign-off above from Santosh probably doesn't apply anymore.
You should change that to a Cc and just mention tht this is based upon
some original work from Santosh.

First, ?some general comments:

There is a lot going on in this patch, so it is hard to follow what all
is related, and why. ?Just a quick glance suggests it needs to be broken
up into at least a few parts:

- low-level PRM support: new APIs for various off-mode features)
?(should probably be done on top of functional power states)
- powerdomain core support or "achievable" states
?(should probably be done on top of functional power states)
- IRQ/GIC context save/restore
- secure RAM save/restore (this has been tightly coupled to the GIC
?but it's not obvious why)
- PM debug support to enable/disable off-mode
?(for testing only, not for merge)

Further split sounds good to me.

Regards
santosh