public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] arm1176: fix relocation
@ 2010-10-24 20:08 Darius Augulis
  2010-10-24 20:22 ` Darius Augulis
  2010-10-25  6:17 ` Heiko Schocher
  0 siblings, 2 replies; 5+ messages in thread
From: Darius Augulis @ 2010-10-24 20:08 UTC (permalink / raw)
  To: u-boot

Fix relocation code for arm1176, do it like other ARM
CPU's are doing.
Tested only with CONFIG_SKIP_RELOCATE_UBOOT defined
and using nand_spl (booting from nand). Test done on
s3c6410 based board (not yet supported in main line).

Signed-off-by: Darius Augulis <augulis.darius@gmail.com>
---
 arch/arm/cpu/arm1176/start.S    |  139 +++++++++++++++++++++++----------------
 arch/arm/cpu/arm1176/u-boot.lds |   15 +++-
 2 files changed, 94 insertions(+), 60 deletions(-)

diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
index 24e5bf4..0d733f7 100644
--- a/arch/arm/cpu/arm1176/start.S
+++ b/arch/arm/cpu/arm1176/start.S
@@ -115,44 +115,52 @@ _armboot_start:
 
 /*
  * These are defined in the board-specific linker script.
+ * Subtracting _start from them lets the linker put their
+ * relative position in the executable instead of leaving
+ * them null.
  */
-.globl _bss_start
-_bss_start:
-	.word __bss_start
 
-.globl _bss_end
-_bss_end:
-	.word _end
+.globl _bss_start_ofs
+_bss_start_ofs:
+	.word __bss_start - _start
 
-#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
-/* IRQ stack memory (calculated at run-time) + 8 bytes */
-.globl IRQ_STACK_START_IN
-IRQ_STACK_START_IN:
-	.word	0x0badc0de
+.globl _bss_end_ofs
+_bss_end_ofs:
+	.word _end - _start
+
+.globl _datarel_start_ofs
+_datarel_start_ofs:
+	.word __datarel_start - _start
 
-.globl _datarel_start
-_datarel_start:
-	.word __datarel_start
+.globl _datarelrolocal_start_ofs
+_datarelrolocal_start_ofs:
+	.word __datarelrolocal_start - _start
 
-.globl _datarelrolocal_start
-_datarelrolocal_start:
-	.word __datarelrolocal_start
+.globl _datarellocal_start_ofs
+_datarellocal_start_ofs:
+	.word __datarellocal_start - _start
 
-.globl _datarellocal_start
-_datarellocal_start:
-	.word __datarellocal_start
+.globl _datarelro_start_ofs
+_datarelro_start_ofs:
+	.word __datarelro_start - _start
 
-.globl _datarelro_start
-_datarelro_start:
-	.word __datarelro_start
+.globl _rel_dyn_start_ofs
+_rel_dyn_start_ofs:
+	.word __rel_dyn_start - _start
 
-.globl _got_start
-_got_start:
-	.word __got_start
+.globl _rel_dyn_end_ofs
+_rel_dyn_end_ofs:
+	.word __rel_dyn_end - _start
 
-.globl _got_end
-_got_end:
-	.word __got_end
+.globl _dynsym_start_ofs
+_dynsym_start_ofs:
+	.word __dynsym_start - _start
+
+#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
+/* IRQ stack memory (calculated at run-time) + 8 bytes */
+.globl IRQ_STACK_START_IN
+IRQ_STACK_START_IN:
+	.word	0x0badc0de
 
 /*
  * the actual reset code
@@ -274,9 +282,8 @@ stack_setup:
 
 	adr	r0, _start
 	ldr	r2, _TEXT_BASE
-	ldr	r3, _bss_start
-	sub	r2, r3, r2		/* r2 <- size of armboot	    */
-	add	r2, r0, r2		/* r2 <- source end address	    */
+	ldr	r3, _bss_start_ofs
+	add	r2, r0, r3		/* r2 <- source end address	    */
 	cmp	r0, r6
 	beq	clear_bss
 
@@ -288,24 +295,44 @@ copy_loop:
 	blo	copy_loop
 
 #ifndef CONFIG_PRELOADER
-	/* fix got entries */
-	ldr	r1, _TEXT_BASE		/* Text base */
-	mov	r0, r7			/* reloc addr */
-	ldr	r2, _got_start		/* addr in Flash */
-	ldr	r3, _got_end		/* addr in Flash */
-	sub	r3, r3, r1
-	add	r3, r3, r0
-	sub	r2, r2, r1
-	add	r2, r2, r0
-
+	/*
+	 * fix .rel.dyn relocations
+	 */
+	ldr	r0, _TEXT_BASE		/* r0 <- Text base */
+	sub	r9, r7, r0		/* r9 <- relocation offset */
+	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table ofs */
+	add	r10, r10, r0		/* r10 <- sym table in FLASH */
+	ldr	r2, _rel_dyn_start_ofs	/* r2 <- rel dyn start ofs */
+	add	r2, r2, r0		/* r2 <- rel dyn start in FLASH */
+	ldr	r3, _rel_dyn_end_ofs	/* r3 <- rel dyn end ofs */
+	add	r3, r3, r0		/* r3 <- rel dyn end in FLASH */
 fixloop:
-	ldr	r4, [r2]
-	sub	r4, r4, r1
-	add	r4, r4, r0
-	str	r4, [r2]
-	add	r2, r2, #4
+	ldr	r0, [r2]	/* r0 <- location to fix up, IN FLASH! */
+	add	r0, r9		/* r0 <- location to fix up in RAM */
+	ldr	r1, [r2, #4]
+	and	r8, r1, #0xff
+	cmp	r8, #23		/* relative fixup? */
+	beq	fixrel
+	cmp	r8, #2		/* absolute fixup? */
+	beq	fixabs
+	/* ignore unknown type of fixup */
+	b	fixnext
+fixabs:
+	/* absolute fix: set location to (offset) symbol value */
+	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
+	add	r1, r10, r1		/* r1 <- address of symbol in table */
+	ldr	r1, [r1, #4]		/* r1 <- symbol value */
+	add	r1, r9			/* r1 <- relocated sym addr */
+	b	fixnext
+fixrel:
+	/* relative fix: increase location by offset */
+	ldr	r1, [r0]
+	add	r1, r1, r9
+fixnext:
+	str	r1, [r0]
+	add	r2, r2, #8	/* each rel.dyn entry is 8 bytes */
 	cmp	r2, r3
-	bne	fixloop
+	ble	fixloop
 #endif
 #endif	/* #ifndef CONFIG_SKIP_RELOCATE_UBOOT */
 
@@ -349,13 +376,11 @@ skip_hw_init:
 
 clear_bss:
 #ifndef CONFIG_PRELOADER
-	ldr	r0, _bss_start
-	ldr	r1, _bss_end
+	ldr	r0, _bss_start_ofs
+	ldr	r1, _bss_end_ofs
 	ldr	r3, _TEXT_BASE		/* Text base */
 	mov	r4, r7			/* reloc addr */
-	sub	r0, r0, r3
 	add	r0, r0, r4
-	sub	r1, r1, r3
 	add	r1, r1, r4
 	mov	r2, #0x00000000		/* clear			    */
 
@@ -377,10 +402,9 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
 
 _nand_boot: .word nand_boot
 #else
-	ldr	r0, _TEXT_BASE
-	ldr	r2, _board_init_r
-	sub	r2, r2, r0
-	add	r2, r2, r7	/* position from board_init_r in RAM */
+	ldr	r0, _board_init_r_ofs
+	adr	r1, _start
+	add	r2, r0, r1
 	/* setup parameters for board_init_r */
 	mov	r0, r5		/* gd_t */
 	mov	r1, r7		/* dest_addr */
@@ -388,7 +412,8 @@ _nand_boot: .word nand_boot
 	mov	lr, r2
 	mov	pc, lr
 
-_board_init_r: .word board_init_r
+_board_init_r_ofs:
+	.word board_init_r - _start
 #endif
 
 #else /* #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) */
diff --git a/arch/arm/cpu/arm1176/u-boot.lds b/arch/arm/cpu/arm1176/u-boot.lds
index fa640ee..d9ed954 100644
--- a/arch/arm/cpu/arm1176/u-boot.lds
+++ b/arch/arm/cpu/arm1176/u-boot.lds
@@ -51,11 +51,14 @@ SECTIONS
 		*(.data.rel.ro)
 	}
 
-	__got_start = .;
 	. = ALIGN(4);
-	.got : { *(.got) }
+	__rel_dyn_start = .;
+	.rel.dyn : { *(.rel.dyn) }
+	__rel_dyn_end = .;
+
+	__dynsym_start = .;
+	.dynsym : { *(.dynsym) }
 
-	__got_end = .;
 	. = .;
 	__u_boot_cmd_start = .;
 	.u_boot_cmd : { *(.u_boot_cmd) }
@@ -65,4 +68,10 @@ SECTIONS
 	__bss_start = .;
 	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
 	_end = .;
+
+	/DISCARD/ : { *(.dynstr*) }
+	/DISCARD/ : { *(.dynamic*) }
+	/DISCARD/ : { *(.plt*) }
+	/DISCARD/ : { *(.interp*) }
+	/DISCARD/ : { *(.gnu*) }
 }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] arm1176: fix relocation
  2010-10-24 20:08 [U-Boot] [PATCH] arm1176: fix relocation Darius Augulis
@ 2010-10-24 20:22 ` Darius Augulis
  2010-10-25  7:11   ` Heiko Schocher
  2010-10-25  6:17 ` Heiko Schocher
  1 sibling, 1 reply; 5+ messages in thread
From: Darius Augulis @ 2010-10-24 20:22 UTC (permalink / raw)
  To: u-boot

On 10/24/2010 11:08 PM, Darius Augulis wrote:
> Fix relocation code for arm1176, do it like other ARM
> CPU's are doing.
> Tested only with CONFIG_SKIP_RELOCATE_UBOOT defined
> and using nand_spl (booting from nand). Test done on
> s3c6410 based board (not yet supported in main line).
>
> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
> ---
>   arch/arm/cpu/arm1176/start.S    |  139 +++++++++++++++++++++++----------------
>   arch/arm/cpu/arm1176/u-boot.lds |   15 +++-
>   2 files changed, 94 insertions(+), 60 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
> index 24e5bf4..0d733f7 100644
> --- a/arch/arm/cpu/arm1176/start.S
> +++ b/arch/arm/cpu/arm1176/start.S
> @@ -115,44 +115,52 @@ _armboot_start:
>
>   /*
>    * These are defined in the board-specific linker script.
> + * Subtracting _start from them lets the linker put their
> + * relative position in the executable instead of leaving
> + * them null.
>    */
> -.globl _bss_start
> -_bss_start:
> -	.word __bss_start
>
> -.globl _bss_end
> -_bss_end:
> -	.word _end
> +.globl _bss_start_ofs
> +_bss_start_ofs:
> +	.word __bss_start - _start
>
> -#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
> -/* IRQ stack memory (calculated at run-time) + 8 bytes */
> -.globl IRQ_STACK_START_IN
> -IRQ_STACK_START_IN:
> -	.word	0x0badc0de
> +.globl _bss_end_ofs
> +_bss_end_ofs:
> +	.word _end - _start
> +
> +.globl _datarel_start_ofs
> +_datarel_start_ofs:
> +	.word __datarel_start - _start
>
> -.globl _datarel_start
> -_datarel_start:
> -	.word __datarel_start
> +.globl _datarelrolocal_start_ofs
> +_datarelrolocal_start_ofs:
> +	.word __datarelrolocal_start - _start
>
> -.globl _datarelrolocal_start
> -_datarelrolocal_start:
> -	.word __datarelrolocal_start
> +.globl _datarellocal_start_ofs
> +_datarellocal_start_ofs:
> +	.word __datarellocal_start - _start
>
> -.globl _datarellocal_start
> -_datarellocal_start:
> -	.word __datarellocal_start
> +.globl _datarelro_start_ofs
> +_datarelro_start_ofs:
> +	.word __datarelro_start - _start
>
> -.globl _datarelro_start
> -_datarelro_start:
> -	.word __datarelro_start
> +.globl _rel_dyn_start_ofs
> +_rel_dyn_start_ofs:
> +	.word __rel_dyn_start - _start
>
> -.globl _got_start
> -_got_start:
> -	.word __got_start
> +.globl _rel_dyn_end_ofs
> +_rel_dyn_end_ofs:
> +	.word __rel_dyn_end - _start
>
> -.globl _got_end
> -_got_end:
> -	.word __got_end
> +.globl _dynsym_start_ofs
> +_dynsym_start_ofs:
> +	.word __dynsym_start - _start
> +
> +#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
> +/* IRQ stack memory (calculated at run-time) + 8 bytes */
> +.globl IRQ_STACK_START_IN
> +IRQ_STACK_START_IN:
> +	.word	0x0badc0de
>
>   /*
>    * the actual reset code
> @@ -274,9 +282,8 @@ stack_setup:
>
>   	adr	r0, _start
>   	ldr	r2, _TEXT_BASE
> -	ldr	r3, _bss_start
> -	sub	r2, r3, r2		/* r2<- size of armboot	    */
> -	add	r2, r0, r2		/* r2<- source end address	    */
> +	ldr	r3, _bss_start_ofs
> +	add	r2, r0, r3		/* r2<- source end address	    */
>   	cmp	r0, r6
>   	beq	clear_bss
>
> @@ -288,24 +295,44 @@ copy_loop:
>   	blo	copy_loop
>
>   #ifndef CONFIG_PRELOADER
> -	/* fix got entries */
> -	ldr	r1, _TEXT_BASE		/* Text base */
> -	mov	r0, r7			/* reloc addr */
> -	ldr	r2, _got_start		/* addr in Flash */
> -	ldr	r3, _got_end		/* addr in Flash */
> -	sub	r3, r3, r1
> -	add	r3, r3, r0
> -	sub	r2, r2, r1
> -	add	r2, r2, r0
> -
> +	/*
> +	 * fix .rel.dyn relocations
> +	 */
> +	ldr	r0, _TEXT_BASE		/* r0<- Text base */
> +	sub	r9, r7, r0		/* r9<- relocation offset */
> +	ldr	r10, _dynsym_start_ofs	/* r10<- sym table ofs */
> +	add	r10, r10, r0		/* r10<- sym table in FLASH */
> +	ldr	r2, _rel_dyn_start_ofs	/* r2<- rel dyn start ofs */
> +	add	r2, r2, r0		/* r2<- rel dyn start in FLASH */
> +	ldr	r3, _rel_dyn_end_ofs	/* r3<- rel dyn end ofs */
> +	add	r3, r3, r0		/* r3<- rel dyn end in FLASH */
>   fixloop:
> -	ldr	r4, [r2]
> -	sub	r4, r4, r1
> -	add	r4, r4, r0
> -	str	r4, [r2]
> -	add	r2, r2, #4
> +	ldr	r0, [r2]	/* r0<- location to fix up, IN FLASH! */
> +	add	r0, r9		/* r0<- location to fix up in RAM */
> +	ldr	r1, [r2, #4]
> +	and	r8, r1, #0xff
> +	cmp	r8, #23		/* relative fixup? */
> +	beq	fixrel
> +	cmp	r8, #2		/* absolute fixup? */
> +	beq	fixabs
> +	/* ignore unknown type of fixup */
> +	b	fixnext
> +fixabs:
> +	/* absolute fix: set location to (offset) symbol value */
> +	mov	r1, r1, LSR #4		/* r1<- symbol index in .dynsym */
> +	add	r1, r10, r1		/* r1<- address of symbol in table */
> +	ldr	r1, [r1, #4]		/* r1<- symbol value */
> +	add	r1, r9			/* r1<- relocated sym addr */
> +	b	fixnext
> +fixrel:
> +	/* relative fix: increase location by offset */
> +	ldr	r1, [r0]
> +	add	r1, r1, r9
> +fixnext:
> +	str	r1, [r0]
> +	add	r2, r2, #8	/* each rel.dyn entry is 8 bytes */
>   	cmp	r2, r3
> -	bne	fixloop
> +	ble	fixloop
>   #endif
>   #endif	/* #ifndef CONFIG_SKIP_RELOCATE_UBOOT */
>
> @@ -349,13 +376,11 @@ skip_hw_init:
>
>   clear_bss:
>   #ifndef CONFIG_PRELOADER
> -	ldr	r0, _bss_start
> -	ldr	r1, _bss_end
> +	ldr	r0, _bss_start_ofs
> +	ldr	r1, _bss_end_ofs
>   	ldr	r3, _TEXT_BASE		/* Text base */
>   	mov	r4, r7			/* reloc addr */
> -	sub	r0, r0, r3
>   	add	r0, r0, r4
> -	sub	r1, r1, r3
>   	add	r1, r1, r4
>   	mov	r2, #0x00000000		/* clear			    */
>
> @@ -377,10 +402,9 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
>
>   _nand_boot: .word nand_boot
>   #else
> -	ldr	r0, _TEXT_BASE
> -	ldr	r2, _board_init_r
> -	sub	r2, r2, r0
> -	add	r2, r2, r7	/* position from board_init_r in RAM */
> +	ldr	r0, _board_init_r_ofs
> +	adr	r1, _start
> +	add	r2, r0, r1

RFC:
I've observed that other ARM CPU's are adding r9 to lr here (add     lr, 
r0, r9). Even though r9 is not pre-loaded with any value in case 
CONFIG_SKIP_RELOCATE_UBOOT is defined. Is it bug or not?
Maybe it's not tested without CONFIG_SKIP_RELOCATE_UBOOT?
I've removed this line because uboot stops booting if it's there.
Any comments about it?

>   	/* setup parameters for board_init_r */
>   	mov	r0, r5		/* gd_t */
>   	mov	r1, r7		/* dest_addr */
> @@ -388,7 +412,8 @@ _nand_boot: .word nand_boot
>   	mov	lr, r2
>   	mov	pc, lr
>
> -_board_init_r: .word board_init_r
> +_board_init_r_ofs:
> +	.word board_init_r - _start
>   #endif
>
>   #else /* #if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC) */
> diff --git a/arch/arm/cpu/arm1176/u-boot.lds b/arch/arm/cpu/arm1176/u-boot.lds
> index fa640ee..d9ed954 100644
> --- a/arch/arm/cpu/arm1176/u-boot.lds
> +++ b/arch/arm/cpu/arm1176/u-boot.lds
> @@ -51,11 +51,14 @@ SECTIONS
>   		*(.data.rel.ro)
>   	}
>
> -	__got_start = .;
>   	. = ALIGN(4);
> -	.got : { *(.got) }
> +	__rel_dyn_start = .;
> +	.rel.dyn : { *(.rel.dyn) }
> +	__rel_dyn_end = .;
> +
> +	__dynsym_start = .;
> +	.dynsym : { *(.dynsym) }
>
> -	__got_end = .;
>   	. = .;
>   	__u_boot_cmd_start = .;
>   	.u_boot_cmd : { *(.u_boot_cmd) }
> @@ -65,4 +68,10 @@ SECTIONS
>   	__bss_start = .;
>   	.bss (NOLOAD) : { *(.bss) . = ALIGN(4); }
>   	_end = .;
> +
> +	/DISCARD/ : { *(.dynstr*) }
> +	/DISCARD/ : { *(.dynamic*) }
> +	/DISCARD/ : { *(.plt*) }
> +	/DISCARD/ : { *(.interp*) }
> +	/DISCARD/ : { *(.gnu*) }
>   }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] arm1176: fix relocation
  2010-10-24 20:08 [U-Boot] [PATCH] arm1176: fix relocation Darius Augulis
  2010-10-24 20:22 ` Darius Augulis
