Fix board init code to use a valid C runtime environment

PATCH Superseded Albert ARIBAUD 2 participants 7 messages Nov 15, 2015 – Nov 16, 2015

Subsystems (6)

Arm (32-bit) x86 PowerPC Synopsys ARC architecture +2 more subsystems

lore

Share & tools

lore

Albert ARIBAUD

author

2015-11-15 18:25 UTC #0 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

board_init_f_mem() alters the C runtime environment's stack it ls actually already using. This is not a valid C runtime environment.

Split board_init_f_mem into C functions which do not alter their own stack and therefore run in a valid C runtime environment.

Signed-off-by Albert ARIBAUD <[email protected]>

---
Cc:ing custodians for all architectures which this patch affects.

This patch has been build-tested for all there architectures, and run-tested on ARM OpenRD Client.

For NIOS2, this patch contains all manual v1 and v2 fixes by Thomas.

For x86, this patch contains all manual v2 fixes by Bin.

Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"

Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function

Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error

 arch/arc/lib/start.S            |  12 +++--
 arch/arm/lib/crt0.S             |   6 ++-
 arch/arm/lib/crt0_64.S          |   6 ++-
 arch/microblaze/cpu/start.S     |   4 +-
 arch/nios2/cpu/start.S          |  13 +++--
 arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
 arch/x86/cpu/start.S            |   5 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c        | 108 +++++++++++++++++++++++++++++++++++-----
 include/common.h                |  33 +++++-------
 10 files changed, 144 insertions(+), 57 deletions(-)

arch/arc/lib/start.S +7 -5

index 26a5934..2a99193 100644
@@ -54,14 +54,16 @@ ENTRY(_start)
 	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
 	mov	%fp, %sp
 
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
-	mov	%sp, %r0
+	sub	%sp, %sp, %r0
 	mov	%fp, %sp
 
+	/* Initialize reserved area */
+	mov	%r0, %sp
+	bl	board_init_f_init_reserve
+
 	/* Zero the one and only argument of "board_init_f" */
 	mov_s	%r0, 0
 	j	board_init_f

arch/arm/lib/crt0.S +4 -2

index 80548eb..4ec89a4 100644
@@ -82,9 +82,11 @@ ENTRY(_main)
 #else
 	bic	sp, sp, #7	/* 8-byte alignment for ABI compliance */
 #endif
+	bl	board_init_f_get_reserve_size
+	sub	sp, sp, r0
+	mov	r9, sp
 	mov	r0, sp
-	bl	board_init_f_mem
-	mov	sp, r0
+	bl	board_init_f_init_reserve
 
 	mov	r0, #0
 	bl	board_init_f

arch/arm/lib/crt0_64.S +4 -2

index cef1c71..2891071 100644
@@ -75,8 +75,10 @@ ENTRY(_main)
 	ldr	x0, =(CONFIG_SYS_INIT_SP_ADDR)
 #endif
 	bic	sp, x0, #0xf	/* 16-byte alignment for ABI compliance */
-	bl	board_init_f_mem
-	mov	sp, x0
+	bl	board_init_f_get_reserve_size
+	sub	sp, sp, x0
+	mov	x0, sp
+	bl	board_init_f_init_reserve
 
 	mov	x0, #0
 	bl	board_init_f

arch/microblaze/cpu/start.S +2 -2

index 14f46a8..206be3e 100644
@@ -25,7 +25,7 @@ _start:
 
 	addi	r8, r0, __end
 	mts	rslr, r8
-	/* TODO: Redo this code to call board_init_f_mem() */
+	/* TODO: Redo this code to call board_init_f_*() */
 #if defined(CONFIG_SPL_BUILD)
 	addi	r1, r0, CONFIG_SPL_STACK_ADDR
 	mts	rshr, r1

@@ -142,7 +142,7 @@ _start:
 	ori	r12, r12, 0x1a0
 	mts	rmsr, r12
 
-	/* TODO: Redo this code to call board_init_f_mem() */
+	/* TODO: Redo this code to call board_init_f_*() */
 clear_bss:
 	/* clear BSS segments */
 	addi	r5, r0, __bss_start

arch/nios2/cpu/start.S +8 -5

index 54787c5..45bf0fd 100644
@@ -106,14 +106,17 @@ _reloc:
 	stw	r0, 4(sp)
 	mov	fp, sp
 
-	/* Allocate and zero GD, update SP */
+	/* Allocate and initialize reserved area, update SP */
+	movhi	r2, %hi(board_init_f_get_reserve_size at h)
+	ori	r2, r2, %lo(board_init_f_get_reserve_size at h)
+	callr	r2
+	sub	sp, sp, r2
 	mov	r4, sp
