arm: use common instructions applicable to armv7m & other arm archs

PATCH Superseded Vikas Manocha 2 participants 3 messages Feb 4, 2016 – Feb 5, 2016 Arm (32-bit)

lore

Share & tools

lore

Vikas Manocha

author

2016-02-04 20:41 UTC #0 lore raw

PATCH v2 1/1 arm: use common instructions applicable to armv7m & other arm archs

This patch cleans the code by using instructions allowed for armv7m as well as other Arm archs.

Signed-off-by Vikas Manocha <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
Changes in v2: reword message commit. Removed info regarding BIC instruction on SP, was creating confusion.

 arch/arm/lib/crt0.S | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

arch/arm/lib/crt0.S +7 ~~-18~~

index 2f4c14e..3731d0c 100644
@@ -71,18 +71,12 @@ ENTRY(_main)
  */
 
 #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
-	ldr	sp, =(CONFIG_SPL_STACK)
+	ldr	r0, =(CONFIG_SPL_STACK)
 #else
-	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
+	ldr	r0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC destination */
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
-	mov	r0, sp
+	bic	r0, r0, #7	/* 8-byte alignment for ABI compliance */
+	mov	sp, r0
 	bl	board_init_f_alloc_reserve
 	mov	sp, r0
 	/* set up gd here, outside any C code */

@@ -100,14 +94,9 @@ ENTRY(_main)
  * 'here' but relocated.
  */
 
-	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC destination */
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
+	ldr	r0, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
+	bic	r0, r0, #7
+	mov	sp, r0		/* 8-byte alignment for ABI compliance */
 	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
 	sub	r9, r9, #GD_SIZE		/* new GD is below bd */

Signature · 1 line

1.9.1

Heiko Schocher

2016-02-05 05:38 UTC #1 lore raw

PATCH v2 1/1 arm: use common instructions applicable to armv7m & other arm archs

Hello Vikas,

Am 04.02.2016 um 21:41 schrieb Vikas Manocha:

Quoted 44 lines

This patch cleans the code by using instructions allowed for armv7m as well as other Arm archs.

Signed-off-by Vikas Manocha <[email protected]>

Reviewed-by Simon Glass <[email protected]>

---
Changes in v2: reword message commit. Removed info regarding BIC instruction on SP, was creating confusion.

  arch/arm/lib/crt0.S | 25 +++++++------------------
  1 file changed, 7 insertions(+), 18 deletions(-)

arch/arm/lib/crt0.S +4 ~~-11~~

index 2f4c14e..3731d0c 100644
@@ -71,18 +71,12 @@ ENTRY(_main)
   */
 
  #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
-	ldr	sp, =(CONFIG_SPL_STACK)
+	ldr	r0, =(CONFIG_SPL_STACK)
  #else
-	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
+	ldr	r0, =(CONFIG_SYS_INIT_SP_ADDR)
  #endif
-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC destination */
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
-	mov	r0, sp
+	bic	r0, r0, #7	/* 8-byte alignment for ABI compliance */
+	mov	sp, r0
  	bl	board_init_f_alloc_reserve
  	mov	sp, r0
  	/* set up gd here, outside any C code */

@@ -100,14 +94,9 @@ ENTRY(_main)
   * 'here' but relocated.
   */
 
-	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */

arch/arm/lib/crt0.S ENTRY(_main) · line 99

Quoted 10 lines

arch/arm/lib/crt0.S +3 -7

-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC destination */
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
+	ldr	r0, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
+	bic	r0, r0, #7
+	mov	sp, r0		/* 8-byte alignment for ABI compliance */

Why do you move this comment to the mov instruction? the "bic r0, r0, #7" does the 8-byte alignment ... please add this comment to this line, thanks!

arch/arm/lib/crt0.S ENTRY(_main) · line 101

Quoted 2 lines

arch/arm/lib/crt0.S +0 -0

  	ldr	r9, [r9, #GD_BD]		/* r9 = gd->bd */
  	sub	r9, r9, #GD_SIZE		/* new GD is below bd */

Beside of this nitpick:

Reviewed-by Heiko Schocher <[email protected]>

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Vikas MANOCHA

author

2016-02-05 18:29 UTC #2 lore raw

PATCH v2 1/1 arm: use common instructions applicable to armv7m & other arm archs

Thanks Heiko,

Quoted 59 lines

-----Original Message-----
From: Heiko Schocher [mailto:hs at denx.de]
Sent: Thursday, February 04, 2016 9:39 PM
To: Vikas MANOCHA
Cc: albert.u.boot at aribaud.net; Simon Glass; Przemyslaw Marczak; Heiko Schocher; rev13 at wp.pl; open list
Subject: Re: [PATCH v2] arm: use common instructions applicable to armv7m & other arm archs
Hello Vikas,
Am 04.02.2016 um 21:41 schrieb Vikas Manocha:
Quoted 17 lines
This patch cleans the code by using instructions allowed for armv7m as well as other Arm archs.
Signed-off-by Vikas Manocha <[email protected]>
Reviewed-by Simon Glass <[email protected]>
---
Changes in v2: reword message commit. Removed info regarding BIC instruction on SP, was creating confusion.
  arch/arm/lib/crt0.S | 25 +++++++------------------
  1 file changed, 7 insertions(+), 18 deletions(-)
arch/arm/lib/crt0.S index renamed from arch/arm/lib/crt0.S +0 -0
2f4c14e..3731d0c 100644
arch/arm/lib/crt0.S ENTRY(_main) · line 78
Quoted 10 lines
arch/arm/lib/crt0.S +2 -3
@@ -71,18 +71,12 @@ ENTRY(_main)
   */
 
  #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_STACK)
-	ldr	sp, =(CONFIG_SPL_STACK)
+	ldr	r0, =(CONFIG_SPL_STACK)
  #else
-	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
+	ldr	r0, =(CONFIG_SYS_INIT_SP_ADDR)
  #endif
-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC
destination */
Quoted 16 lines
arch/arm/lib/crt0.S +2 -7
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
-	mov	r0, sp
+	bic	r0, r0, #7	/* 8-byte alignment for ABI compliance */
+	mov	sp, r0
  	bl	board_init_f_alloc_reserve
  	mov	sp, r0
  	/* set up gd here, outside any C code */ @@ -100,14 +94,9 @@
ENTRY(_main)
* 'here' but relocated.
*/
arch/arm/lib/crt0.S
Quoted 2 lines
arch/arm/lib/crt0.S +0 -2
-	ldr	sp, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
-#if defined(CONFIG_CPU_V7M)	/* v7M forbids using SP as BIC
destination */

Quoted 12 lines

arch/arm/lib/crt0.S ENTRY(_main) · line 99
Quoted 9 lines
arch/arm/lib/crt0.S +3 -6
-	mov	r3, sp
-	bic	r3, r3, #7
-	mov	sp, r3
-#else
-	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
-#endif
+	ldr	r0, [r9, #GD_START_ADDR_SP]	/* sp = gd->start_addr_sp */
+	bic	r0, r0, #7
+	mov	sp, r0		/* 8-byte alignment for ABI compliance */
Why do you move this comment to the mov instruction? the "bic r0, r0, #7" does the 8-byte alignment ... please add this comment to this line, thanks!

Ok, I will send v3 with this correction.

Cheers,
Vikas

Quoted 12 lines

Quoted 3 lines
ldr r9, [r9, #GD_BD] /* r9 = gd->bd */
sub r9, r9, #GD_SIZE /* new GD is below bd */
Beside of this nitpick:
Reviewed-by Heiko Schocher <[email protected]>
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany