Re: [RFC PATCH v5] ARM hibernation / suspend-to-disk (fwd)

[Dave Martin <dave.martin@linaro.org>]

On Mon, Jun 13, 2011 at 02:20:12PM +0100, Frank Hofmann wrote:
> 
> 
> On Mon, 13 Jun 2011, Russell King - ARM Linux wrote:
> 
> >On Mon, Jun 13, 2011 at 01:04:02PM +0100, Frank Hofmann wrote:
> >>  To make it clear: IF AND ONLY IF your suspend(-to-ram) func looks like:
> >>
> >>	ENTRY(acmeSoC_cpu_suspend)
> >>		stmfd	sp!, {r4-r12,lr}
> >>		ldr	r3, resume_mmu_done
> >>		bl	cpu_suspend
> >>	resume_mmu_done:
> >>		ldmfd	sp!, {r3-r12,pc}
> >>	ENDPROC(acmeSoC_cpu_suspend)
> >
> >Nothing has that - because you can't execute that ldmfd after the call
> >to cpu_suspend returns.  I don't think you've understood what I said on
> >that subject in the previous thread.
> >
> 
> Ok, to illustrate a bit more, what is ok and what not.
> 
> 
> From the current code ...
> 
> a) a case where the hibernation patch as I posted it is ok for core state:
> 
> arch/arm/mach-exynos4/sleep.S has:
> 
> ENTRY(s3c_cpu_save)
> 
>         stmfd   sp!, { r3 - r12, lr }
>         ldr     r3, =resume_with_mmu
>         bl      cpu_suspend
> 
>         ldr     r0, =pm_cpu_sleep
>         ldr     r0, [ r0 ]
>         mov     pc, r0
> 
> resume_with_mmu:
>         ldmfd   sp!, { r3 - r12, pc }
> 
> ENTRY(s3c_cpu_resume)
>         b       cpu_resume
> 
> 
> I.e. it does nothing before but set up the arguments for
> cpu_suspend, and does nothing afterwards but redirect to a function
> that enters low power (and switches the CPU off).
> Likewise, all it does for resume is redirect to cpu_resume which
> will ultimately end up jumping to resume_with_mmu: as requested.
> 
> 
> b) a case where the hibernation patch is insufficient even though the
>    code in the soc suspend func uses cpu_suspend:
> 
> arch/arm/mach-pxa/sleep.S has:
> 
> /*
>  * pxa27x_cpu_suspend()
>  *
>  * Forces CPU into sleep state.
>  *
>  * r0 = value for PWRMODE M field for desired sleep state
>  * r1 = v:p offset
>  */
> ENTRY(pxa27x_cpu_suspend)
> 
> #ifndef CONFIG_IWMMXT
>         mra     r2, r3, acc0
> #endif
>         stmfd   sp!, {r2 - r12, lr}             @ save registers on stack
>         mov     r4, r0                          @ save sleep mode
>         ldr     r3, =pxa_cpu_resume             @ resume function
>         bl      cpu_suspend
> [ ... some stuff ... ]
>         ldr     r6, =CCCR
>         ldr     r8, [r6]                @ keep original value for resume
> 
>         ldr     r7, =CCCR_SLEEP         @ prepare CCCR sleep value
>         mov     r0, #0x2                @ prepare value for CLKCFG
> 
>         @ align execution to a cache line
>         b       pxa_cpu_do_suspend
> #endif
> [ ... ]
> pxa_cpu_do_suspend:
> 
>         @ All needed values are now in registers.
>         @ These last instructions should be in cache
> 
>         @ initiate the frequency change...
>         str     r7, [r6]
>         mcr     p14, 0, r0, c6, c0, 0
> 
>         @ restore the original cpu speed value for resume
>         str     r8, [r6]
> [ ... ]
> /*
>  * pxa_cpu_resume()
>  *
>  * entry point from bootloader into kernel during resume
>  */
>         .align 5
> pxa_cpu_resume:
>         ldmfd   sp!, {r2, r3}
> #ifndef CONFIG_IWMMXT
>         mar     acc0, r2, r3
> #endif
>         ldmfd   sp!, {r4 - r12, pc}             @ return to caller
> 
> 
> 
> In this case, there's _MORE_ state saved (the CCCR / CCCR_SLEEP
> accesses) after the call to cpu_suspend things that aren't dealt
> with by the way the hibernation support patch currently operates.
> 
> There also is _more_ restored at resume than what the generic part
> does for you.
> 
> Whether these things (for example) are relevant wrt. to a
> hibernation resume is something I simply do not know.
> 
> 
> A third example of a soc suspend func, even more complex, with more
> constraints, would be as already mentioned, OMAP. On that as well
> the caveat applies, "what works empirically might not be correct in
> all cases".

Santosh, Frank: to what extent do you think the OMAP suspend code could
be abstracted using something like Colin's CPU pm notifier framework?

I'd hope that at least some stuff can be abstracted out, but I don't
understand the OMAP code intimately enough to be certain of that...

We shouldn't expect to remove absolutely all the SoC specifics from
suspend code, but the more we can hook into a generic framework, the
better for everyone.

Cheers
---Dave