ARM: cpu_pj4b_suspend_size should base on cpu_v7_suspend_size

PATCH New Shawn Guo 2 participants 4 messages Jul 2, 2014 – Jul 3, 2014 Arm (32-bit)

Share & tools

Shawn Guo

author

2014-07-02 06:48 UTC #0 lore raw

PATCH 1/1 ARM: cpu_pj4b_suspend_size should base on cpu_v7_suspend_size

Since pj4b suspend/resume routines are implemented based on generic ARMv7 ones, instead of hard-coding cpu_pj4b_suspend_size, we should have it be cpu_v7_suspend_size plus pj4b specific bytes. Otherwise, if cpu_v7_suspend_size gets updated alone, the pj4b suspend/resume will likely be broken.

While at it, fix the comments in cpu_pj4b_do_resume, as we're restoring CP15 registers rather than saving in there.

Signed-off-by Shawn Guo <[email protected]>

---
Completely untested.

 arch/arm/mm/proc-v7.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mm/proc-v7.S +6 -6

index 3db2c2f04a30..604dda5f65cc 100644
@@ -184,16 +184,16 @@ ENDPROC(cpu_pj4b_do_suspend)
 
 ENTRY(cpu_pj4b_do_resume)
 	ldmia	r0!, {r6 - r10}
-	mcr	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
-	mcr	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
-	mcr	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
-	mcr	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
-	mcr	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	mcr	p15, 1, r6, c15, c1, 0  @ retore CP15 - extra features
+	mcr	p15, 1, r7, c15, c2, 0	@ retore CP15 - Aux Func Modes Ctrl 0
+	mcr	p15, 1, r8, c15, c1, 2	@ retore CP15 - Aux Debug Modes Ctrl 2
+	mcr	p15, 1, r9, c15, c1, 1  @ retore CP15 - Aux Debug Modes Ctrl 1
+	mcr	p15, 0, r10, c9, c14, 0  @ retore CP15 - PMC
 	b cpu_v7_do_resume
 ENDPROC(cpu_pj4b_do_resume)
 #endif
 .globl	cpu_pj4b_suspend_size
-.equ	cpu_pj4b_suspend_size, 4 * 14
+.equ	cpu_pj4b_suspend_size, cpu_v7_suspend_size + 4 * 5
 
 #endif

Signature · 1 line

1.8.3.2

Gregory CLEMENT

2014-07-02 07:12 UTC #1 lore raw

PATCH 1/1 ARM: cpu_pj4b_suspend_size should base on cpu_v7_suspend_size

Hi Shawn,

Quoted 5 lines · Shawn Guo

Since pj4b suspend/resume routines are implemented based on generic ARMv7 ones, instead of hard-coding cpu_pj4b_suspend_size, we should have it be cpu_v7_suspend_size plus pj4b specific bytes. Otherwise, if cpu_v7_suspend_size gets updated alone, the pj4b suspend/resume will likely be broken.

It was something I thought I did but I forgot to push it! Thanks for having spotted it. I am going to test it soon.

Thanks,

Gregory

Quoted 39 lines

While at it, fix the comments in cpu_pj4b_do_resume, as we're restoring CP15 registers rather than saving in there.

Signed-off-by Shawn Guo <[email protected]>

---
Completely untested.

 arch/arm/mm/proc-v7.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mm/proc-v7.S +6 -6

index 3db2c2f04a30..604dda5f65cc 100644
@@ -184,16 +184,16 @@ ENDPROC(cpu_pj4b_do_suspend)
 
 ENTRY(cpu_pj4b_do_resume)
 	ldmia	r0!, {r6 - r10}
-	mcr	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
-	mcr	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
-	mcr	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
-	mcr	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
-	mcr	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	mcr	p15, 1, r6, c15, c1, 0  @ retore CP15 - extra features
+	mcr	p15, 1, r7, c15, c2, 0	@ retore CP15 - Aux Func Modes Ctrl 0
+	mcr	p15, 1, r8, c15, c1, 2	@ retore CP15 - Aux Debug Modes Ctrl 2
+	mcr	p15, 1, r9, c15, c1, 1  @ retore CP15 - Aux Debug Modes Ctrl 1
+	mcr	p15, 0, r10, c9, c14, 0  @ retore CP15 - PMC
 	b cpu_v7_do_resume
 ENDPROC(cpu_pj4b_do_resume)
 #endif
 .globl	cpu_pj4b_suspend_size
-.equ	cpu_pj4b_suspend_size, 4 * 14
+.equ	cpu_pj4b_suspend_size, cpu_v7_suspend_size + 4 * 5
 
 #endif

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Gregory CLEMENT

2014-07-02 09:14 UTC #2 lore raw

PATCH 1/1 ARM: cpu_pj4b_suspend_size should base on cpu_v7_suspend_size

In reply to Shawn Guo

Hi Shawn,

Quoted 10 lines · Shawn Guo

Since pj4b suspend/resume routines are implemented based on generic ARMv7 ones, instead of hard-coding cpu_pj4b_suspend_size, we should have it be cpu_v7_suspend_size plus pj4b specific bytes. Otherwise, if cpu_v7_suspend_size gets updated alone, the pj4b suspend/resume will likely be broken.
While at it, fix the comments in cpu_pj4b_do_resume, as we're restoring CP15 registers rather than saving in there.
Signed-off-by Shawn Guo <[email protected]>

Acked-by Gregory CLEMENT <[email protected]>

and

Tested-by Gregory CLEMENT <[email protected]>

Thanks,

Gregory

Quoted 34 lines

---
Completely untested.

 arch/arm/mm/proc-v7.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

arch/arm/mm/proc-v7.S +6 -6

index 3db2c2f04a30..604dda5f65cc 100644
@@ -184,16 +184,16 @@ ENDPROC(cpu_pj4b_do_suspend)
 
 ENTRY(cpu_pj4b_do_resume)
 	ldmia	r0!, {r6 - r10}
-	mcr	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
-	mcr	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
-	mcr	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
-	mcr	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
-	mcr	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	mcr	p15, 1, r6, c15, c1, 0  @ retore CP15 - extra features
+	mcr	p15, 1, r7, c15, c2, 0	@ retore CP15 - Aux Func Modes Ctrl 0
+	mcr	p15, 1, r8, c15, c1, 2	@ retore CP15 - Aux Debug Modes Ctrl 2
+	mcr	p15, 1, r9, c15, c1, 1  @ retore CP15 - Aux Debug Modes Ctrl 1
+	mcr	p15, 0, r10, c9, c14, 0  @ retore CP15 - PMC
 	b cpu_v7_do_resume
 ENDPROC(cpu_pj4b_do_resume)
 #endif
 .globl	cpu_pj4b_suspend_size
-.equ	cpu_pj4b_suspend_size, 4 * 14
+.equ	cpu_pj4b_suspend_size, cpu_v7_suspend_size + 4 * 5
 
 #endif

Signature · 4 lines

Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Shawn Guo

author

2014-07-03 09:01 UTC #3 lore raw

PATCH 1/1 ARM: cpu_pj4b_suspend_size should base on cpu_v7_suspend_size

In reply to Shawn Guo

Quoted 24 lines · Shawn Guo

Since pj4b suspend/resume routines are implemented based on generic ARMv7 ones, instead of hard-coding cpu_pj4b_suspend_size, we should have it be cpu_v7_suspend_size plus pj4b specific bytes. Otherwise, if cpu_v7_suspend_size gets updated alone, the pj4b suspend/resume will likely be broken.
While at it, fix the comments in cpu_pj4b_do_resume, as we're restoring CP15 registers rather than saving in there.
Signed-off-by Shawn Guo <[email protected]>
---
Completely untested.
 arch/arm/mm/proc-v7.S | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
arch/arm/mm/proc-v7.S +0 -0
index 3db2c2f04a30..604dda5f65cc 100644
@@ -184,16 +184,16 @@ ENDPROC(cpu_pj4b_do_suspend)
 
 ENTRY(cpu_pj4b_do_resume)
 	ldmia	r0!, {r6 - r10}

arch/arm/mm/proc-v7.S ENDPROC(cpu_pj4b_do_suspend) · line 191

Quoted 10 lines

arch/arm/mm/proc-v7.S +5 -5

-	mcr	p15, 1, r6, c15, c1, 0  @ save CP15 - extra features
-	mcr	p15, 1, r7, c15, c2, 0	@ save CP15 - Aux Func Modes Ctrl 0
-	mcr	p15, 1, r8, c15, c1, 2	@ save CP15 - Aux Debug Modes Ctrl 2
-	mcr	p15, 1, r9, c15, c1, 1  @ save CP15 - Aux Debug Modes Ctrl 1
-	mcr	p15, 0, r10, c9, c14, 0  @ save CP15 - PMC
+	mcr	p15, 1, r6, c15, c1, 0  @ retore CP15 - extra features
+	mcr	p15, 1, r7, c15, c2, 0	@ retore CP15 - Aux Func Modes Ctrl 0
+	mcr	p15, 1, r8, c15, c1, 2	@ retore CP15 - Aux Debug Modes Ctrl 2
+	mcr	p15, 1, r9, c15, c1, 1  @ retore CP15 - Aux Debug Modes Ctrl 1
+	mcr	p15, 0, r10, c9, c14, 0  @ retore CP15 - PMC

s/retore/restore

I fixed the typo, added Gregory's Acked-by and Tested-by tags, and put it into patch system as 8089/1.

Shawn

Quoted 12 lines

arch/arm/mm/proc-v7.S +1 -1

 	b cpu_v7_do_resume
 ENDPROC(cpu_pj4b_do_resume)
 #endif
 .globl	cpu_pj4b_suspend_size
-.equ	cpu_pj4b_suspend_size, 4 * 14
+.equ	cpu_pj4b_suspend_size, cpu_v7_suspend_size + 4 * 5
 
 #endif

Signature · 1 line

1.8.3.2