-	movhi	r2, %hi(board_init_f_mem at h)
-	ori	r2, r2, %lo(board_init_f_mem at h)
+	movhi	r2, %hi(board_init_f_init_reserve at h)
+	ori	r2, r2, %lo(board_init_f_init_reserve at h)
 	callr	r2
 
-	/* Update stack- and frame-pointers */
-	mov	sp, r2
+	/* Update frame-pointer */
 	mov	fp, sp
 
 	/* Call board_init_f -- never returns */

arch/powerpc/cpu/ppc4xx/start.S +6 -4

index 77d4040..44222d2 100644
@@ -761,9 +761,10 @@ _start:
 
 	bl	cpu_init_f	/* run low-level CPU init code	   (from Flash) */
 #ifdef CONFIG_SYS_GENERIC_BOARD
+	bl	board_init_f_get_reserve_size
+	sub	r1, r1, r3
 	mr	r3, r1
-	bl	board_init_f_mem
-	mr	r1, r3
+	bl	board_init_f_init_reserve
 	li	r0,0
 	stwu	r0, -4(r1)
 	stwu	r0, -4(r1)

@@ -1037,9 +1038,10 @@ _start:
 
 	bl	cpu_init_f	/* run low-level CPU init code	   (from Flash) */
 #ifdef CONFIG_SYS_GENERIC_BOARD
+	bl	board_init_f_get_reserve_size
+	sub	r1, r1, r3
 	mr	r3, r1
-	bl	board_init_f_mem
-	mr	r1, r3
+	bl	board_init_f_init_reserve
 	stwu	r0, -4(r1)
 	stwu	r0, -4(r1)
 #endif

arch/x86/cpu/start.S +3 -2

index 5b4ee79..932672c 100644
@@ -122,9 +122,10 @@ car_init_ret:
 	 */
 #endif
 	/* Set up global data */
+	call	board_init_f_get_reserve_size
+	subl	%eax, %esp
 	mov	%esp, %eax
-	call	board_init_f_mem
-	mov	%eax, %esp
+	call	board_init_f_init_reserve
 
 #ifdef CONFIG_DEBUG_UART
 	call	debug_uart_init

arch/x86/lib/fsp/fsp_common.c +2 -2

index c78df94..72c458f 100644
@@ -95,8 +95,8 @@ int x86_fsp_init(void)
 		/*
 		 * The second time we enter here, adjust the size of malloc()
 		 * pool before relocation. Given gd->malloc_base was adjusted
-		 * after the call to board_init_f_mem() in arch/x86/cpu/start.S,
-		 * we should fix up gd->malloc_limit here.
+		 * after the call to board_init_f_init_reserve() in arch/x86/
+		 * cpu/start.S, we should fix up gd->malloc_limit here.
 		 */
 		gd->malloc_limit += CONFIG_FSP_SYS_MALLOC_F_LEN;
 	}

common/init/board_init.c +95 ~~-13~~

134 lines not on this page · raw message

include/common.h +13 ~~-20~~

index 09a131d..988d67a 100644
@@ -225,32 +225,25 @@ void board_init_f(ulong);
 void board_init_r(gd_t *, ulong) __attribute__ ((noreturn));
 
 /**
- * board_init_f_mem() - Allocate global data and set stack position
+ * ulong board_init_f_get_reserve_size - return size of reserved area
  *
  * This function is called by each architecture very early in the start-up
- * code to set up the environment for board_init_f(). It allocates space for
- * global_data (see include/asm-generic/global_data.h) and places the stack
- * below this.
+ * code to allow the C runtime to reserve space on the stack for writable
+ * 'globals' such as GD and the malloc arena.
  *
- * This function requires a stack[1] Normally this is at @top. The function
- * starts allocating space from 64 bytes below @top. First it creates space
- * for global_data. Then it calls arch_setup_gd() which sets gd to point to
- * the global_data space and can reserve additional bytes of space if
- * required). Finally it allocates early malloc() memory
- * (CONFIG_SYS_MALLOC_F_LEN). The new top of the stack is just below this,
- * and it returned by this function.
+ * @return:	size of reserved area
+ */
+ulong board_init_f_get_reserve_size(void);
+
+/**
+ * board_init_f_init_reserve - initialize the reserved area(s)
  *
- * [1] Strictly speaking it would be possible to implement this function
- * in C on many archs such that it does not require a stack. However this
- * does not seem hugely important as only 64 byte are wasted. The 64 bytes
- * are used to handle the calling standard which generally requires pushing
- * addresses or registers onto the stack. We should be able to get away with
- * less if this becomes important.
+ * This function is called once the C runtime has allocated the reserved
+ * area on the stack. It must initialize the GD at the base of that area.
  *
- * @top:	Top of available memory, also normally the top of the stack
- * @return:	New stack location
+ * @base:	top from which reservation was done
  */
-ulong board_init_f_mem(ulong top);
+void board_init_f_init_reserve(ulong base);
 
 /**
  * arch_setup_gd() - Set up the global_data pointer

Signature · 1 line

2.5.0

Albert ARIBAUD

author

2015-11-15 18:31 UTC #1 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

Hello Albert,

Quoted 7 lines · Albert ARIBAUD

board_init_f_mem() alters the C runtime environment's stack it ls actually already using. This is not a valid C runtime environment.
Split board_init_f_mem into C functions which do not alter their own stack and therefore run in a valid C runtime environment.

Of course, I only see the typos above now: that paragraph should read:

board_init_f_mem() alters the C runtime environment's stack it is actually already using. This is not a valid behaviour within a C runtime environment.

Split board_init_f_mem into C functions which do not alter their own stack and always behave properly with respect to their C runtime environment.

If no comment calls for a v5, then whoever applies this v4 please fix the commit message accordingly.

Amicalement,

Signature · 1 line

Albert.

Alexey Brodkin

2015-11-16 13:12 UTC #2 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

In reply to Albert ARIBAUD

Hi Albert,

Quoted 56 lines · Albert ARIBAUD

board_init_f_mem() alters the C runtime environment's stack it ls actually already using. This is not a valid C runtime environment.
Split board_init_f_mem into C functions which do not alter their own stack and therefore run in a valid C runtime environment.
Signed-off-by Albert ARIBAUD <[email protected]>
---
Cc:ing custodians for all architectures which this patch affects.
This patch has been build-tested for all there architectures, and run-tested on ARM OpenRD Client.
For NIOS2, this patch contains all manual v1 and v2 fixes by Thomas.
For x86, this patch contains all manual v2 fixes by Bin.
Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"
Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function
Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error
 arch/arc/lib/start.S            |  12 +++--
 arch/arm/lib/crt0.S             |   6 ++-
 arch/arm/lib/crt0_64.S          |   6 ++-
 arch/microblaze/cpu/start.S     |   4 +-
 arch/nios2/cpu/start.S          |  13 +++--
 arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
 arch/x86/cpu/start.S            |   5 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c        | 108 +++++++++++++++++++++++++++++++++++-----
 include/common.h                |  33 +++++-------
 10 files changed, 144 insertions(+), 57 deletions(-)
arch/arc/lib/start.S +0 -0
index 26a5934..2a99193 100644

arch/arc/lib/start.S ENTRY(_start) · line 59

Quoted 10 lines

arch/arc/lib/start.S +2 -4

@@ -54,14 +54,16 @@ ENTRY(_start)
 	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
 	mov	%fp, %sp
 
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */

I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely:
---------->8----------

	/* Update stack- and frame-pointers */

---------->8----------

Not sure if this tiny nitpick worth respinning thought.

Otherwise build and run tested on free nSIM,
see "Running U-Boot on ARC in Free nSIM" section in
http://www.denx.de/wiki/U-Boot/ARCNotes if of any interest how to do it yourself.

Feel free to add
---------->8----------

Acked-by Alexey Brodkin <[email protected]>

---------->8----------

-Alexey

Albert ARIBAUD

author

2015-11-16 13:34 UTC #3 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

Hello Alexey,

Quoted 74 lines · Alexey Brodkin

