[RFC][PATCH] ARM: Add initial hibernation support for Cortex A8 and A9

On 21 December 2010 05:43, MyungJoo Ham [off-list ref] wrote:

Hibernation (Suspend-To-Disk) support for ARM Cortex A8 and A9.

This patch is based on the work of Hiroshi DOYU at Nokia, which is
stated to be based on the original work of Romit and Raghu at TI.

The hibernation support is tested with S5PC210 (ARM Cortex A9 MP2). In
order to add support for Cortex A9 on the original patch of Hiroshi
DOYU, which support Cortex A8 only, we have edited the list of registers
to be saved in the case of A9.

In the original work of Hiroshi DOYU, arch/arm/kernel/vmlinux.lds.S was
modified; however, we have reverted it because it only created build
failure in our tested system.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Please note that I only had a quick look at the code and I won't have
time to do a proper review before the end of the year. But I have a
few comments which are consider essential.

We should use run-time detection of the CPU rather than #ifdef's. We
have the ability to build multiple processors support into the same
kernel image.

+struct saved_context {
+/*
+ * FIXME: Only support for Cortex A8 and Cortex A9 now
+ */
+ ? ? ? /* CR0 */
+ ? ? ? u32 cssr; ? ? ? /* Cache Size Selection */
+ ? ? ? /* CR1 */
+#ifdef CORTEX_A8
+ ? ? ? u32 cr; ? ? ? ? /* Control */
+ ? ? ? u32 cacr; ? ? ? /* Coprocessor Access Control */
+#elif defined(CORTEX_A9)
+ ? ? ? u32 cr;
+ ? ? ? u32 actlr;
+ ? ? ? u32 cacr;
+ ? ? ? u32 sder;
+ ? ? ? u32 vcr;
+#endif

We could follow the ARM ARM and define a save_context structure which
has architected registers and implementation-defined ones. For the
latter you could use a union inside this structure to differentiate
between A8, A9 and other registers for different implementations.

+ ? ? ? /* CR2 */
+ ? ? ? u32 ttb_0r; ? ? /* Translation Table Base 0 */
+ ? ? ? u32 ttb_1r; ? ? /* Translation Table Base 1 */
+ ? ? ? u32 ttbcr; ? ? ?/* Translation Talbe Base Control */
+ ? ? ? /* CR3 */
+ ? ? ? u32 dacr; ? ? ? /* Domain Access Control */
+ ? ? ? /* CR5 */
+ ? ? ? u32 d_fsr; ? ? ?/* Data Fault Status */
+ ? ? ? u32 i_fsr; ? ? ?/* Instruction Fault Status */
+ ? ? ? u32 d_afsr; ? ? /* Data Auxilirary Fault Status */ ? ? ? ;
+ ? ? ? u32 i_afsr; ? ? /* Instruction Auxilirary Fault Status */;
+       /* CR6 */
+       u32 d_far;      /* Data Fault Address */
+       u32 i_far;      /* Instruction Fault Address */

I haven't checked the ARM ARM but some registers may be read-only and
not really needing saving/restoring. Some of them also don't have any
information that is essential when coming back from hibernation (like
the FSR/FAR ones).

+static inline void __save_processor_state(struct saved_context *ctxt)
+{
+ ? ? ? /* CR0 */
+ ? ? ? asm volatile ("mrc p15, 2, %0, c0, c0, 0" : "=r"(ctxt->cssr));
+ ? ? ? /* CR1 */
+#ifdef CORTEX_A8
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c0, 0" : "=r"(ctxt->cr));
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c0, 2" : "=r"(ctxt->cacr));
+#elif defined(CORTEX_A9)
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c0, 0" : "=r"(ctxt->cr));
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c0, 1" : "=r"(ctxt->actlr));
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c0, 2" : "=r"(ctxt->cacr));
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c1, 1" : "=r"(ctxt->sder));
+ ? ? ? asm volatile ("mrc p15, 0, %0, c1, c1, 3" : "=r"(ctxt->vcr));
+#endif

I think this could be split into a generic state saving function and a
per-processor one. You can then do some checks on the CPU ID to and
call the corresponding implementation-specific save function.

+static inline void __restore_processor_state(struct saved_context *ctxt)
+{

[...]

+#ifdef CORTEX_A8
+ ? ? ? asm volatile ("mcr p15, 1, %0, c9, c0, 0" : : "r"(ctxt->l2clr));
+#endif

This register (and maybe others) is not always writable in non-secure
mode. If running in non-secure mode (which cannot easily be detected
at run-time), we should first check the Nonsecure Access Control
Register.

-- 
Catalin