Re-jig cpu_suspend for a saner calling convention

PATCH Superseded Russell King - ARM Linux 3 participants 20 messages Jun 13, 2011 – Jun 17, 2011

Subsystems (4)

Arm (32-bit) Arm sub-architectures Arm Samsung Exynos PXA2xx/PXA3xx support

lore

Share & tools

lore

Russell King - ARM Linux

author

2011-06-13 17:14 UTC #0 lore raw

cover Re-jig cpu_suspend for a saner calling convention

Currently cpu_suspend is not like a normal C function - when it's called it returns normally to a bunch of code which is not expected to return. The return path is via code pointed to by 'r3'.

It also corrupts a bunch of registers in ways which make it non-compliant with a C API.

If we do make this complaint as a normal C-like function, it eliminates this register saving. We also swap 'lr' and 'r3', so cpu_suspend effectively only returns to following code on resume - and r3 points to the suspend code.

So, this becomes:
ENTRY(acmeSoC_cpu_suspend)
stmfd sp!, {lr}
adr r3, soc_finish_suspend
bl cpu_suspend
ldmfd sp!, {pc}
ENDPROC(acmeSoC_cpu_suspend)

soc_finish_suspend:
blah
blah
put soc to sleep
never return

or even:

static void soc_suspend(void)
{
[soc specific preparation]

cpu_suspend(0, PLAT_PHYS_OFFSET - PAGE_OFFSET,
soc_suspend_arg, soc_suspend_fn);

[soc specific cleanup ]
}

where soc_suspend_fn can be either assembly or C code - but must never return.

Tested on Assabet (SA1100) only.

 arch/arm/kernel/sleep.S       |   70 +++++++++++++++-------------------------
 arch/arm/mach-exynos4/sleep.S |   10 ++----
 arch/arm/mach-pxa/sleep.S     |   48 ++++++++++++++--------------
 arch/arm/mach-s3c64xx/sleep.S |   11 ++-----
 arch/arm/mach-s5pv210/sleep.S |   10 ++----
 arch/arm/mach-sa1100/sleep.S  |   17 +++-------
 arch/arm/plat-s3c24xx/sleep.S |   12 ++-----
 7 files changed, 67 insertions(+), 111 deletions(-)

Russell King - ARM Linux

author

2011-06-13 17:15 UTC #1 lore raw

PATCH 1/14 ARM: suspend: make MULTI_CPU and !MULTI_CPU resume paths the same

Eliminate the differences between MULTI_CPU and non-MULTI_CPU resume paths, making the saved structure identical irrespective of the way the kernel was configured.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

arch/arm/kernel/sleep.S +3 -9

index 6398ead..97a6577 100644
@@ -40,9 +40,11 @@ ENTRY(cpu_suspend)
 #else
 	mov	r2, sp			@ current virtual SP
 	ldr	r0, =cpu_suspend_size
+	ldr	ip, =cpu_do_resume
 	sub	sp, sp, r0		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
-	stmfd	sp!, {r1, r2, r3}	@ save v:p, virt SP, return fn
+	add	ip, ip, r1		@ convert resume fn to phys
+	stmfd	sp!, {r1, r2, r3, ip}	@ save v:p, virt SP, retfn, phys resume fn
 	ldr	r3, =sleep_save_sp
 	add	r2, sp, r1		@ convert SP to phys
 #ifdef CONFIG_SMP

@@ -120,20 +122,12 @@ ENTRY(cpu_resume)
 	ldr	r0, sleep_save_sp	@ stack phys addr
 #endif
 	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1  @ set SVC, irqs off
-#ifdef MULTI_CPU
 	@ load v:p, stack, return fn, resume fn
   ARM(	ldmia	r0!, {r1, sp, lr, pc}	)
 THUMB(	ldmia	r0!, {r1, r2, r3, r4}	)
 THUMB(	mov	sp, r2			)
 THUMB(	mov	lr, r3			)
 THUMB(	bx	r4			)
-#else
-	@ load v:p, stack, return fn
-  ARM(	ldmia	r0!, {r1, sp, lr}	)
-THUMB(	ldmia	r0!, {r1, r2, lr}	)
-THUMB(	mov	sp, r2			)
-	b	cpu_do_resume
-#endif
 ENDPROC(cpu_resume)
 
 sleep_save_sp:

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:15 UTC #2 lore raw

PATCH 2/14 ARM: suspend: move return address (for cpu_resume) to top of stack

In reply to Russell King - ARM Linux

Move the return address for cpu_resume to the top of stack so that cpu_resume looks more like a normal function.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

arch/arm/kernel/sleep.S +8 -8

index 97a6577..f8e9251 100644
@@ -15,6 +15,7 @@
  * r0-r3,r9,r10,lr corrupted
  */
 ENTRY(cpu_suspend)
+	stmfd	sp!, {r3}
 	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor

@@ -24,7 +25,7 @@ ENTRY(cpu_suspend)
 	sub	sp, sp, r0		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
 	add	ip, ip, r1		@ convert resume fn to phys