@ 2010-10-25  6:17 ` Heiko Schocher
  1 sibling, 0 replies; 5+ messages in thread
From: Heiko Schocher @ 2010-10-25  6:17 UTC (permalink / raw)
  To: u-boot

Hello Darius,

Darius Augulis wrote:
> Fix relocation code for arm1176, do it like other ARM
> CPU's are doing.
> Tested only with CONFIG_SKIP_RELOCATE_UBOOT defined
> and using nand_spl (booting from nand). Test done on
> s3c6410 based board (not yet supported in main line).
> 
> Signed-off-by: Darius Augulis <augulis.darius@gmail.com>
> ---
>  arch/arm/cpu/arm1176/start.S    |  139 +++++++++++++++++++++++----------------
>  arch/arm/cpu/arm1176/u-boot.lds |   15 +++-
>  2 files changed, 94 insertions(+), 60 deletions(-)

Thanks, some comments below:

> diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
> index 24e5bf4..0d733f7 100644
> --- a/arch/arm/cpu/arm1176/start.S
> +++ b/arch/arm/cpu/arm1176/start.S
[...]
> @@ -288,24 +295,44 @@ copy_loop:
>  	blo	copy_loop
>  
>  #ifndef CONFIG_PRELOADER
> -	/* fix got entries */
> -	ldr	r1, _TEXT_BASE		/* Text base */
> -	mov	r0, r7			/* reloc addr */
> -	ldr	r2, _got_start		/* addr in Flash */
> -	ldr	r3, _got_end		/* addr in Flash */
> -	sub	r3, r3, r1
> -	add	r3, r3, r0
> -	sub	r2, r2, r1
> -	add	r2, r2, r0
> -
> +	/*
> +	 * fix .rel.dyn relocations
> +	 */
> +	ldr	r0, _TEXT_BASE		/* r0 <- Text base */
> +	sub	r9, r7, r0		/* r9 <- relocation offset */
> +	ldr	r10, _dynsym_start_ofs	/* r10 <- sym table ofs */
> +	add	r10, r10, r0		/* r10 <- sym table in FLASH */
> +	ldr	r2, _rel_dyn_start_ofs	/* r2 <- rel dyn start ofs */
> +	add	r2, r2, r0		/* r2 <- rel dyn start in FLASH */
> +	ldr	r3, _rel_dyn_end_ofs	/* r3 <- rel dyn end ofs */
> +	add	r3, r3, r0		/* r3 <- rel dyn end in FLASH */
>  fixloop:
> -	ldr	r4, [r2]
> -	sub	r4, r4, r1
> -	add	r4, r4, r0
> -	str	r4, [r2]
> -	add	r2, r2, #4
> +	ldr	r0, [r2]	/* r0 <- location to fix up, IN FLASH! */
> +	add	r0, r9		/* r0 <- location to fix up in RAM */

should be

add	r0, r0, r9

> +	ldr	r1, [r2, #4]
> +	and	r8, r1, #0xff
> +	cmp	r8, #23		/* relative fixup? */
> +	beq	fixrel
> +	cmp	r8, #2		/* absolute fixup? */
> +	beq	fixabs
> +	/* ignore unknown type of fixup */
> +	b	fixnext
> +fixabs:
> +	/* absolute fix: set location to (offset) symbol value */
> +	mov	r1, r1, LSR #4		/* r1 <- symbol index in .dynsym */
> +	add	r1, r10, r1		/* r1 <- address of symbol in table */
> +	ldr	r1, [r1, #4]		/* r1 <- symbol value */
> +	add	r1, r9			/* r1 <- relocated sym addr */

should be

add r1, r1, r9

> +	b	fixnext
> +fixrel:
> +	/* relative fix: increase location by offset */
> +	ldr	r1, [r0]
> +	add	r1, r1, r9
> +fixnext:
> +	str	r1, [r0]
> +	add	r2, r2, #8	/* each rel.dyn entry is 8 bytes */
>  	cmp	r2, r3
> -	bne	fixloop
> +	ble	fixloop

should be blo instead of blo

>  #endif
>  #endif	/* #ifndef CONFIG_SKIP_RELOCATE_UBOOT */
>  
[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] arm1176: fix relocation
  2010-10-24 20:22 ` Darius Augulis