Hi Albert,
Quoted 56 lines · Albert ARIBAUD
board_init_f_mem() alters the C runtime environment's stack it ls actually already using. This is not a valid C runtime environment.
Split board_init_f_mem into C functions which do not alter their own stack and therefore run in a valid C runtime environment.
Signed-off-by Albert ARIBAUD <[email protected]>
---
Cc:ing custodians for all architectures which this patch affects.
This patch has been build-tested for all there architectures, and run-tested on ARM OpenRD Client.
For NIOS2, this patch contains all manual v1 and v2 fixes by Thomas.
For x86, this patch contains all manual v2 fixes by Bin.
Changes in v4:
- Add comments on reserving GD at the lowest location
- Add comments on post-incrementing base for each "chunk"
Changes in v3:
- Rebase after commit 9ac4fc82
- Simplify malloc conditional as per commit 9ac4fc82
- Fix NIOS2 return value register (r2, not r4)
- Fix x86 subl argument inversion
- Group allocations in a single function
- Group initializations in a single function
Changes in v2:
- Fix all checkpatch issues
- Fix board_init_f_malloc prototype mismatch
- Fix board_init_[f_]xxx typo in NIOS2
- Fix aarch64 asm 'sub' syntax error
 arch/arc/lib/start.S            |  12 +++--
 arch/arm/lib/crt0.S             |   6 ++-
 arch/arm/lib/crt0_64.S          |   6 ++-
 arch/microblaze/cpu/start.S     |   4 +-
 arch/nios2/cpu/start.S          |  13 +++--
 arch/powerpc/cpu/ppc4xx/start.S |  10 ++--
 arch/x86/cpu/start.S            |   5 +-
 arch/x86/lib/fsp/fsp_common.c   |   4 +-
 common/init/board_init.c        | 108 +++++++++++++++++++++++++++++++++++-----
 include/common.h                |  33 +++++-------
 10 files changed, 144 insertions(+), 57 deletions(-)
arch/arc/lib/start.S +0 -0
index 26a5934..2a99193 100644
arch/arc/lib/start.S ENTRY(_start) · line 59
Quoted 10 lines
arch/arc/lib/start.S +2 -4
@@ -54,14 +54,16 @@ ENTRY(_start)
 	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
 	mov	%fp, %sp
 
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely:
---------->8----------

Quoted 2 lines

	/* Update stack- and frame-pointers */
---------->8----------

Not sure where you see two SP+FP 'update' comments here; probably you're referring to the 'setup' comment on line 53 and the 'update' one on line 59. If that is what you meant, I tink these comments are different and deserve staying both...

... However, these comments also pretty much just paraphrase the code which follows them and thus serve little purpose; they could be reworded to show less of what is being done and more of why it is being done:

- the "Update stack- and frame-pointer" comment could be turned into "Allocate reserved size on stack and adjust frame pointer accordingly", and

- the "Setup stack- and frame-pointers" comment could be turned into "Establish C runtime stack and frame".

Opinions?

Quoted 1 line

Not sure if this tiny nitpick worth respinning thought.

That's fine, since I've got the commit message to fix too.

Quoted 4 lines

Otherwise build and run tested on free nSIM,
see "Running U-Boot on ARC in Free nSIM" section in
http://www.denx.de/wiki/U-Boot/ARCNotes if of any interest how to do it yourself.

Thanks, I'll try this, and I4ll certainly keep it in my bookmarks.

Quoted 6 lines

Feel free to add
---------->8----------
Acked-by Alexey Brodkin <[email protected]>
---------->8----------
-Alexey

Amicalement,

Signature · 1 line

Albert.

Alexey Brodkin

2015-11-16 13:43 UTC #4 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

Hi Albert,

Quoted 22 lines · Albert ARIBAUD

Hello Alexey,
Quoted 15 lines · Alexey Brodkin
Hi Albert,
arch/arc/lib/start.S ENTRY(_start) · line 59
Quoted 8 lines
arch/arc/lib/start.S +2 -4
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely:
---------->8----------
Quoted 2 lines
	/* Update stack- and frame-pointers */
---------->8----------

Quoted 4 lines

Not sure where you see two SP+FP 'update' comments here; probably you're referring to the 'setup' comment on line 53 and the 'update' one on line 59. If that is what you meant, I tink these comments are different and deserve staying both...

Ok, that's what I have after your patch application:

---------->8----------

	/* Setup stack- and frame-pointers */
	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
	mov	%fp, %sp

	/* Get reserved area size, update SP and FP */
	bl	board_init_f_get_reserve_size
	/* Update stack- and frame-pointers */  <-- that's already mentioned 2 lines above
	sub	%sp, %sp, %r0
	mov	%fp, %sp

---------->8----------

Quoted 5 lines

... However, these comments also pretty much just paraphrase the code which follows them and thus serve little purpose; they could be reworded to show less of what is being done and more of why it is being done:

Quoted 8 lines

- the "Update stack- and frame-pointer" comment could be turned into "Allocate reserved size on stack and adjust frame pointer accordingly", and
- the "Setup stack- and frame-pointers" comment could be turned into "Establish C runtime stack and frame".
Opinions?

Totally agree, care to implement it?

-Alexey

Albert ARIBAUD

author

2015-11-16 14:15 UTC #5 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

Hello Alexey,

Quoted 42 lines · Alexey Brodkin

Hi Albert,
Quoted 22 lines · Albert ARIBAUD
Hello Alexey,
Quoted 15 lines · Alexey Brodkin
Hi Albert,
arch/arc/lib/start.S ENTRY(_start) · line 59
Quoted 8 lines
arch/arc/lib/start.S +2 -4
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely:
---------->8----------
Quoted 2 lines
	/* Update stack- and frame-pointers */
---------->8----------
Quoted 4 lines
Not sure where you see two SP+FP 'update' comments here; probably you're referring to the 'setup' comment on line 53 and the 'update' one on line 59. If that is what you meant, I tink these comments are different and deserve staying both...
Ok, that's what I have after your patch application:
---------->8----------
	/* Setup stack- and frame-pointers */
	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
	mov	%fp, %sp

	/* Get reserved area size, update SP and FP */
	bl	board_init_f_get_reserve_size
	/* Update stack- and frame-pointers */  <-- that's already mentioned 2 lines above
	sub	%sp, %sp, %r0
	mov	%fp, %sp

Quoted 1 line

---------->8----------

My bad, I'd missed that one. I'll turn

/* Get reserved area size, update SP and FP */

Into

/* Get reserved area size */

Quoted 9 lines

Quoted 9 lines
... However, these comments also pretty much just paraphrase the code which follows them and thus serve little purpose; they could be reworded to show less of what is being done and more of why it is being done:
- the "Update stack- and frame-pointer" comment could be turned into "Allocate reserved size on stack and adjust frame pointer accordingly", and

Quoted 6 lines

Quoted 4 lines
- the "Setup stack- and frame-pointers" comment could be turned into "Establish C runtime stack and frame".
Opinions?
Totally agree, care to implement it?

That, and the removal of the repetition. v5 in approach.

Quoted 1 line

-Alexey

Amicalement,

Signature · 1 line

Albert.

Alexey Brodkin

2015-11-16 14:22 UTC #6 lore raw

PATCH v4 1/1 Fix board init code to use a valid C runtime environment

Hi Albert,

Quoted 70 lines · Albert ARIBAUD

Hello Alexey,
Quoted 42 lines · Alexey Brodkin
Hi Albert,
Quoted 22 lines · Albert ARIBAUD
Hello Alexey,
Quoted 15 lines · Alexey Brodkin
Hi Albert,
arch/arc/lib/start.S ENTRY(_start) · line 59
Quoted 8 lines
arch/arc/lib/start.S +2 -4
-	/* Allocate and zero GD, update SP */
-	mov	%r0, %sp
-	bl	board_init_f_mem
-
+	/* Get reserved area size, update SP and FP */
+	bl	board_init_f_get_reserve_size
 	/* Update stack- and frame-pointers */
I think we don't need to mention SP/FP update in comments twice here. I.e. either strip ", update SP and FP" from your introduced comment or which I really like more remove following line with comment entirely:
---------->8----------
Quoted 2 lines
	/* Update stack- and frame-pointers */
---------->8----------
Quoted 4 lines
Not sure where you see two SP+FP 'update' comments here; probably you're referring to the 'setup' comment on line 53 and the 'update' one on line 59. If that is what you meant, I tink these comments are different and deserve staying both...
Ok, that's what I have after your patch application:
---------->8----------
	/* Setup stack- and frame-pointers */
	mov	%sp, CONFIG_SYS_INIT_SP_ADDR
	mov	%fp, %sp

	/* Get reserved area size, update SP and FP */
	bl	board_init_f_get_reserve_size
	/* Update stack- and frame-pointers */  <-- that's already mentioned 2 lines above
	sub	%sp, %sp, %r0
	mov	%fp, %sp
Quoted 1 line
---------->8----------
My bad, I'd missed that one. I'll turn
/* Get reserved area size, update SP and FP */
Into
/* Get reserved area size */
Quoted 14 lines
Quoted 13 lines
... However, these comments also pretty much just paraphrase the code which follows them and thus serve little purpose; they could be reworded to show less of what is being done and more of why it is being done:
- the "Update stack- and frame-pointer" comment could be turned into "Allocate reserved size on stack and adjust frame pointer accordingly", and
- the "Setup stack- and frame-pointers" comment could be turned into "Establish C runtime stack and frame".
Opinions?

Quoted 7 lines

Quoted 1 line
Totally agree, care to implement it?
That, and the removal of the repetition. v5 in approach.
Quoted 1 line
-Alexey
Amicalement,

Thanks for doing that!

-Alexey