-	stmfd	sp!, {r1, r2, r3, ip}	@ save v:p, virt SP, retfn, phys resume fn
+	stmfd	sp!, {r1, r2, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r3, =sleep_save_sp
 	add	r2, sp, r1		@ convert SP to phys
 #ifdef CONFIG_SMP

@@ -44,7 +45,7 @@ ENTRY(cpu_suspend)
 	sub	sp, sp, r0		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
 	add	ip, ip, r1		@ convert resume fn to phys
-	stmfd	sp!, {r1, r2, r3, ip}	@ save v:p, virt SP, retfn, phys resume fn
+	stmfd	sp!, {r1, r2, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r3, =sleep_save_sp
 	add	r2, sp, r1		@ convert SP to phys
 #ifdef CONFIG_SMP

@@ -99,7 +100,7 @@ ENDPROC(cpu_resume_turn_mmu_on)
 cpu_resume_after_mmu:
 	str	r5, [r2, r4, lsl #2]	@ restore old mapping
 	mcr	p15, 0, r0, c1, c0, 0	@ turn on D-cache
-	mov	pc, lr
+	ldmfd	sp!, {pc}
 ENDPROC(cpu_resume_after_mmu)
 
 /*

@@ -122,12 +123,11 @@ ENTRY(cpu_resume)
 	ldr	r0, sleep_save_sp	@ stack phys addr
 #endif
 	setmode	PSR_I_BIT | PSR_F_BIT | SVC_MODE, r1  @ set SVC, irqs off
-	@ load v:p, stack, return fn, resume fn
-  ARM(	ldmia	r0!, {r1, sp, lr, pc}	)
-THUMB(	ldmia	r0!, {r1, r2, r3, r4}	)
+	@ load v:p, stack, resume fn
+  ARM(	ldmia	r0!, {r1, sp, pc}	)
+THUMB(	ldmia	r0!, {r1, r2, r3}	)
 THUMB(	mov	sp, r2			)
-THUMB(	mov	lr, r3			)
-THUMB(	bx	r4			)
+THUMB(	bx	r3			)
 ENDPROC(cpu_resume)
 
 sleep_save_sp:

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:15 UTC #3 lore raw

PATCH 3/14 ARM: suspend: extract common code from MULTI_CPU/!MULTI_CPU paths

In reply to Russell King - ARM Linux

Very little code is different between these two paths now, so extract the common code.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |   24 ++++++------------------
 1 files changed, 6 insertions(+), 18 deletions(-)

arch/arm/kernel/sleep.S +6 ~~-18~~

index f8e9251..0a778c3 100644
@@ -19,29 +19,13 @@ ENTRY(cpu_suspend)
 	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor
-	mov	r2, sp			@ current virtual SP
 	ldr	r0, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state
 	ldr	ip, [r10, #CPU_DO_RESUME] @ virtual resume function
-	sub	sp, sp, r0		@ allocate CPU state on stack
-	mov	r0, sp			@ save pointer
-	add	ip, ip, r1		@ convert resume fn to phys
-	stmfd	sp!, {r1, r2, ip}	@ save v:p, virt SP, phys resume fn
-	ldr	r3, =sleep_save_sp
-	add	r2, sp, r1		@ convert SP to phys
-#ifdef CONFIG_SMP
-	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
-	ALT_UP(mov lr, #0)
-	and	lr, lr, #15
-	str	r2, [r3, lr, lsl #2]	@ save phys SP
 #else
-	str	r2, [r3]		@ save phys SP
-#endif
-	mov	lr, pc
-	ldr	pc, [r10, #CPU_DO_SUSPEND] @ save CPU state
-#else
-	mov	r2, sp			@ current virtual SP
 	ldr	r0, =cpu_suspend_size
 	ldr	ip, =cpu_do_resume
+#endif
+	mov	r2, sp			@ current virtual SP
 	sub	sp, sp, r0		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
 	add	ip, ip, r1		@ convert resume fn to phys

@@ -56,6 +40,10 @@ ENTRY(cpu_suspend)
 #else
 	str	r2, [r3]		@ save phys SP
 #endif
+#ifdef MULTI_CPU
+	mov	lr, pc
+	ldr	pc, [r10, #CPU_DO_SUSPEND] @ save CPU state
+#else
 	bl	cpu_do_suspend
 #endif

Signature · 1 line

1.7.4.4

Nicolas Pitre

2011-06-13 19:29 UTC #4 lore raw

PATCH 3/14 ARM: suspend: extract common code from MULTI_CPU/!MULTI_CPU paths

Quoted 2 lines · Russell King - ARM Linux

Very little code is different between these two paths now, so extract the common code.

What about simply always using the MULTI_CPU path unconditionally here? The struct processor pointer could be made available in all cases, and non critical paths could choose to always use it to simplify the code.

Quoted 61 lines

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |   24 ++++++------------------
 1 files changed, 6 insertions(+), 18 deletions(-)

arch/arm/kernel/sleep.S +6 ~~-18~~

index f8e9251..0a778c3 100644
@@ -19,29 +19,13 @@ ENTRY(cpu_suspend)
 	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor
-	mov	r2, sp			@ current virtual SP
 	ldr	r0, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state
 	ldr	ip, [r10, #CPU_DO_RESUME] @ virtual resume function
-	sub	sp, sp, r0		@ allocate CPU state on stack
-	mov	r0, sp			@ save pointer
-	add	ip, ip, r1		@ convert resume fn to phys
-	stmfd	sp!, {r1, r2, ip}	@ save v:p, virt SP, phys resume fn
-	ldr	r3, =sleep_save_sp
-	add	r2, sp, r1		@ convert SP to phys
-#ifdef CONFIG_SMP
-	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
-	ALT_UP(mov lr, #0)
-	and	lr, lr, #15
-	str	r2, [r3, lr, lsl #2]	@ save phys SP
 #else
-	str	r2, [r3]		@ save phys SP
-#endif
-	mov	lr, pc
-	ldr	pc, [r10, #CPU_DO_SUSPEND] @ save CPU state
-#else
-	mov	r2, sp			@ current virtual SP
 	ldr	r0, =cpu_suspend_size
 	ldr	ip, =cpu_do_resume
+#endif
+	mov	r2, sp			@ current virtual SP
 	sub	sp, sp, r0		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
 	add	ip, ip, r1		@ convert resume fn to phys

@@ -56,6 +40,10 @@ ENTRY(cpu_suspend)
 #else
 	str	r2, [r3]		@ save phys SP
 #endif
+#ifdef MULTI_CPU
+	mov	lr, pc
+	ldr	pc, [r10, #CPU_DO_SUSPEND] @ save CPU state
+#else
 	bl	cpu_do_suspend
 #endif

Signature · 7 lines

1.7.4.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Russell King - ARM Linux

author

2011-06-13 20:01 UTC #5 lore raw

PATCH 3/14 ARM: suspend: extract common code from MULTI_CPU/!MULTI_CPU paths

Quoted 8 lines · Nicolas Pitre

Quoted 2 lines · Russell King - ARM Linux
Very little code is different between these two paths now, so extract the common code.
What about simply always using the MULTI_CPU path unconditionally here? The struct processor pointer could be made available in all cases, and non critical paths could choose to always use it to simplify the code.

For the ifdef sections I don't think its worth it.

Russell King - ARM Linux

author

2011-06-13 17:16 UTC #6 lore raw

PATCH 4/14 ARM: suspend: preserve r4 - r11 across a suspend

In reply to Russell King - ARM Linux

Make cpu_suspend()..return function preserve r4 to r11 across a suspend cycle. This is in preparation of relieving platform support code from this task.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

arch/arm/kernel/sleep.S +3 -2

index 0a778c3..8dbca93 100644
@@ -12,10 +12,11 @@
  *  r1 = v:p offset
  *  r3 = virtual return function
  * Note: sp is decremented to allocate space for CPU state on stack
- * r0-r3,r9,r10,lr corrupted
+ * r0-r3,ip,lr corrupted
  */
 ENTRY(cpu_suspend)
 	stmfd	sp!, {r3}
+	stmfd	sp!, {r4 - r11}
 	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor

@@ -88,7 +89,7 @@ ENDPROC(cpu_resume_turn_mmu_on)
 cpu_resume_after_mmu:
 	str	r5, [r2, r4, lsl #2]	@ restore old mapping
 	mcr	p15, 0, r0, c1, c0, 0	@ turn on D-cache
-	ldmfd	sp!, {pc}
+	ldmfd	sp!, {r4 - r11, pc}
 ENDPROC(cpu_resume_after_mmu)
 
 /*

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:16 UTC #7 lore raw

PATCH 5/14 ARM: suspend: reallocate registers to avoid r2, r3

In reply to Russell King - ARM Linux

Avoid using r2 and r3 in the suspend code, allowing these to be passed further into the function as arguments.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

arch/arm/kernel/sleep.S +9 -9

index 8dbca93..358be13 100644
@@ -20,26 +20,26 @@ ENTRY(cpu_suspend)
 	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor
-	ldr	r0, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state
+	ldr	r5, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state
 	ldr	ip, [r10, #CPU_DO_RESUME] @ virtual resume function
 #else
-	ldr	r0, =cpu_suspend_size
+	ldr	r5, =cpu_suspend_size
 	ldr	ip, =cpu_do_resume
 #endif
-	mov	r2, sp			@ current virtual SP
-	sub	sp, sp, r0		@ allocate CPU state on stack
+	mov	r6, sp			@ current virtual SP
+	sub	sp, sp, r5		@ allocate CPU state on stack
 	mov	r0, sp			@ save pointer
 	add	ip, ip, r1		@ convert resume fn to phys
-	stmfd	sp!, {r1, r2, ip}	@ save v:p, virt SP, phys resume fn
-	ldr	r3, =sleep_save_sp
-	add	r2, sp, r1		@ convert SP to phys
+	stmfd	sp!, {r1, r6, ip}	@ save v:p, virt SP, phys resume fn
+	ldr	r5, =sleep_save_sp
+	add	r6, sp, r1		@ convert SP to phys
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
 	ALT_UP(mov lr, #0)
 	and	lr, lr, #15
-	str	r2, [r3, lr, lsl #2]	@ save phys SP
+	str	r6, [r5, lr, lsl #2]	@ save phys SP
 #else
-	str	r2, [r3]		@ save phys SP
+	str	r6, [r5]		@ save phys SP
 #endif
 #ifdef MULTI_CPU
 	mov	lr, pc

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:16 UTC #8 lore raw

PATCH 6/14 ARM: suspend: rejig suspend follow-on function calling convention

In reply to Russell King - ARM Linux

Save the suspend function pointer onto the stack for use when returning. Allocate r2 to pass an argument to the suspend function.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

arch/arm/kernel/sleep.S +5 -4

index 358be13..b924bcc 100644
@@ -10,6 +10,7 @@
 /*
  * Save CPU state for a suspend
  *  r1 = v:p offset
+ *  r2 = suspend function arg0
  *  r3 = virtual return function
  * Note: sp is decremented to allocate space for CPU state on stack
  * r0-r3,ip,lr corrupted

@@ -17,7 +18,6 @@
 ENTRY(cpu_suspend)
 	stmfd	sp!, {r3}
 	stmfd	sp!, {r4 - r11}
-	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor
 	ldr	r5, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state

@@ -33,6 +33,7 @@ ENTRY(cpu_suspend)
 	stmfd	sp!, {r1, r6, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r5, =sleep_save_sp
 	add	r6, sp, r1		@ convert SP to phys
+	stmfd	sp!, {r2, lr}		@ save suspend func arg and pointer
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
 	ALT_UP(mov lr, #0)

@@ -51,12 +52,12 @@ ENTRY(cpu_suspend)
 	@ flush data cache
 #ifdef MULTI_CACHE
 	ldr	r10, =cpu_cache
-	mov	lr, r9
+	mov	lr, pc
 	ldr	pc, [r10, #CACHE_FLUSH_KERN_ALL]
 #else
-	mov	lr, r9
-	b	__cpuc_flush_kern_all
+	bl	__cpuc_flush_kern_all
 #endif
+	ldmfd	sp!, {r0, pc}		@ call suspend fn
 ENDPROC(cpu_suspend)
 	.ltorg

Signature · 1 line

1.7.4.4

Rob Herring

2011-06-17 02:54 UTC #9 lore raw

PATCH 6/14 ARM: suspend: rejig suspend follow-on function calling convention

Russell,

Quoted 38 lines · Russell King - ARM Linux

Save the suspend function pointer onto the stack for use when returning. Allocate r2 to pass an argument to the suspend function.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

arch/arm/kernel/sleep.S +2 -1

index 358be13..b924bcc 100644
@@ -10,6 +10,7 @@
 /*
  * Save CPU state for a suspend
  *  r1 = v:p offset
+ *  r2 = suspend function arg0
  *  r3 = virtual return function
  * Note: sp is decremented to allocate space for CPU state on stack
  * r0-r3,ip,lr corrupted

@@ -17,7 +18,6 @@
 ENTRY(cpu_suspend)
 	stmfd	sp!, {r3}
 	stmfd	sp!, {r4 - r11}
-	mov	r9, lr
 #ifdef MULTI_CPU
 	ldr	r10, =processor
 	ldr	r5, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state

@@ -33,6 +33,7 @@ ENTRY(cpu_suspend)
 	stmfd	sp!, {r1, r6, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r5, =sleep_save_sp
 	add	r6, sp, r1		@ convert SP to phys
+	stmfd	sp!, {r2, lr}		@ save suspend func arg and pointer
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
 	ALT_UP(mov lr, #0)

@@ -51,12 +52,12 @@ ENTRY(cpu_suspend)
 	@ flush data cache

arch/arm/kernel/sleep.S ENTRY(cpu_suspend) · line 59

Quoted 10 lines

arch/arm/kernel/sleep.S +2 -3

 #ifdef MULTI_CACHE
 	ldr	r10, =cpu_cache
-	mov	lr, r9
+	mov	lr, pc
 	ldr	pc, [r10, #CACHE_FLUSH_KERN_ALL]
 #else
-	mov	lr, r9
-	b	__cpuc_flush_kern_all
+	bl	__cpuc_flush_kern_all
 #endif

If you change:

arch/arm/kernel/sleep.S ENTRY(cpu_suspend) · line 60

Quoted 1 line

arch/arm/kernel/sleep.S +1 -0
+	ldmfd	sp!, {r0, pc}		@ call suspend fn

To:

	mov	lr, pc
	ldmfd	sp!, {r0, pc}
	ldmfd	sp!, {r1, r6, ip}
	mov	sp, r6
	ldmfd	sp!, {r4-r11, pc}

It will allow the suspend fn to return and abort suspend. With that, I'm able to use this for core powerdown in cpuidle.

Rob

Russell King - ARM Linux

author

2011-06-17 07:21 UTC #10 lore raw

PATCH 6/14 ARM: suspend: rejig suspend follow-on function calling convention

Quoted 5 lines · Rob Herring

If you change:
arch/arm/kernel/sleep.S ENTRY(cpu_suspend) · line 60
Quoted 1 line
arch/arm/kernel/sleep.S +1 -0
+	ldmfd	sp!, {r0, pc}		@ call suspend fn
To:

Quoted 8 lines

	mov	lr, pc
	ldmfd	sp!, {r0, pc}
	ldmfd	sp!, {r1, r6, ip}
	mov	sp, r6
	ldmfd	sp!, {r4-r11, pc}
It will allow the suspend fn to return and abort suspend. With that, I'm able to use this for core powerdown in cpuidle.

Did you see my last message in the "[RFC PATCH v5] ARM hibernation / suspend-to-disk (fwd)" thread?

Russell King - ARM Linux

author

2011-06-13 17:17 UTC #11 lore raw

PATCH 7/14 ARM: suspend: move sa1100 to use proper suspend func arg0

In reply to Russell King - ARM Linux

In the previous commit, we introduced an official way to supply an argument to the suspend function. Convert the sa1100 suspend code to use this method.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-pxa/sleep.S |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

arch/arm/mach-pxa/sleep.S +4 -4

index 6f53688..613ddfa 100644
@@ -59,7 +59,7 @@ ENTRY(pxa27x_cpu_suspend)
 	mra	r2, r3, acc0
 #endif
 	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
-	mov	r4, r0				@ save sleep mode
+	mov	r2, r0				@ save sleep mode
 	ldr	r3, =pxa_cpu_resume		@ resume function
 	bl	cpu_suspend

@@ -67,7 +67,7 @@ ENTRY(pxa27x_cpu_suspend)
 	@ (also workaround for sighting 28071)
 
 	@ prepare value for sleep mode
-	mov	r1, r4				@ sleep mode
+	mov	r1, r0				@ sleep mode
 
 	@ prepare pointer to physical address 0 (virtual mapping in generic.c)
 	mov	r2, #UNCACHED_PHYS_0

@@ -109,11 +109,11 @@ ENTRY(pxa27x_cpu_suspend)
 
 ENTRY(pxa25x_cpu_suspend)
 	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
-	mov	r4, r0				@ save sleep mode
+	mov	r2, r0				@ save sleep mode
 	ldr	r3, =pxa_cpu_resume		@ resume function
 	bl	cpu_suspend
 	@ prepare value for sleep mode
-	mov	r1, r4				@ sleep mode
+	mov	r1, r0				@ sleep mode
 
 	@ prepare pointer to physical address 0 (virtual mapping in generic.c)
 	mov	r2, #UNCACHED_PHYS_0

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:17 UTC #12 lore raw

PATCH 8/14 ARM: suspend: convert cpu_suspend() to a normal function

In reply to Russell King - ARM Linux

cpu_suspend() has a weird calling method which makes it only possible to call from assembly code: it returns with a modified stack pointer to finish the suspend, but on resume, it 'returns' via a provided pointer.

We can make cpu_suspend() appear to be a normal function merely by swapping the resume pointer argument and the link register.

Do so, and update all callers to take account of this more traditional behaviour.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/kernel/sleep.S       |   10 ++++------
 arch/arm/mach-exynos4/sleep.S |    7 +++----
 arch/arm/mach-pxa/sleep.S     |   13 ++++++++++---
 arch/arm/mach-s3c64xx/sleep.S |    9 +++------
 arch/arm/mach-s5pv210/sleep.S |    7 +++----
 arch/arm/mach-sa1100/sleep.S  |   15 ++++-----------
 arch/arm/plat-s3c24xx/sleep.S |   10 +++-------
 7 files changed, 30 insertions(+), 41 deletions(-)

arch/arm/kernel/sleep.S +4 -6

index b924bcc..e062677 100644
@@ -11,13 +11,11 @@
  * Save CPU state for a suspend
  *  r1 = v:p offset
  *  r2 = suspend function arg0
- *  r3 = virtual return function
- * Note: sp is decremented to allocate space for CPU state on stack
- * r0-r3,ip,lr corrupted
+ *  r3 = suspend function
+ * Note: does not return until system resumes
  */
 ENTRY(cpu_suspend)
-	stmfd	sp!, {r3}
-	stmfd	sp!, {r4 - r11}
+	stmfd	sp!, {r4 - r11, lr}
 #ifdef MULTI_CPU
 	ldr	r10, =processor
 	ldr	r5, [r10, #CPU_SLEEP_SIZE] @ size of CPU sleep state

@@ -33,7 +31,7 @@ ENTRY(cpu_suspend)
 	stmfd	sp!, {r1, r6, ip}	@ save v:p, virt SP, phys resume fn
 	ldr	r5, =sleep_save_sp
 	add	r6, sp, r1		@ convert SP to phys
-	stmfd	sp!, {r2, lr}		@ save suspend func arg and pointer
+	stmfd	sp!, {r2, r3}		@ save suspend func arg and pointer
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc p15, 0, lr, c0, c0, 5)
 	ALT_UP(mov lr, #0)

arch/arm/mach-exynos4/sleep.S +3 -4

index 6b62425..d9a2287 100644
@@ -42,16 +42,15 @@
 ENTRY(s3c_cpu_save)
 
 	stmfd	sp!, { r3 - r12, lr }
-	ldr	r3, =resume_with_mmu
+	adr	r3, BSYM(exynos4_finish_suspend)
 	bl	cpu_suspend
+	ldmfd	sp!, { r3 - r12, pc }
 
+exynos4_finish_suspend:
 	ldr	r0, =pm_cpu_sleep
 	ldr	r0, [ r0 ]
 	mov	pc, r0
 
-resume_with_mmu:
-	ldmfd	sp!, { r3 - r12, pc }
-
 	.ltorg
 
 	/*

arch/arm/mach-pxa/sleep.S +10 -3

index 613ddfa..3a67887 100644
@@ -35,9 +35,11 @@ ENTRY(pxa3xx_cpu_suspend)
 #endif
 	stmfd	sp!, {r2 - r12, lr}	@ save registers on stack
 	mov	r1, r0
-	ldr	r3, =pxa_cpu_resume	@ resume function
+	adr	r3, BSYM(pxa3xx_finish_suspend)
 	bl	cpu_suspend
+	b	pxa_cpu_resume
 
+pxa3xx_finish_suspend:
 	mov	r0, #0x06		@ S2D3C4 mode
 	mcr	p14, 0, r0, c7, c0, 0	@ enter sleep

@@ -60,9 +62,11 @@ ENTRY(pxa27x_cpu_suspend)
 #endif
 	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
 	mov	r2, r0				@ save sleep mode
-	ldr	r3, =pxa_cpu_resume		@ resume function
+	adr	r3, BSYM(pxa27x_finish_suspend)
 	bl	cpu_suspend
+	b	pxa_cpu_resume
 
+pxa27x_finish_suspend:
 	@ Put the processor to sleep
 	@ (also workaround for sighting 28071)

@@ -110,8 +114,11 @@ ENTRY(pxa27x_cpu_suspend)
 ENTRY(pxa25x_cpu_suspend)
 	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
 	mov	r2, r0				@ save sleep mode
-	ldr	r3, =pxa_cpu_resume		@ resume function
+	adr	r3, BSYM(pxa25x_finish_suspend)
 	bl	cpu_suspend
+	b	pxa_cpu_resume
+
+pxa25x_finish_suspend:
 	@ prepare value for sleep mode
 	mov	r1, r0				@ sleep mode

arch/arm/mach-s3c64xx/sleep.S +3 -6

index 1f87732..dc4f582 100644
@@ -36,18 +36,15 @@
 
 ENTRY(s3c_cpu_save)
 	stmfd	sp!, { r4 - r12, lr }
-	ldr	r3, =resume_with_mmu
+	adr	r3, BSYM(s3c64xx_finish_suspend)
 	bl	cpu_suspend
+	ldmfd	sp!, { r4 - r12, pc }
 
+s3c64xx_finish_suspend:
 	@@ call final suspend code
 	ldr	r0, =pm_cpu_sleep
 	ldr	pc, [r0]
 	
-	@@ return to the caller, after the MMU is turned on.
-	@@ restore the last bits of the stack and return.
-resume_with_mmu:
-	ldmfd	sp!, { r4 - r12, pc }	@ return, from sp from s3c_cpu_save
-
 	/* Sleep magic, the word before the resume entry point so that the
 	 * bootloader can check for a resumeable image. */

arch/arm/mach-s5pv210/sleep.S +3 -4

index a3d6494..1182fc8 100644
@@ -41,16 +41,15 @@
 ENTRY(s3c_cpu_save)
 
 	stmfd	sp!, { r3 - r12, lr }
-	ldr	r3, =resume_with_mmu
+	adr	r3, BSYM(s5pv210_finish_suspend)
 	bl	cpu_suspend
+	ldmfd	sp!, { r3 - r12, pc }
 
+s5pv210_finish_suspend:
 	ldr	r0, =pm_cpu_sleep
 	ldr	r0, [ r0 ]
 	mov	pc, r0
 
-resume_with_mmu:
-	ldmfd	sp!, { r3 - r12, pc }
-
 	.ltorg
 
 	/* sleep magic, to allow the bootloader to check for an valid

arch/arm/mach-sa1100/sleep.S +4 ~~-11~~

index 04f2a61..214f4e2 100644
@@ -31,9 +31,12 @@
 ENTRY(sa1100_cpu_suspend)
 	stmfd	sp!, {r4 - r12, lr}		@ save registers on stack
 	mov	r1, r0
-	ldr	r3, =sa1100_cpu_resume		@ return function
+	adr	r3, BSYM(sa1100_finish_suspend)
 	bl	cpu_suspend
+	mcr	p15, 0, r1, c15, c1, 2		@ enable clock switching
+	ldmfd	sp!, {r4 - r12, pc}		@ return to caller
 
+sa1100_finish_suspend:
 	@ disable clock switching
 	mcr	p15, 0, r1, c15, c2, 2

@@ -139,13 +142,3 @@ sa1110_sdram_controller_fix:
 	str	r13, [r12]
 
 20:	b	20b			@ loop waiting for sleep
-
-/*
- * cpu_sa1100_resume()
- *
- * entry point from bootloader into kernel during resume
- */
-	.align 5
-sa1100_cpu_resume:
-	mcr	p15, 0, r1, c15, c1, 2		@ enable clock switching
-	ldmfd	sp!, {r4 - r12, pc}		@ return to caller

arch/arm/plat-s3c24xx/sleep.S +3 -7

index fd7032f..f822e62 100644
@@ -49,21 +49,17 @@
 
 ENTRY(s3c_cpu_save)
 	stmfd	sp!, { r4 - r12, lr }
-	ldr	r3, =resume_with_mmu
+	adr	r3, BSYM(s3c24xx_finish_suspend)
 	bl	cpu_suspend
+	ldmfd	sp!, { r4 - r12, pc }
 
+s3c24xx_finish_suspend:
 	@@ jump to final code to send system to sleep
 	ldr	r0, =pm_cpu_sleep
 	@@ldr	pc, [ r0 ]
 	ldr	r0, [ r0 ]
 	mov	pc, r0
 	
-	@@ return to the caller, after having the MMU
-	@@ turned on, this restores the last bits from the
-	@@ stack
-resume_with_mmu:
-	ldmfd	sp!, { r4 - r12, pc }
-
 	.ltorg
 
 	/* sleep magic, to allow the bootloader to check for an valid

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:17 UTC #13 lore raw

PATCH 9/14 ARM: suspend: plat-s3c24xx: cleanup s3c_cpu_save

In reply to Russell King - ARM Linux

s3c_cpu_save does not need to save any registers with the new cpu_suspend calling convention. Remove these redundant instructions.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/plat-s3c24xx/sleep.S |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

arch/arm/plat-s3c24xx/sleep.S +1 -3

index f822e62..6ada459 100644
@@ -48,10 +48,8 @@
 	*/
 
 ENTRY(s3c_cpu_save)
-	stmfd	sp!, { r4 - r12, lr }
 	adr	r3, BSYM(s3c24xx_finish_suspend)
-	bl	cpu_suspend
-	ldmfd	sp!, { r4 - r12, pc }
+	b	cpu_suspend
 
 s3c24xx_finish_suspend:
 	@@ jump to final code to send system to sleep

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:18 UTC #14 lore raw

PATCH 10/14 ARM: suspend: sa1100: cleanup sa1100_cpu_suspend

In reply to Russell King - ARM Linux

sa1100_cpu_suspend does not need to save any registers to the stack with the new cpu_suspend calling convention. Remove these redundant instructions.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-sa1100/sleep.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-sa1100/sleep.S +2 -2

index 214f4e2..e814b60 100644
@@ -29,12 +29,12 @@
  */
 
 ENTRY(sa1100_cpu_suspend)
-	stmfd	sp!, {r4 - r12, lr}		@ save registers on stack
+	mov	r9, lr
 	mov	r1, r0
 	adr	r3, BSYM(sa1100_finish_suspend)
 	bl	cpu_suspend
 	mcr	p15, 0, r1, c15, c1, 2		@ enable clock switching
-	ldmfd	sp!, {r4 - r12, pc}		@ return to caller
+	mov	pc, r9				@ return to caller
 
 sa1100_finish_suspend:
 	@ disable clock switching

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:18 UTC #15 lore raw

PATCH 11/14 ARM: suspend: mach-s5pv210: cleanup s3c_cpu_save

In reply to Russell King - ARM Linux

s3c_cpu_save does not need to save any registers with the new cpu_suspend calling convention. Remove these redundant instructions.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-s5pv210/sleep.S |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

arch/arm/mach-s5pv210/sleep.S +1 -4

index 1182fc8..28dfeff 100644
@@ -39,11 +39,8 @@
 	*/
 
 ENTRY(s3c_cpu_save)
-
-	stmfd	sp!, { r3 - r12, lr }
 	adr	r3, BSYM(s5pv210_finish_suspend)
-	bl	cpu_suspend
-	ldmfd	sp!, { r3 - r12, pc }
+	b	cpu_suspend
 
 s5pv210_finish_suspend:
 	ldr	r0, =pm_cpu_sleep

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:18 UTC #16 lore raw

PATCH 12/14 ARM: suspend: mach-exynos4: cleanup s3c_cpu_save

In reply to Russell King - ARM Linux

s3c_cpu_save does not need to save any registers with the new cpu_suspend calling convention. Remove these redundant instructions.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-exynos4/sleep.S |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

arch/arm/mach-exynos4/sleep.S +1 -4

index d9a2287..b56a491 100644
@@ -40,11 +40,8 @@
 	 */
 
 ENTRY(s3c_cpu_save)
-
-	stmfd	sp!, { r3 - r12, lr }
 	adr	r3, BSYM(exynos4_finish_suspend)
-	bl	cpu_suspend
-	ldmfd	sp!, { r3 - r12, pc }
+	b	cpu_suspend
 
 exynos4_finish_suspend:
 	ldr	r0, =pm_cpu_sleep

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:19 UTC #17 lore raw

PATCH 13/14 ARM: suspend: mach-s3c64xx: cleanup s3c_cpu_save

In reply to Russell King - ARM Linux

s3c_cpu_save does not need to save any registers with the new cpu_suspend calling convention. Remove these redundant instructions.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-s3c64xx/sleep.S |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

arch/arm/mach-s3c64xx/sleep.S +1 -3

index dc4f582..2fd3433 100644
@@ -35,10 +35,8 @@
 	*/
 
 ENTRY(s3c_cpu_save)
-	stmfd	sp!, { r4 - r12, lr }
 	adr	r3, BSYM(s3c64xx_finish_suspend)
-	bl	cpu_suspend
-	ldmfd	sp!, { r4 - r12, pc }
+	b	cpu_suspend
 
 s3c64xx_finish_suspend:
 	@@ call final suspend code

Signature · 1 line

1.7.4.4

Russell King - ARM Linux

author

2011-06-13 17:19 UTC #18 lore raw

PATCH 14/14 ARM: suspend: pxa: cleanup PXA suspend code

In reply to Russell King - ARM Linux

Remove now redundant stacking of registers.

Signed-off-by Russell King <[email protected]>

---
 arch/arm/mach-pxa/sleep.S |   33 +++++++++++++--------------------
 1 files changed, 13 insertions(+), 20 deletions(-)

arch/arm/mach-pxa/sleep.S +13 ~~-20~~

index 3a67887..cd39293 100644
@@ -33,11 +33,15 @@ ENTRY(pxa3xx_cpu_suspend)
 #ifndef CONFIG_IWMMXT
 	mra	r2, r3, acc0
 #endif
-	stmfd	sp!, {r2 - r12, lr}	@ save registers on stack
+	stmfd	sp!, {r2, r3, lr}	@ save registers on stack
 	mov	r1, r0
 	adr	r3, BSYM(pxa3xx_finish_suspend)
 	bl	cpu_suspend
-	b	pxa_cpu_resume
+	ldmfd	sp!, {r2, r3, lr}
+#ifndef CONFIG_IWMMXT
+	mar	acc0, r2, r3
+#endif
+	mov	pc, lr
 
 pxa3xx_finish_suspend:
 	mov	r0, #0x06		@ S2D3C4 mode

@@ -60,11 +64,15 @@ ENTRY(pxa27x_cpu_suspend)
 #ifndef CONFIG_IWMMXT
 	mra	r2, r3, acc0
 #endif
-	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
+	stmfd	sp!, {r2, r3, lr}		@ save registers on stack
 	mov	r2, r0				@ save sleep mode
 	adr	r3, BSYM(pxa27x_finish_suspend)
 	bl	cpu_suspend
-	b	pxa_cpu_resume
+	ldmfd	sp!, {r2, r3, lr}
+#ifndef CONFIG_IWMMXT
+	mar	acc0, r2, r3
+#endif
+	mov	pc, lr
 
 pxa27x_finish_suspend:
 	@ Put the processor to sleep

@@ -112,11 +120,9 @@ pxa27x_finish_suspend:
  */
 
 ENTRY(pxa25x_cpu_suspend)
-	stmfd	sp!, {r2 - r12, lr}		@ save registers on stack
 	mov	r2, r0				@ save sleep mode
 	adr	r3, BSYM(pxa25x_finish_suspend)
-	bl	cpu_suspend
-	b	pxa_cpu_resume
+	b	cpu_suspend
 
 pxa25x_finish_suspend:
 	@ prepare value for sleep mode

@@ -202,16 +208,3 @@ pxa_cpu_do_suspend:
 	mcr	p14, 0, r1, c7, c0, 0		@ PWRMODE
 
 20:	b	20b				@ loop waiting for sleep
-
-/*
- * 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

Signature · 1 line

1.7.4.4

Nicolas Pitre

2011-06-13 19:16 UTC #19 lore raw

cover Re-jig cpu_suspend for a saner calling convention

In reply to Russell King - ARM Linux

Quoted 26 lines · Russell King - ARM Linux

Currently cpu_suspend is not like a normal C function - when it's called it returns normally to a bunch of code which is not expected to return. The return path is via code pointed to by 'r3'.
It also corrupts a bunch of registers in ways which make it non-compliant with a C API.
If we do make this complaint as a normal C-like function, it eliminates this register saving. We also swap 'lr' and 'r3', so cpu_suspend effectively only returns to following code on resume - and r3 points to the suspend code.
So, this becomes:
ENTRY(acmeSoC_cpu_suspend)
stmfd sp!, {lr}
adr r3, soc_finish_suspend
bl cpu_suspend
ldmfd sp!, {pc}
ENDPROC(acmeSoC_cpu_suspend)
soc_finish_suspend:
blah
blah
put soc to sleep
never return

Quoted 7 lines

or even:
static void soc_suspend(void)
{
[soc specific preparation]
cpu_suspend(0, PLAT_PHYS_OFFSET - PAGE_OFFSET,

Please let's try not to use PLAT_PHYS_OFFSET if it can be avoided, especially when the plain PHYS_OFFSET would do just as well. I'm currently trying to get rid of all instances of PLAT_PHYS_OFFSET from the kernel in order to eventually support multiple SOCs in a single kernel image.

Nicolas