@ 2010-10-25  7:11   ` Heiko Schocher
  2010-10-25  7:47     ` Darius Augulis
  0 siblings, 1 reply; 5+ messages in thread
From: Heiko Schocher @ 2010-10-25  7:11 UTC (permalink / raw)
  To: u-boot

Hello Darius,

Darius Augulis wrote:
> On 10/24/2010 11:08 PM, Darius Augulis wrote:
>> Fix relocation code for arm1176, do it like other ARM
>> CPU's are doing.
>> Tested only with CONFIG_SKIP_RELOCATE_UBOOT defined
>> and using nand_spl (booting from nand). Test done on
>> s3c6410 based board (not yet supported in main line).
>>
>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>> ---
>>   arch/arm/cpu/arm1176/start.S    |  139 +++++++++++++++++++++++----------------
>>   arch/arm/cpu/arm1176/u-boot.lds |   15 +++-
>>   2 files changed, 94 insertions(+), 60 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
>> index 24e5bf4..0d733f7 100644
>> --- a/arch/arm/cpu/arm1176/start.S
>> +++ b/arch/arm/cpu/arm1176/start.S
[...]
>> @@ -377,10 +402,9 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
>>
>>   _nand_boot: .word nand_boot
>>   #else
>> -	ldr	r0, _TEXT_BASE
>> -	ldr	r2, _board_init_r
>> -	sub	r2, r2, r0
>> -	add	r2, r2, r7	/* position from board_init_r in RAM */
>> +	ldr	r0, _board_init_r_ofs
>> +	adr	r1, _start
>> +	add	r2, r0, r1
> 
> RFC:
> I've observed that other ARM CPU's are adding r9 to lr here (add     lr, 
> r0, r9). Even though r9 is not pre-loaded with any value in case 
> CONFIG_SKIP_RELOCATE_UBOOT is defined. Is it bug or not?
> Maybe it's not tested without CONFIG_SKIP_RELOCATE_UBOOT?
> I've removed this line because uboot stops booting if it's there.
> Any comments about it?

