[linux-pm] [RFC PATCH v4] ARM hibernation/suspend-to-disk support

[Frank Hofmann]

On Thu, 9 Jun 2011, Russell King - ARM Linux wrote:

> On Thu, Jun 09, 2011 at 10:23:07PM +0530, Santosh Shilimkar wrote:
[ ... ]
>> 3. Avoid direct write to AUXCTRL in generic suspend code.
>
> This is the only problematical one that I can see.  We need to restore
> this on systems running in secure mode.  What we could do is rather than
> writing to the register, read it first and compare its value with what
> was saved to see whether we need to write it.
>
> Then, if platforms run in non-secure mode, they are responsible for
> restoring that register back to its pre-suspend value before their
> assembly calls cpu_resume().

While this is ok from the point of view of having cpu_suspend / resume 
being service functions for the platform-specific idle/off-mode code, it 
also illustrates the difficulty this creates for the hibernation code.

If it's not possible to call cpu_suspend / cpu_resume (or something like 
it - not tied to names ...) as a full-featured generic interface, then 
creating a true snapshot capability becomes problematic.

>
>> 4. Before MMU is enabled in resume a callback to restore
>> secure register, setup auxctrl etc.
>
> You can do this before your assembly calls cpu_resume().

Only that it's known at the point of call to cpu_suspend/resume.

Again, I admit to being biased regarding the usecase here ...

See below.

>
>> Additionally the L2 cache handling isn't part of
>> these common suspend hooks.
>
> L2 cache handling can't fit into the generic code - it doesn't really
> belong there either.  It needs to be in the parent or hooked into the
> syscore_ops stuff as I've said previously.

For the "take things down" side, dode like machine_restart() already is 
able to flush/inval/disable all these things on the way down, without any 
SoC-specific knowledge.

OMAP suspend/resume has, just recently, gone half-way there (use the 
provided flush/inval instead of the home-grown table walker code),

Agree with that. Cache flushing / disabling / invalidation has interfaces 
(the outer_*, l2* and cache ops) already, and e.g. the OMAP code has 
recently started to use some of those (kernel_flush instead of the 
home-grown inval loop). Looks like that part is on its way.

On the resume side, there's actually a problem here - the generic way of 
enabling a cache is through initcalls - which don't happen on resume, so 
if the system comes out of a "low enough" state there's some work to do 
here - which generic cpu_resume() does not do.

>
> So:
>
> ENTRY(my_soc_suspend)
>        stmfd   sp!, {r4 - r12, lr}
>        ldr     r3, =resume
>        bl      cpu_suspend
> 	/*
> 	 * Insert whatever code is required here for suspend
> 	 * eg, save secure mode, then jump to sram to call WFI function
> 	 */
> resume:
> 	ldmfd	sp!, {r4 - r12, pc}
> ENDPROC(my_soc_suspend)
>
> ENTRY(my_soc_resume)
> 	/*
> 	 * Insert whatever other code is required to be run before resume
> 	 * eg, WFI function returns to this symbol after DDR becomes
> 	 * accessible.  restore secure mode state
> 	 */
> 	b	cpu_resume
> ENDPROC(my_soc_resume)

That alone doesn't accommodate the following situations:

a) there might be pre-suspend / post-resume activities necessary, i.e.
    the assumption that any SoC-specific "go down" activity can be done
    after cpu_suspend() and any SoC-specific "bring up" activity before
    cpu_resume() might not be sufficient.
    Case in point: Reenabling L2 caches after resume.

b) my bias - snapshotting state (for hibernation).
    Delegating this to a SoC-specific method risks creating code like
    the OMAP stuff - where state saving, power management, off mode
    and whatnot is all interwoven and interdependent.
    It also creates the problem that _generic_ (platform-independent)
    hibernation code becomes impossible to do ...

    A clean thing are separate steps:

 	<mach preparation for suspend state snapshot>
 	<generic state snapshot>
 	<mach state snapshot>
 	...
 	<wfi>	/* or not ... */
 	...
 	<mach prep for resume / basic initialization>
 	<generic resume>
 	<mach resume>

So why not have those hooks _inside_ cpu_suspend / cpu_resume, i.e. like:

.data
ENTRY(cpu_suspend)
 	mov	r9, lr
#ifdef	CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
 	mov	lr, pc
 	ldr	pc, mach_pre_suspend_hook
#endif
 	...
#ifdef	CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
 	mov	lr, r9
 	ldr	r4, mach_post_suspend_hook
 	b	r4
#else
 	mov	pc, r9
END(cpu_suspend)

#ifdef	CONFIG_ARCH_NEEDS_SPAGHETTI_SUSPEND
mach_pre_suspend_hook:
.long 0
mach_post_suspend_hook:
.long 0
#endif


and let the SoC initialization set them if it so desires ?


This would allow to use them for snapshotting the state as well.

Key point, again, from the hibernation bias, is really to have that 
stuff _separate_ from power-down / wfi / whatever-to-enter-lowpower-mode.


As many of these activities as possible should be dealt with by sysdev / 
syscore, agreed; but unfortunately there might be certain things, 
especially around secure state, that are too closely tied in to delegate 
it to those ?


>
> What makes it far more complicated in the OMAP case is all that "is l1 state
> lost?  is l2 state lost?" stuff.
>

It looks like it's structured this way:

omap_cpu_suspend()
{
 	switch (state) {
 	case 3:
 	case 1:
 		/* save context */
 	case 2:
 		/* clean caches */
 	case 0:
 		wfi();
 	}
}

omap_cpu_resume()	/* from OFF, case 2 / 0 never happens */
{
 	if (state == 3)
 		/* disable / inval L2 */

 	/* restore context */
 	/* reenable L2 */
}


But the code doesn't perfectly match the comments in it.


FrankH.