diff mbox

[U-Boot,1/5] devkit8000 nand_spl: armv7 support nand_spl boot

Message ID 1309270480-31918-2-git-send-email-schwarz@corscience.de
State Superseded
Headers show

Commit Message

Simon Schwarz June 28, 2011, 2:14 p.m. UTC 
This adds changes to armv7 files to support NAND_SPL. The execution of lowlevel init is prevented in normal u-boot. No Exeception code in SPL. board_init_f/r are replaced by board_init_spl. Much code is deactivated by defines for the SPL case.

Signed-off-by: Simon Schwarz <schwarz@corscience.de>
--

Comments

Andreas Bießmann June 29, 2011, 8:27 a.m. UTC | #1 
Dear Simon Schwarz,

(this is a review of RFC for omap3 nand spl Simon does for his bachelor
thesis, there is one question in regarding the current 'SPL framework
redesign' discussion -> entry point for spl code).

Am 28.06.2011 16:14, schrieb simonschwarzcor@googlemail.com:

<snip long line>

> --
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index d91ae12..ebbfce4 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -33,6 +33,11 @@
>  #include <config.h>
>  #include <version.h>
>  
> +/* prevent lowlevel init if this is not the preloader*/
> +#if defined(CONFIG_SPL) && !defined(CONFIG_PRELOADER)
> +#define CONFIG_SKIP_LOWLEVEL_INIT
> +#endif
> +

not here, use your board configuration header for this.

>  .globl _start
>  _start: b	reset
>  	ldr	pc, _undefined_instruction
> @@ -43,6 +48,17 @@ _start: b	reset
>  	ldr	pc, _irq
>  	ldr	pc, _fiq
>  
> +#ifdef CONFIG_PRELOADER
> +/* If in Preloader don't use interrupts...*/
> +_undefined_instruction: .word undefined_instruction
> +_software_interrupt:    .word _software_interrupt
> +_prefetch_abort:    .word _prefetch_abort
> +_data_abort:        .word data_abort
> +_not_used:      .word _not_used
> +_irq:           .word _irq
> +_fiq:           .word _fiq
> +_pad:           .word 0x12345678 /* now 16*4=64 */
> +#else
>  _undefined_instruction: .word undefined_instruction
>  _software_interrupt:	.word software_interrupt
>  _prefetch_abort:	.word prefetch_abort
> @@ -51,6 +67,7 @@ _not_used:		.word not_used
>  _irq:			.word irq
>  _fiq:			.word fiq
>  _pad:			.word 0x12345678 /* now 16*4=64 */
> +#endif /* CONFIG_PRELOADER */

why resetting the vector table? just do not enable irq at all should
also do the job, isn't it?

>  .global _end_vect
>  _end_vect:
>  
> @@ -85,6 +102,7 @@ _armboot_start:
>  /*
>   * These are defined in the board-specific linker script.
>   */
> +#ifndef CONFIG_PRELOADER

isn't that SPL specific?

>  .globl _bss_start_ofs
>  _bss_start_ofs:
>  	.word __bss_start - _start
> @@ -96,6 +114,16 @@ _bss_end_ofs:
>  .globl _end_ofs
>  _end_ofs:
>  	.word _end - _start
> +#else
> +/* preserved the _ofs because although there is no offset */
> +.globl _bss_start_ofs
> +_bss_start_ofs:
> +	.word __bss_start
> +
> +.globl _bss_end_ofs
> +_bss_end_ofs:
> +	.word __bss_end__
> +#endif
>  
>  #ifdef CONFIG_USE_IRQ
>  /* IRQ stack memory (calculated at run-time) */
> @@ -153,8 +181,15 @@ next:
>  	/* the mask ROM code should have PLL and others stable */
>  #ifndef CONFIG_SKIP_LOWLEVEL_INIT
>  	bl	cpu_init_crit
> -#endif
> -
> +#endif /* #ifdef CONFIG_PRELOADER */

ok to mark this #endif, but do not use  '/* #ifdef', it looks like
commented out ifdef.

> +
> +#ifdef CONFIG_PRELOADER
> +call_board_init_spl:
> +    ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
> +    bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
> +    ldr r0,=0x00000000
> +    bl  board_init_spl

why a new interface for entry point? You could just use
call_board_init_f() even for spl (just do not compile lib/board.c and
provide another one for spl). -> this should be discussed in 'SPL
framework redesign'.

> +#else
>  /* Set stackpointer in internal RAM to call board_init_f */
>  call_board_init_f:
>  	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
> @@ -182,10 +217,8 @@ stack_setup:
>  	mov	sp, r4
>  
>  	adr	r0, _start
> -#ifndef CONFIG_PRELOADER

hmm ... that ifndef was intentionally there

>  	cmp	r0, r6
>  	beq	clear_bss		/* skip relocation */
> -#endif
>  	mov	r1, r6			/* r1 <- scratch for copy_loop */
>  	ldr	r3, _bss_start_ofs
>  	add	r2, r0, r3		/* r2 <- source end address	    */
> @@ -196,7 +229,6 @@ copy_loop:
>  	cmp	r0, r2			/* until source end address [r2]    */
>  	blo	copy_loop
>  
> -#ifndef CONFIG_PRELOADER
>  	/*
>  	 * fix .rel.dyn relocations
>  	 */
> @@ -248,7 +280,6 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
>  	add	r0, r0, #4
>  	cmp	r0, r1
>  	bne	clbss_l
> -#endif	/* #ifndef CONFIG_PRELOADER */
>  
>  /*
>   * We are done. Do not return, instead branch to second part of board
> @@ -265,16 +296,18 @@ jump_2_ram:
>  	/* jump to it ... */
>  	mov	pc, lr
>  
> -_board_init_r_ofs:
> -	.word board_init_r - _start
> -
>  _rel_dyn_start_ofs:
> -	.word __rel_dyn_start - _start
> +    .word __rel_dyn_start - _start
>  _rel_dyn_end_ofs:
> -	.word __rel_dyn_end - _start
> +    .word __rel_dyn_end - _start
>  _dynsym_start_ofs:
> -	.word __dynsym_start - _start
> +    .word __dynsym_start - _start
>  
> +_board_init_r_ofs:
> +	.word board_init_r - _start

these changes are not necessary!

> +#endif /* #ifndef CONFIG_PRELOADER */
> +
> +#ifndef CONFIG_SKIP_LOWLEVEL_INIT

this shouldn't be required here. If there is an error in start.S please
provide another patch fixing the error.

>  /*************************************************************************
>   *
>   * CPU_init_critical registers
> @@ -311,6 +344,8 @@ cpu_init_crit:
>  	bl	lowlevel_init		@ go setup pll,mux,memory
>  	mov	lr, ip			@ restore link
>  	mov	pc, lr			@ back to my caller
> +#endif /* CONFIG_SKIP_LOWLEVEL_INIT */
> +
>  /*
>   *************************************************************************
>   *
> @@ -437,6 +472,7 @@ cpu_init_crit:
>  /*
>   * exception handlers
>   */
> +#ifndef CONFIG_PRELOADER
>  	.align	5
>  undefined_instruction:
>  	get_bad_stack
> @@ -499,3 +535,14 @@ fiq:
>  	bl	do_fiq
>  
>  #endif
> +#endif /* CONFIG_PRELOADER */

#endif /* CONFIG_PRELOADER */ followed by #ifdef CONFIG_PRELOADER? ...
how about #else?

> +

no empty line then

> +#ifdef CONFIG_PRELOADER
> +	.align 	5
> +undefined_instruction:
> +	b undefined_instruction
> +
> +	.align	5
> +data_abort:
> +	b data_abort
> +#endif

I guess you want to save space by reducing the irq vector overhead. But
shouldn't this be some #ifdef CONFIG_USE_IRQ instead of CONFIG_PRELOADER?

> diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
> index 08e6acb..ad444cb 100644
> --- a/arch/arm/lib/reset.c
> +++ b/arch/arm/lib/reset.c
> @@ -44,8 +44,9 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  	puts ("resetting ...\n");
>  
>  	udelay (50000);				/* wait 50 ms */
> -
> +#ifndef CONFIG_PRELOADER
>  	disable_interrupts();
> +#endif

again, use CONFIG_USE_IRQ here would be better. But providing an empty
disable_interrupts() in either case you can drop these #ifndef here too.

>  	reset_cpu(0);
>  
>  	/*NOTREACHED*/

regards

Andreas Bießmann
Aneesh V June 30, 2011, 11:10 a.m. UTC | #2 
This seems to be an extension of the work I did for OMAP4. So, I shall
respond for the code I had added.

On Wednesday 29 June 2011 01:57 PM, Andreas Bießmann wrote:
> Dear Simon Schwarz,

[snip ..]

>> +#ifdef CONFIG_PRELOADER
>> +/* If in Preloader don't use interrupts...*/
>> +_undefined_instruction: .word undefined_instruction
>> +_software_interrupt:    .word _software_interrupt
>> +_prefetch_abort:    .word _prefetch_abort
>> +_data_abort:        .word data_abort
>> +_not_used:      .word _not_used
>> +_irq:           .word _irq
>> +_fiq:           .word _fiq
>> +_pad:           .word 0x12345678 /* now 16*4=64 */
>> +#else
>>   _undefined_instruction: .word undefined_instruction
>>   _software_interrupt:	.word software_interrupt
>>   _prefetch_abort:	.word prefetch_abort
>> @@ -51,6 +67,7 @@ _not_used:		.word not_used
>>   _irq:			.word irq
>>   _fiq:			.word fiq
>>   _pad:			.word 0x12345678 /* now 16*4=64 */
>> +#endif /* CONFIG_PRELOADER */
>
> why resetting the vector table? just do not enable irq at all should
> also do the job, isn't it?

The main purpose was to compile out the exception handlers and related
code down below, that would not be much needed in SPL. In the absence
of these handlers I wanted spin-loops in the vector so that we get
some debugging hints with JTAG debuggers.

>
>>   .global _end_vect
>>   _end_vect:
[snip ..]
>> +#endif /* #ifdef CONFIG_PRELOADER */
>
> ok to mark this #endif, but do not use  '/* #ifdef', it looks like
> commented out ifdef.

Ok.

>
>> +

[snip ..]

>>   	adr	r0, _start
>> -#ifndef CONFIG_PRELOADER
>
> hmm ... that ifndef was intentionally there

I wanted to skip relocation but allow clear_bss() for SPL.

>
>>   	cmp	r0, r6
>>   	beq	clear_bss		/* skip relocation */

[snip ..]

>> +#ifdef CONFIG_PRELOADER
>> +	.align 	5
>> +undefined_instruction:
>> +	b undefined_instruction
>> +
>> +	.align	5
>> +data_abort:
>> +	b data_abort
>> +#endif
>
> I guess you want to save space by reducing the irq vector overhead. But
> shouldn't this be some #ifdef CONFIG_USE_IRQ instead of CONFIG_PRELOADER?

Not just IRQ but all exception handelrs.
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index d91ae12..ebbfce4 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -33,6 +33,11 @@ 
 #include <config.h>
 #include <version.h>
 
+/* prevent lowlevel init if this is not the preloader*/
+#if defined(CONFIG_SPL) && !defined(CONFIG_PRELOADER)
+#define CONFIG_SKIP_LOWLEVEL_INIT
+#endif
+
 .globl _start
 _start: b	reset
 	ldr	pc, _undefined_instruction
@@ -43,6 +48,17 @@  _start: b	reset
 	ldr	pc, _irq
 	ldr	pc, _fiq
 
+#ifdef CONFIG_PRELOADER
+/* If in Preloader don't use interrupts...*/
+_undefined_instruction: .word undefined_instruction
+_software_interrupt:    .word _software_interrupt
+_prefetch_abort:    .word _prefetch_abort
+_data_abort:        .word data_abort
+_not_used:      .word _not_used
+_irq:           .word _irq
+_fiq:           .word _fiq
+_pad:           .word 0x12345678 /* now 16*4=64 */
+#else
 _undefined_instruction: .word undefined_instruction
 _software_interrupt:	.word software_interrupt
 _prefetch_abort:	.word prefetch_abort
@@ -51,6 +67,7 @@  _not_used:		.word not_used
 _irq:			.word irq
 _fiq:			.word fiq
 _pad:			.word 0x12345678 /* now 16*4=64 */
+#endif /* CONFIG_PRELOADER */
 .global _end_vect
 _end_vect:
 
@@ -85,6 +102,7 @@  _armboot_start:
 /*
  * These are defined in the board-specific linker script.
  */
+#ifndef CONFIG_PRELOADER
 .globl _bss_start_ofs
 _bss_start_ofs:
 	.word __bss_start - _start
@@ -96,6 +114,16 @@  _bss_end_ofs:
 .globl _end_ofs
 _end_ofs:
 	.word _end - _start
+#else
+/* preserved the _ofs because although there is no offset */
+.globl _bss_start_ofs
+_bss_start_ofs:
+	.word __bss_start
+
+.globl _bss_end_ofs
+_bss_end_ofs:
+	.word __bss_end__
+#endif
 
 #ifdef CONFIG_USE_IRQ
 /* IRQ stack memory (calculated at run-time) */
@@ -153,8 +181,15 @@  next:
 	/* the mask ROM code should have PLL and others stable */
 #ifndef CONFIG_SKIP_LOWLEVEL_INIT
 	bl	cpu_init_crit
-#endif
-
+#endif /* #ifdef CONFIG_PRELOADER */
+
+#ifdef CONFIG_PRELOADER
+call_board_init_spl:
+    ldr sp, =(CONFIG_SYS_INIT_SP_ADDR)
+    bic sp, sp, #7 /* 8-byte alignment for ABI compliance */
+    ldr r0,=0x00000000
+    bl  board_init_spl
+#else
 /* Set stackpointer in internal RAM to call board_init_f */
 call_board_init_f:
 	ldr	sp, =(CONFIG_SYS_INIT_SP_ADDR)
@@ -182,10 +217,8 @@  stack_setup:
 	mov	sp, r4
 
 	adr	r0, _start
-#ifndef CONFIG_PRELOADER
 	cmp	r0, r6
 	beq	clear_bss		/* skip relocation */
-#endif
 	mov	r1, r6			/* r1 <- scratch for copy_loop */
 	ldr	r3, _bss_start_ofs
 	add	r2, r0, r3		/* r2 <- source end address	    */
@@ -196,7 +229,6 @@  copy_loop:
 	cmp	r0, r2			/* until source end address [r2]    */
 	blo	copy_loop
 
-#ifndef CONFIG_PRELOADER
 	/*
 	 * fix .rel.dyn relocations
 	 */
@@ -248,7 +280,6 @@  clbss_l:str	r2, [r0]		/* clear loop...		    */
 	add	r0, r0, #4
 	cmp	r0, r1
 	bne	clbss_l
-#endif	/* #ifndef CONFIG_PRELOADER */
 
 /*
  * We are done. Do not return, instead branch to second part of board
@@ -265,16 +296,18 @@  jump_2_ram:
 	/* jump to it ... */
 	mov	pc, lr
 
-_board_init_r_ofs:
-	.word board_init_r - _start
-
 _rel_dyn_start_ofs:
-	.word __rel_dyn_start - _start
+    .word __rel_dyn_start - _start
 _rel_dyn_end_ofs:
-	.word __rel_dyn_end - _start
+    .word __rel_dyn_end - _start
 _dynsym_start_ofs:
-	.word __dynsym_start - _start
+    .word __dynsym_start - _start
 
+_board_init_r_ofs:
+	.word board_init_r - _start
+#endif /* #ifndef CONFIG_PRELOADER */
+
+#ifndef CONFIG_SKIP_LOWLEVEL_INIT
 /*************************************************************************
  *
  * CPU_init_critical registers
@@ -311,6 +344,8 @@  cpu_init_crit:
 	bl	lowlevel_init		@ go setup pll,mux,memory
 	mov	lr, ip			@ restore link
 	mov	pc, lr			@ back to my caller
+#endif /* CONFIG_SKIP_LOWLEVEL_INIT */
+
 /*
  *************************************************************************
  *
@@ -437,6 +472,7 @@  cpu_init_crit:
 /*
  * exception handlers
  */
+#ifndef CONFIG_PRELOADER
 	.align	5
 undefined_instruction:
 	get_bad_stack
@@ -499,3 +535,14 @@  fiq:
 	bl	do_fiq
 
 #endif
+#endif /* CONFIG_PRELOADER */
+
+#ifdef CONFIG_PRELOADER
+	.align 	5
+undefined_instruction:
+	b undefined_instruction
+
+	.align	5
+data_abort:
+	b data_abort
+#endif
diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
index 08e6acb..ad444cb 100644
--- a/arch/arm/lib/reset.c
+++ b/arch/arm/lib/reset.c
@@ -44,8 +44,9 @@  int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 	puts ("resetting ...\n");
 
 	udelay (50000);				/* wait 50 ms */
-
+#ifndef CONFIG_PRELOADER
 	disable_interrupts();
+#endif
 	reset_cpu(0);
 
 	/*NOTREACHED*/