Yep, thats a Bug, thanks for detecting this.

But your fix seems to me, is not working for the case if
CONFIG_SKIP_RELOCATE_UBOOT is !defined, because then you need this
update of the lr register ... so could you please add this line
with a "#if !defined(CONFIG_SKIP_RELOCATE_UBOOT)" ... "#endif"
around it? (And maybe fix all other arm start.S files too?)

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [U-Boot] [PATCH] arm1176: fix relocation
  2010-10-25  7:11   ` Heiko Schocher
@ 2010-10-25  7:47     ` Darius Augulis
  0 siblings, 0 replies; 5+ messages in thread
From: Darius Augulis @ 2010-10-25  7:47 UTC (permalink / raw)
  To: u-boot

Hello Heiko,

thanks for reviewing. ok, I will make patch to fix this bug in all arm*/start.S.

Darius.

On Mon, Oct 25, 2010 at 10:11 AM, Heiko Schocher <hs@denx.de> wrote:
> Hello Darius,
>
> Darius Augulis wrote:
>> On 10/24/2010 11:08 PM, Darius Augulis wrote:
>>> Fix relocation code for arm1176, do it like other ARM
>>> CPU's are doing.
>>> Tested only with CONFIG_SKIP_RELOCATE_UBOOT defined
>>> and using nand_spl (booting from nand). Test done on
>>> s3c6410 based board (not yet supported in main line).
>>>
>>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>>> ---
>>> ? arch/arm/cpu/arm1176/start.S ? ?| ?139 +++++++++++++++++++++++----------------
>>> ? arch/arm/cpu/arm1176/u-boot.lds | ? 15 +++-
>>> ? 2 files changed, 94 insertions(+), 60 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/arm1176/start.S b/arch/arm/cpu/arm1176/start.S
>>> index 24e5bf4..0d733f7 100644
>>> --- a/arch/arm/cpu/arm1176/start.S
>>> +++ b/arch/arm/cpu/arm1176/start.S
> [...]
>>> @@ -377,10 +402,9 @@ clbss_l:str ? ? r2, [r0] ? ? ? ? ? ? ? ?/* clear loop... ? ? ? ? ? ? ? ? ? ?*/
>>>
>>> ? _nand_boot: .word nand_boot
>>> ? #else
>>> - ? ?ldr ? ? r0, _TEXT_BASE
>>> - ? ?ldr ? ? r2, _board_init_r
>>> - ? ?sub ? ? r2, r2, r0
>>> - ? ?add ? ? r2, r2, r7 ? ? ?/* position from board_init_r in RAM */
>>> + ? ?ldr ? ? r0, _board_init_r_ofs
>>> + ? ?adr ? ? r1, _start
>>> + ? ?add ? ? r2, r0, r1
>>
>> RFC:
>> I've observed that other ARM CPU's are adding r9 to lr here (add ? ? lr,
>> r0, r9). Even though r9 is not pre-loaded with any value in case
>> CONFIG_SKIP_RELOCATE_UBOOT is defined. Is it bug or not?
>> Maybe it's not tested without CONFIG_SKIP_RELOCATE_UBOOT?
>> I've removed this line because uboot stops booting if it's there.
>> Any comments about it?
>
> Yep, thats a Bug, thanks for detecting this.
>
> But your fix seems to me, is not working for the case if
> CONFIG_SKIP_RELOCATE_UBOOT is !defined, because then you need this
> update of the lr register ... so could you please add this line
> with a "#if !defined(CONFIG_SKIP_RELOCATE_UBOOT)" ... "#endif"
> around it? (And maybe fix all other arm start.S files too?)
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-10-25  7:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-24 20:08 [U-Boot] [PATCH] arm1176: fix relocation Darius Augulis
2010-10-24 20:22 ` Darius Augulis
2010-10-25  7:11   ` Heiko Schocher
2010-10-25  7:47     ` Darius Augulis
2010-10-25  6:17 ` Heiko Schocher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox