S3C24XX: Add support to MINI2416 board

Jumping to board_init_r is not performed due to a bug on address computation.

Relocation offsets are not needed when building SPL.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- None

---
 arch/arm/cpu/arm926ejs/start.S |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +2 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
 	.word nand_boot
 #else
 	ldr	r0, _board_init_r_ofs
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start
 	add	lr, r0, r1
 	add	lr, lr, r9
 	/* setup parameters for board_init_r */

@@ -338,12 +338,14 @@ _board_init_r_ofs:
 	.word board_init_r - _start
 #endif
 
+#ifndef CONFIG_SPL_BUILD

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Quoted 2 lines

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

Quoted 35 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- None

---
  arch/arm/cpu/arm926ejs/start.S |    4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +3 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
  	.word nand_boot
  #else
  	ldr	r0, _board_init_r_ofs
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start
  	add	lr, r0, r1
  	add	lr, lr, r9
  	/* setup parameters for board_init_r */

@@ -338,12 +338,14 @@ _board_init_r_ofs:
  	.word board_init_r - _start
  #endif
 
+#ifndef CONFIG_SPL_BUILD
  _rel_dyn_start_ofs:
  	.word __rel_dyn_start - _start
  _rel_dyn_end_ofs:
  	.word __rel_dyn_end - _start
  _dynsym_start_ofs:
  	.word __dynsym_start - _start
+#endif
 

arch/arm/cpu/arm926ejs/start.S _board_init_r_ofs: · line 351

Quoted 2 lines

arch/arm/cpu/arm926ejs/start.S +0 -0

  /*
   *************************************************************************

Best regards,
Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Quoted 3 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 50 lines

Quoted 50 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

  arch/arm/cpu/arm926ejs/start.S |    4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
  	.word nand_boot
 
  #else
 
  	ldr	r0, _board_init_r_ofs
 
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start

add lr, r0, r1
add lr, lr, r9
/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
  	.word board_init_r - _start
 
  #endif
 
+#ifndef CONFIG_SPL_BUILD
 
  _rel_dyn_start_ofs:
  	.word __rel_dyn_start - _start
 
  _rel_dyn_end_ofs:
  	.word __rel_dyn_end - _start
 
  _dynsym_start_ofs:
  	.word __dynsym_start - _start
 
+#endif
 
  /*

Quoted 5 lines

Quoted 2 lines

   *********************************************************************
   ****

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Dear Jos? Miguel Gon?alves,

Quoted 5 lines · Marek Vasut

Quoted 4 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Quoted 3 lines

Quoted 2 lines

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

Quoted 55 lines

Quoted 3 lines

Quoted 2 lines

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 50 lines

Quoted 50 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

   arch/arm/cpu/arm926ejs/start.S |    4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
   	.word nand_boot
 
   #else
 
   	ldr	r0, _board_init_r_ofs
 
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start

   	add	lr, r0, r1
   	add	lr, lr, r9
   	/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
   	.word board_init_r - _start
 
   #endif
 
+#ifndef CONFIG_SPL_BUILD
 
   _rel_dyn_start_ofs:
   	.word __rel_dyn_start - _start
 
   _rel_dyn_end_ofs:
   	.word __rel_dyn_end - _start
 
   _dynsym_start_ofs:
   	.word __dynsym_start - _start
 
+#endif
 
   /*

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

    *********************************************************************
    ****

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 62 lines

Quoted 62 lines

Quoted 3 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 55 lines

Quoted 42 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

   arch/arm/cpu/arm926ejs/start.S |    4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
   	.word nand_boot
 
   #else
 
   	ldr	r0, _board_init_r_ofs
 
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start

   	add	lr, r0, r1
   	add	lr, lr, r9
   	/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +1 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
   	.word board_init_r - _start
 
   #endif
 
+#ifndef CONFIG_SPL_BUILD
 
   _rel_dyn_start_ofs:
   	.word __rel_dyn_start - _start
 
   _rel_dyn_end_ofs:
   	.word __rel_dyn_end - _start

arch/arm/cpu/arm926ejs/start.S _board_init_r_ofs:

Quoted 10 lines

arch/arm/cpu/arm926ejs/start.S +1 -0

 
   _dynsym_start_ofs:
   	.word __dynsym_start - _start
 
+#endif
 
   /*
 
    ******************************************************************
    *** ****

Best regards,
Marek Vasut

Quoted 5 lines

Quoted 2 lines

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Dear Jos? Miguel Gon?alves,

Quoted 14 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 8 lines · Marek Vasut

Quoted 6 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Quoted 2 lines

Quoted 1 line

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL or something like that.

What do you boot the rest from ?

Quoted 69 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 54 lines

Quoted 54 lines

Quoted 2 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 50 lines

Quoted 50 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

    arch/arm/cpu/arm926ejs/start.S |    4 +++-
    1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
    	.word nand_boot
 
    #else
 
    	ldr	r0, _board_init_r_ofs
 
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start

    	add	lr, r0, r1
    	add	lr, lr, r9
    	/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
    	.word board_init_r - _start
 
    #endif
 
+#ifndef CONFIG_SPL_BUILD
 
    _rel_dyn_start_ofs:
    	.word __rel_dyn_start - _start
 
    _rel_dyn_end_ofs:
    	.word __rel_dyn_end - _start
 
    _dynsym_start_ofs:
    	.word __dynsym_start - _start
 
+#endif
 
    /*

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

     ******************************************************************
     *** ****

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Quoted 23 lines

Dear Jos? Miguel Gon?alves,

Quoted 21 lines · Marek Vasut

Quoted 19 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

Quoted 8 lines

Quoted 1 line

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

My bad. I should be more explicit on the patch description.

Quoted 8 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL or something like that.

Quoted 1 line

What do you boot the rest from ?

Both SPL and U-Boot are in NAND Flash.

The board's SPL code in board/boardcon/mini2416/mini2416_spl.c that needs this patch was based on existing code from arch/arm/cpu/arm926ejs/davinci/spl.c and arm/cpu/armv7/omap-common/spl.c

The need to call relocate_code() in board_init_f() is explained on the SPL source comment, i.e., only to initialize .bss before we could use it in board_init_r() (in the serial driver initialization).

Quoted 74 lines

Quoted 74 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 62 lines

Quoted 62 lines

Quoted 3 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 55 lines

Quoted 42 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

    arch/arm/cpu/arm926ejs/start.S |    4 +++-
    1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
    	.word nand_boot
 
    #else
 
    	ldr	r0, _board_init_r_ofs
 
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start

    	add	lr, r0, r1
    	add	lr, lr, r9
    	/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +1 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
    	.word board_init_r - _start
 
    #endif
 
+#ifndef CONFIG_SPL_BUILD
 
    _rel_dyn_start_ofs:
    	.word __rel_dyn_start - _start
 
    _rel_dyn_end_ofs:
    	.word __rel_dyn_end - _start

arch/arm/cpu/arm926ejs/start.S _board_init_r_ofs:

Quoted 10 lines

arch/arm/cpu/arm926ejs/start.S +1 -0

 
    _dynsym_start_ofs:
    	.word __dynsym_start - _start
 
+#endif
 
    /*
 
     ***************************************************************
     *** *** ****

Best regards,
Marek Vasut

Quoted 5 lines

Quoted 2 lines

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Dear Jos? Miguel Gon?alves,

Quoted 21 lines · Marek Vasut

Quoted 19 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

Quoted 1 line

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL or something like that.

What do you boot the rest from ?

Quoted 66 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 62 lines

Quoted 62 lines

Quoted 3 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 55 lines

Quoted 42 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

   arch/arm/cpu/arm926ejs/start.S |    4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
         .word nand_boot
 
   #else
 
         ldr     r0, _board_init_r_ofs
 
-        ldr     r1, _TEXT_BASE
+        adr     r1, _start

add lr, r0, r1
add lr, lr, r9
/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +1 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
         .word board_init_r - _start
 
   #endif
 
+#ifndef CONFIG_SPL_BUILD
 
   _rel_dyn_start_ofs:
         .word __rel_dyn_start - _start
 
   _rel_dyn_end_ofs:
         .word __rel_dyn_end - _start

arch/arm/cpu/arm926ejs/start.S _board_init_r_ofs:

Quoted 10 lines

arch/arm/cpu/arm926ejs/start.S +1 -0

 
   _dynsym_start_ofs:
         .word __dynsym_start - _start
 
+#endif
 
   /*
 
    ******************************************************************
    *** ****

Best regards,
Marek Vasut

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi,

Quoted 18 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 16 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 5 lines · Marek Vasut

Quoted 4 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Quoted 7 lines

Quoted 3 lines

Quoted 2 lines

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

Quoted 5 lines

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Same for me - I have no idea what you are trying to fix here. In my SPL configuration, _TEXT_BASE and _start point to the same location, so please explain why they are different on your board.

Quoted 7 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL or something like that.

Marek, going into board_init_r is fine for SPL, for example the davinci SPL does some hw initialization in board_init_f and then loads u-boot in board_init_r.
Regards, Christian

Quoted 70 lines

What do you boot the rest from ?

Quoted 68 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 54 lines

Quoted 54 lines

Quoted 2 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 50 lines

Quoted 50 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

    arch/arm/cpu/arm926ejs/start.S |    4 +++-
    1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
          .word nand_boot
 
    #else
 
          ldr     r0, _board_init_r_ofs
 
-        ldr     r1, _TEXT_BASE
+        adr     r1, _start

add lr, r0, r1
add lr, lr, r9
/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
          .word board_init_r - _start
 
    #endif
 
+#ifndef CONFIG_SPL_BUILD
 
    _rel_dyn_start_ofs:
          .word __rel_dyn_start - _start
 
    _rel_dyn_end_ofs:
          .word __rel_dyn_end - _start
 
    _dynsym_start_ofs:
          .word __dynsym_start - _start
 
+#endif
 
    /*

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

     ******************************************************************
     *** ****

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Quoted 41 lines · Christian Riesch

Hi,

Quoted 28 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 16 lines · Marek Vasut

Quoted 15 lines

Dear Jos? Miguel Gon?alves,

Quoted 11 lines · Marek Vasut

Quoted 8 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 8 lines

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 3 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

Quoted 8 lines

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it
causes or ... well, anything. So please, try to be more elaborate in your patch
description next time. Anyway ..

Quoted 3 lines

Same for me - I have no idea what you are trying to fix here. In my SPL configuration, _TEXT_BASE and _start point to the same location, so please explain why they are different on your board.

They are different because of how start.S is implemented in arm926ejs.

In my SPL map file I see:

.text 0x0000000000000000 0xc24
arch/arm/cpu/arm926ejs/start.o(.text)
.text 0x0000000000000000 0x120 arch/arm/cpu/arm926ejs/start.o

                0x0000000000000000                _start
                0x0000000000000040                _TEXT_BASE
                0x0000000000000044                _bss_start_ofs
                0x0000000000000048                _bss_end_ofs
                0x000000000000004c                _end_ofs
                0x0000000000000050                IRQ_STACK_START_IN
                0x0000000000000074                relocate_code

Quoted 13 lines

Quoted 7 lines

Quoted 6 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

Quoted 6 lines

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only
to load the real U-Boot from whatever media, so you usually use either NAND SPL
or something like that.

Quoted 82 lines

Marek, going into board_init_r is fine for SPL, for example the davinci SPL does some hw initialization in board_init_f and then loads u-boot in board_init_r.
Regards, Christian

Quoted 77 lines

What do you boot the rest from ?

Quoted 75 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 63 lines

Quoted 63 lines

Quoted 3 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 56 lines

Quoted 51 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

    arch/arm/cpu/arm926ejs/start.S |    4 +++-
    1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
          .word nand_boot
 
    #else
 
          ldr     r0, _board_init_r_ofs
 
-        ldr     r1, _TEXT_BASE
+        adr     r1, _start

add lr, r0, r1
add lr, lr, r9
/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
          .word board_init_r - _start
 
    #endif
 
+#ifndef CONFIG_SPL_BUILD
 
    _rel_dyn_start_ofs:
          .word __rel_dyn_start - _start
 
    _rel_dyn_end_ofs:
          .word __rel_dyn_end - _start
 
    _dynsym_start_ofs:
          .word __dynsym_start - _start
 
+#endif
 
    /*

Quoted 2 lines

******************************************************************

     *** ****

Best regards,
Marek Vasut

Quoted 5 lines

Quoted 2 lines

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Jos? Gon?alves
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi,

Quoted 41 lines · Jos? Miguel Gon?alves

Quoted 24 lines · Christian Riesch

Hi,

Quoted 21 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 8 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 5 lines · Marek Vasut

Quoted 4 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

Quoted 2 lines

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Quoted 8 lines

Quoted 5 lines

Ok, I don't know how to replicate the bug from this comment or what effects it
causes or ... well, anything. So please, try to be more elaborate in your patch
description next time. Anyway ..

Same for me - I have no idea what you are trying to fix here. In my SPL configuration, _TEXT_BASE and _start point to the same location, so please explain why they are different on your board.

They are different because of how start.S is implemented in arm926ejs.

In my SPL map file I see:

.text 0x0000000000000000 0xc24
arch/arm/cpu/arm926ejs/start.o(.text)
.text 0x0000000000000000 0x120 arch/arm/cpu/arm926ejs/start.o

Quoted 7 lines

                 0x0000000000000000                _start
                 0x0000000000000040                _TEXT_BASE
                 0x0000000000000044                _bss_start_ofs
                 0x0000000000000048                _bss_end_ofs
                 0x000000000000004c                _end_ofs
                 0x0000000000000050                IRQ_STACK_START_IN
                 0x0000000000000074                relocate_code

So _start is 0x00000000, and in your config/include/include/configs/mini2416.h you have

#define CONFIG_SPL_TEXT_BASE 0x00000000

and in arch/arm/cpu/arm926ejs/start.S we have

.globl _TEXT_BASE
_TEXT_BASE:
#ifdef CONFIG_NAND_SPL /* deprecated, use instead CONFIG_SPL_BUILD */
.word CONFIG_SYS_TEXT_BASE
#else
#ifdef CONFIG_SPL_BUILD
.word CONFIG_SPL_TEXT_BASE
#else
.word CONFIG_SYS_TEXT_BASE
#endif
#endif

So

ldr r1, _TEXT_BASE

should be the same as

adr r1, _start

However, if you do not load your SPL to 0x00000000 but to another address and execute it from there, it will be different, since adr uses relative adressing, right? Are you sure you are loading it to 0x00000000?

Regards, Christian

Quoted 90 lines

Quoted 11 lines

Quoted 11 lines

Quoted 5 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only
to load the real U-Boot from whatever media, so you usually use either NAND SPL
or something like that.

Quoted 76 lines

Marek, going into board_init_r is fine for SPL, for example the davinci SPL does some hw initialization in board_init_f and then loads u-boot in board_init_r.
Regards, Christian

Quoted 71 lines

What do you boot the rest from ?

Quoted 69 lines

If the bug is not from here please
suggest me what I need to change in the configuration in order to correctly boot my board.

Quoted 55 lines

Quoted 55 lines

Quoted 2 lines

Quoted 1 line

Relocation offsets are not needed when building SPL.

Do they cause any trouble?

No! Just not needed.

Quoted 51 lines

Quoted 51 lines

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---

Changes for v2:
- None

---

     arch/arm/cpu/arm926ejs/start.S |    4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/start.S index 6f05f1a..2da5342 100644

arch/arm/cpu/arm926ejs/start.S +1 -1

@@ -325,7 +325,7 @@ _nand_boot_ofs:
           .word nand_boot
 
     #else
 
           ldr     r0, _board_init_r_ofs
 
-        ldr     r1, _TEXT_BASE
+        adr     r1, _start

add lr, r0, r1
add lr, lr, r9
/* setup parameters for board_init_r */

arch/arm/cpu/arm926ejs/start.S +2 -0

@@ -338,12 +338,14 @@ _board_init_r_ofs:
           .word board_init_r - _start
 
     #endif
 
+#ifndef CONFIG_SPL_BUILD
 
     _rel_dyn_start_ofs:
           .word __rel_dyn_start - _start
 
     _rel_dyn_end_ofs:
           .word __rel_dyn_end - _start
 
     _dynsym_start_ofs:
           .word __dynsym_start - _start
 
+#endif
 
     /*

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

******************************************************************

      *** ****

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Marek Vasut

Best regards,
Jos? Gon?alves

Best regards,
Jos? Gon?alves

Dear Jos? Miguel Gon?alves,

Quoted 14 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 8 lines

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Quoted 9 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 7 lines · Marek Vasut

Quoted 6 lines

Dear Jos? Miguel Gon?alves,

Quoted 4 lines · Marek Vasut

Quoted 3 lines

Dear Jos? Miguel Gon?alves,

Quoted 1 line

Jumping to board_init_r is not performed due to a bug on

address

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't

detect any

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 1 line

Please check the same chunk of code in other start.S for

arm1176 and

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address

computation."

Quoted 2 lines

Ok, I don't know how to replicate the bug from this comment or what

effects it

Quoted 1 line

causes or ... well, anything. So please, try to be more elaborate

in your patch

Quoted 4 lines

description next time. Anyway ..

Quoted 2 lines

Without this
change the code never reaches board_init_r in the SPL and I think

I have

Quoted 3 lines

Quoted 1 line

all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL

is there only

Quoted 1 line

to load the real U-Boot from whatever media, so you usually use

either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is board_init_f calls relocate_code() to clear the BSS _and_ get our
jump
to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

- In my series this has been changed slightly to be board_init_f calls memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Quoted 57 lines · Tom Rini

Quoted 9 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 7 lines · Marek Vasut

Quoted 6 lines

Dear Jos? Miguel Gon?alves,

Quoted 4 lines · Marek Vasut

Quoted 3 lines

Dear Jos? Miguel Gon?alves,

Quoted 1 line

Jumping to board_init_r is not performed due to a bug on

address

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't

detect any

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 1 line

Please check the same chunk of code in other start.S for

arm1176 and

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on

address computation."

Quoted 2 lines

Ok, I don't know how to replicate the bug from this comment or

what effects it

Quoted 1 line

causes or ... well, anything. So please, try to be more

elaborate in your patch

Quoted 4 lines

description next time. Anyway ..

Quoted 2 lines

Without this
change the code never reaches board_init_r in the SPL and I

think I have

Quoted 3 lines

Quoted 1 line

all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL.

SPL is there only

Quoted 1 line

to load the real U-Boot from whatever media, so you usually use

either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is board_init_f calls relocate_code() to clear the BSS _and_ get
our jump
to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

Quoted 3 lines

- In my series this has been changed slightly to be board_init_f calls memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

So you've removed the ability to relocate at all? What about hardware where you boot from an I/O buffer, that you need to get out of in order to load more pages?

Quoted 32 lines

Dear Jos? Miguel Gon?alves,

Quoted 14 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 8 lines

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 7 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is board_init_f calls relocate_code() to clear the BSS _and_ get our jump to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

- In my series this has been changed slightly to be board_init_f calls memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Dear Tom Rini,

Quoted 49 lines · Marek Vasut

Quoted 32 lines

Dear Jos? Miguel Gon?alves,

Quoted 14 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 8 lines

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 7 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is board_init_f calls relocate_code() to clear the BSS _and_ get our jump to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

Quoted 3 lines

- In my series this has been changed slightly to be board_init_f calls memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Do we need to do all the relocation in assembler code btw? Can it not be moved into C code and made generic across all platforms (module the stack adjustment and a few minor things) ?

Quoted 57 lines

Dear Tom Rini,

Quoted 54 lines · Marek Vasut

Quoted 32 lines

Dear Jos? Miguel Gon?alves,

Quoted 14 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 10 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

Quoted 8 lines

Quoted 6 lines

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 7 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is

board_init_f calls relocate_code() to clear the BSS _and_ get our jump to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

- In my series this has been changed slightly to be board_init_f calls

memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Quoted 3 lines

Do we need to do all the relocation in assembler code btw? Can it not be moved into C code and made generic across all platforms (module the stack adjustment and a few minor things) ?

I think people have posted patches for this before and yes, once CONFIG_NEEDS_MANUAL_RELOC dies it should be all possible to do in C, so long as it doesn't grow in size or doesn't grow in size that would be problematic (remember the 4kb people).

Dear Tom Rini,

Quoted 69 lines · Marek Vasut

Quoted 67 lines

Dear Tom Rini,

Quoted 55 lines · Marek Vasut

Quoted 36 lines

Dear Jos? Miguel Gon?alves,

Quoted 17 lines · Marek Vasut

Quoted 16 lines

Dear Jos? Miguel Gon?alves,

Quoted 11 lines · Marek Vasut

Quoted 8 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a
bug on address computation.

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the
issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 7 lines

Quoted 3 lines

Without this change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand: - In the current in-tree SPL implementations the code flow is

board_init_f calls relocate_code() to clear the BSS _and_ get our jump to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

Quoted 5 lines

- In my series this has been changed slightly to be board_init_f calls

memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Do we need to do all the relocation in assembler code btw? Can it not be moved into C code and made generic across all platforms (module the stack adjustment and a few minor things) ?

Quoted 4 lines

I think people have posted patches for this before and yes, once CONFIG_NEEDS_MANUAL_RELOC dies it should be all possible to do in C, so long as it doesn't grow in size or doesn't grow in size that would be problematic (remember the 4kb people).

How does MANUAL_RELOC interfere with that? Eg. on ARM, it can already be shifted to C. Then if we made it generic enough, those MANUAL_RELOC platforms could simply adopt the C code.

Quoted 24 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 8 lines · Marek Vasut

Quoted 7 lines

Dear Jos? Miguel Gon?alves,

Quoted 5 lines · Marek Vasut

Quoted 4 lines

Dear Jos? Miguel Gon?alves,

Quoted 2 lines

Jumping to board_init_r is not performed due to a bug on address computation.

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

Quoted 2 lines

Is your CONFIG_SYS_TEXT_BASE configured correctly? I don't detect any misbehavior on my arm926 boards.

Maybe because you are not using it to build an SPL?

I do ... and I use CONFIG_SPL_TEXT_BASE properly .

Quoted 2 lines

Please check the same chunk of code in other start.S for arm1176 and armv7. They have the same code that I put for arm926ejs.

Please wait and please first explain what is the issue.

The issue is what I've explained in the patch comments.

"Jumping to board_init_r is not performed due to a bug on address computation."

Ok, I don't know how to replicate the bug from this comment or what effects it causes or ... well, anything. So please, try to be more elaborate in your patch description next time. Anyway ..

Quoted 5 lines

Quoted 3 lines

Without this
change the code never reaches board_init_r in the SPL and I think I have all the configurations correctly set.

I wonder why you'd ever want to reach board_init_r in the SPL. SPL is there only to load the real U-Boot from whatever media, so you usually use either NAND SPL

Here's a good point for me to jump in, I think. There's two things to understand:
- In the current in-tree SPL implementations the code flow is board_init_f calls relocate_code() to clear the BSS _and_ get our jump to board_init_r. It does not actually relocate the running U-Boot, just clears the BSS. board_init_r is what calls the things to load and boot the next stage (U-Boot or Linux).

- In my series this has been changed slightly to be board_init_f calls memset and then board_init_r directly. So this patch should not be needed once rebased on that series.

Jumping to board_init_r is not performed due to a bug on address computation. Relocation offsets are not needed when building SPL.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- None

---
 arch/arm/cpu/arm926ejs/start.S |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

arch/arm/cpu/arm926ejs/start.S +2 -1

index 6f05f1a..2da5342 100644
@@ -325,7 +325,7 @@ _nand_boot_ofs:
 	.word nand_boot
 #else
 	ldr	r0, _board_init_r_ofs
-	ldr	r1, _TEXT_BASE
+	adr	r1, _start
 	add	lr, r0, r1
 	add	lr, lr, r9
 	/* setup parameters for board_init_r */

@@ -338,12 +338,14 @@ _board_init_r_ofs:
 	.word board_init_r - _start
 #endif
 
+#ifndef CONFIG_SPL_BUILD

+static const getfreq freq_f[] = {

arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c new file +16 -0

+int print_cpuinfo(void)
+{
+	int i;
+	char buf[32];
+	ulong cpuid;
+	struct s3c24xx_gpio *const gpio = s3c24xx_get_base_gpio();
+
+	cpuid = readl(&gpio->gstatus[1]);
+	printf("CPU:  %8s (id %08lX) @ %s MHz\n", s3c24xx_get_cpu_name(),
+	       cpuid, strmhz(buf, get_ARMCLK()));
+	for (i = 0; i < ARRAY_SIZE(freq_f); i++)
+		printf("%cCLK: %8s MHz\n", freq_c[i],
+		       strmhz(buf, freq_f[i] ()));
+
+	return 0;
+}

arch/arm/cpu/arm926ejs/s3c24xx/s3c2412_speed.c new file +7 -0

+}
+
+/* return UCLK frequency */
+ulong get_UCLK(void)
+{
+	return get_PLLCLK(UPLL);
+}

arch/arm/cpu/arm926ejs/s3c24xx/s3c2412_speed.c new file +2 -0

+	return (clkdivn & 0x8) ? get_FCLK() / 2 : get_FCLK();
+}

(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/s3c24xx/timer.c new file mode 100644 index 0000000..23d3343

arch/arm/cpu/arm926ejs/s3c24xx/timer.c new file +5 -0

@@ -0,0 +1,152 @@
+/*
+ * (C) Copyright 2012 INOV - INESC Inovacao
+ * Jose Goncalves <[email protected]>
+ *
+ * Based on arch/arm/cpu/armv7/s5p-common/timer.c and U-Boot 1.3.4 from

Samsung. + *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */

arch/arm/cpu/arm926ejs/s3c24xx/timer.c new file +77 -0

+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static ulong get_current_tick(void)
+{
+	struct s3c24xx_timers *const timers = s3c24xx_get_base_timers();
+	ulong now = readl(&timers->tcnto4);
+
+	if (gd->lastinc >= now)
+		gd->tbl += gd->lastinc - now;
+	else
+		gd->tbl += gd->lastinc + gd->tbu - now;
+
+	gd->lastinc = now;
+
+	return gd->tbl;
+}
+
+int timer_init(void)
+{
+	struct s3c24xx_timers *const timers = s3c24xx_get_base_timers();
+
+	/* use PWM Timer 4 because it has no output */
+	if (gd->tbu == 0) {
+		/* set divider value for Timer 4 to 2 */
+		clrsetbits_le32(&timers->tcfg1, TCFG1_MUX4_MASK,
+				TCFG1_MUX4_DIV2);
+		/* set prescaler value for Timer 4 to 15 */
+		clrsetbits_le32(&timers->tcfg0, TCFG0_PRESCALER1_MASK,
+				TCFG0_PRESCALER1(15));
+		/*
+		 * Timer Freq(HZ) =
+		 *      PCLK / (prescaler_value + 1) / (divider_value)
+		 */
+		gd->timer_rate_hz = get_PCLK() / (15 + 1) / 2;
+		gd->tbu = gd->timer_rate_hz / CONFIG_SYS_HZ;
+	}
+	/* load value for selected timeout */
+	writel(gd->tbu, &timers->tcntb4);
+	/* auto reload, manual update of timer 4 */
+	clrsetbits_le32(&timers->tcon, TCON_T4START,
+			TCON_T4RELOAD | TCON_T4MANUALUPD);
+	/* auto reload, start timer 4 */
+	clrsetbits_le32(&timers->tcon, TCON_T4MANUALUPD,
+			TCON_T4RELOAD | TCON_T4START);
+	gd->lastinc = 0;
+	gd->tbl = 0;
+
+	return 0;
+}
+
+ulong get_timer(ulong base)
+{
+	return get_timer_masked() - base;
+}
+
+void __udelay(unsigned long usec)
+{
+	ulong tmo, tmp;
+
+	if (usec >= 1000) {
+		/*
+		 * if "big" number, spread normalization
+		 * to seconds
+		 * 1. start to normalize for usec to ticks per sec
+		 * 2. find number of "ticks" to wait to achieve target
+		 * 3. finish normalize.
+		 */
+		tmo = usec / 1000;
+		tmo *= gd->timer_rate_hz;
+		tmo /= 1000;
+	} else {
+		/* else small number, don't kill it prior to HZ multiply */

arch/arm/cpu/arm926ejs/s3c24xx/timer.c new file +26 -0

+	if ((tmo + tmp + 1) < tmp)
+		reset_timer_masked();
+	else
+		tmo += tmp;
+
+	/* loop till event */
+	while (get_current_tick() < tmo)
+		/* NOP */;
+}
+
+void reset_timer_masked(void)
+{
+	struct s3c24xx_timers *const timers = s3c24xx_get_base_timers();
+
+	/* reset time */
+	gd->lastinc = readl(&timers->tcnto4);
+	gd->tbl = 0;
+}
+
+ulong get_timer_masked(void)
+{
+	return get_current_tick() / gd->tbu;
+}
+
+/*
+ * This function is derived from PowerPC code (read timebase as long

long). + * On ARM it just returns the timer value.
+ */

arch/arm/cpu/arm926ejs/s3c24xx/timer.c new file +13 -0

+unsigned long long get_ticks(void)
+{
+	return get_timer_masked();
+}
+
+/*
+ * This function is derived from PowerPC code (timebase clock frequency).
+ * On ARM it returns the number of timer ticks per second.
+ */
+ulong get_tbclk(void)
+{
+	return CONFIG_SYS_HZ;
+}

Hi Marek,

Quoted 3 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

It's getting better :)

Hopefully :-)

Quoted 6 lines

[...]

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+typedef ulong(*getfreq) (void);

Is this used?

In the array declaration bellow...

Quoted 3 lines

Quoted 1 line

+static const getfreq freq_f[] = {

const array const members, no?

Do you mean I should declare it like this:

static const getfreq const freq[] = { ...

I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.

Quoted 29 lines

arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c line 40

Quoted 7 lines

arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c new file +7 -0

+	get_FCLK,
+	get_HCLK,
+	get_PCLK,
+	get_UCLK,
+};
+
+static const char freq_c[] = { 'F', 'H', 'P', 'U' };

Same here.

Quoted 16 lines

arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c new file +16 -0

+int print_cpuinfo(void)
+{
+	int i;
+	char buf[32];
+	ulong cpuid;
+	struct s3c24xx_gpio *const gpio = s3c24xx_get_base_gpio();
+
+	cpuid = readl(&gpio->gstatus[1]);
+	printf("CPU:  %8s (id %08lX) @ %s MHz\n", s3c24xx_get_cpu_name(),
+	       cpuid, strmhz(buf, get_ARMCLK()));
+	for (i = 0; i < ARRAY_SIZE(freq_f); i++)
+		printf("%cCLK: %8s MHz\n", freq_c[i],
+		       strmhz(buf, freq_f[i] ()));
+
+	return 0;
+}

[...]

Quoted 11 lines

arch/arm/cpu/arm926ejs/s3c24xx/s3c2412_speed.c line 81

Quoted 9 lines

arch/arm/cpu/arm926ejs/s3c24xx/s3c2412_speed.c new file +9 -0

+ulong get_HCLK(void)
+{
+	struct s3c2412_sysctl *const sysctl = s3c2412_get_base_sysctl();
+	u32 clkdivn;
+	u16 hclk_div, arm_div;
+
+	clkdivn = readl(&sysctl->clkdivn);
+	hclk_div = (clkdivn & 0x3) + 1;
+	arm_div = ((clkdivn >> 3) & 0x1) + 1;

Magic.

Missed that... I will fix it.

Quoted 10 lines

Quoted 10 lines

include/common.h +0 -0

index 55025c0..36f0636 100644
@@ -628,6 +628,7 @@ ulong	get_OPB_freq (void);
 
  ulong	get_PCI_freq (void);
  #endif
  #if defined(CONFIG_S3C24X0) || \
 

Quoted 7 lines

Quoted 5 lines

include/common.h +1 -0

+    defined(CONFIG_S3C24XX) || \
 

      defined(CONFIG_LH7A40X) || \
      defined(CONFIG_S3C6400) || \

defined(CONFIG_EP93XX)

What's this change? Split into different patch please.

This is related. I need this to export the functions get_FCLK(), get_HCLK(), get_PCLK() and get_UCLK() used in arch/arm/cpu/arm926ejs/s3c24xx/cpu_info.c

Best regards,
Jos? Gon?alves

Dear Jos? Miguel Gon?alves,

Quoted 11 lines

Hi Marek,

Quoted 3 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

It's getting better :)

Hopefully :-)

Quoted 2 lines

[...]

Quoted 6 lines

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+typedef ulong(*getfreq) (void);

Is this used?

In the array declaration bellow...

Why, these are only values, no ?

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

+static const getfreq freq_f[] = {

const array const members, no?

Do you mean I should declare it like this:

static const getfreq const freq[] = { ...

Yes

Quoted 2 lines

I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.

Type checking, if you ever decided to write into the array, it'll prevent you from doing so.

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 11 lines

Hi Marek,

Quoted 3 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

It's getting better :)

Hopefully :-)

Quoted 2 lines

[...]

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+typedef ulong(*getfreq) (void);

Is this used?

In the array declaration bellow...

Why, these are only values, no ?

They're function pointers. If they were values the compiler should complain, because "getfreq" is used as the type of the array.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

+static const getfreq freq_f[] = {

const array const members, no?

Quoted 5 lines

Quoted 3 lines

Do you mean I should declare it like this:

static const getfreq const freq[] = { ...

Yes

Why? When can you ever change what an array (not a pointer) points to?

Quoted 6 lines

Quoted 3 lines

I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.

Type checking, if you ever decided to write into the array, it'll prevent you from doing so.

The first const takes care of that.

-Scott

Dear Scott Wood,

Quoted 30 lines · Marek Vasut

Quoted 13 lines

Dear Jos? Miguel Gon?alves,

Quoted 11 lines

Hi Marek,

Quoted 3 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

It's getting better :)

Hopefully :-)

Quoted 2 lines

[...]

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+
+typedef ulong(*getfreq) (void);

Is this used?

In the array declaration bellow...

Why, these are only values, no ?

They're function pointers. If they were values the compiler should complain, because "getfreq" is used as the type of the array.

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

Quoted 1 line

+static const getfreq freq_f[] = {

const array const members, no?

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Do you mean I should declare it like this:

static const getfreq const freq[] = { ...

Yes

Why? When can you ever change what an array (not a pointer) points to?

Uh oh ... now I see the stuff with the functions. Crazy

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

I don't see the point because an array has no other storage besides it's elements. Moreover GCC generates the same object code in both ways.

Quoted 4 lines

Quoted 2 lines

Type checking, if you ever decided to write into the array, it'll prevent you from doing so.

The first const takes care of that.

Doesn't one prevent you from manupulating the elements and the other manipulating the array ?

This patch adds the support for Samsung's S3C24XX SoCs that have an ARM926EJS core. Currently it supports S3C2412, S3C2413, S3C2416 and S3C2450.
Tested on an S3C2416 platform.

arch/arm/cpu/arm926ejs/s3c24xx/cpu.c new file +9 -0

+/*
+ * Reset the cpu by setting up the watchdog timer and let him time out.
+ */
+void reset_cpu(ulong addr)
+{
+	struct s3c24xx_watchdog *const watchdog = s3c24xx_get_base_watchdog();
+
+	/* Disable watchdog */
+	writel(0x0000, &watchdog->wtcon);

Program udivslot register in order to obtain a more precise baudrate.

+static const int udivslot[] = {

drivers/serial/serial_s3c24x0.c +6 -2

+	0x0000, 0x0080, 0x0808, 0x0888, 0x2222, 0x4924, 0x4A52, 0x54AA,
+	0x5555, 0xD555, 0xD5D5, 0xDDD5, 0xDDDD, 0xDFDD, 0xDFDF, 0xFFDF,
+};
+
 void _serial_setbrg(const int dev_index)
 {
 	struct s3c24x0_uart *uart = s3c24x0_get_base_uart(dev_index);
-	unsigned int reg = 0;
+	u32 pclk;
+	u32 baudrate;
 	int i;
 
-	/* value is calculated so : (int)(PCLK/16./baudrate) -1 */

The loop used to make a delay after baudrate setting is not necessary. Moreover it is removed by the GCC optimizer (at least with GCC 4.6).

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- New patch

---
 drivers/serial/serial_s3c24x0.c |    3 ---
 1 file changed, 3 deletions(-)

(quoted diff) +0 -0

b/drivers/serial/serial_s3c24x0.c index c9bc121..ec5d1cb 100644

drivers/serial/serial_s3c24x0.c +0 -1

@@ -111,15 +111,12 @@ void _serial_setbrg(const int dev_index)
 	struct s3c24x0_uart *uart = s3c24x0_get_base_uart(dev_index);
 	u32 pclk;
 	u32 baudrate;
-	int i;
 

A better approach to avoid reading the RTC during updates, as sugested in the S3C2416 User's Manual.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- New patch

---
 drivers/rtc/s3c24x0_rtc.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

drivers/rtc/s3c24x0_rtc.c +1 -1

index c16ff2e..7d04b74 100644
@@ -65,20 +65,26 @@ int rtc_get(struct rtc_time *tmp)
 	uchar sec, min, hour, mday, wday, mon, year;
 	__maybe_unused uchar a_sec, a_min, a_hour, a_date,
 			     a_mon, a_year, a_armed;
+	int have_retried = 0;
 
 	/* enable access to RTC registers */
 	SetRTC_Access(RTC_ENABLE);
 
 	/* read RTC registers */
 	do {
-		sec  = readb(&rtc->bcdsec);
 		min  = readb(&rtc->bcdmin);
 		hour = readb(&rtc->bcdhour);
 		mday = readb(&rtc->bcddate);
 		wday = readb(&rtc->bcdday);

rtc_reset() must set the RTC date to the UNIX Epoch.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- New patch

---
 drivers/rtc/s3c24x0_rtc.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/rtc/s3c24x0_rtc.c +4 -4

index 7d04b74..54bf6e3 100644
@@ -167,10 +167,17 @@ int rtc_set(struct rtc_time *tmp)
 
 void rtc_reset(void)
 {
-	struct s3c24x0_rtc *rtc = s3c24x0_get_base_rtc();
-
-	writeb((readb(&rtc->rtccon) & ~0x06) | 0x08, &rtc->rtccon);
-	writeb(readb(&rtc->rtccon) & ~(0x08 | 0x01), &rtc->rtccon);
+	static struct rtc_time tmp = {
+		.tm_year = 1970,
+		.tm_mon = 1,
+		.tm_mday = 1,

This RTC only supports a 100 years range so rtc_set() should not allow setting years bellow 1970 or above 2069.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

---
Changes for v2:
- New patch

---
 drivers/rtc/s3c24x0_rtc.c |    5 +++++
 1 file changed, 5 insertions(+)

drivers/rtc/s3c24x0_rtc.c +0 -0

index 54bf6e3..f96cb11 100644
@@ -139,6 +139,11 @@ int rtc_set(struct rtc_time *tmp)
 	       tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

drivers/mtd/nand/s3c24xx_nand.c new file +30 -0

+#endif
+
+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+	struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+	u_long nfcont;
+
+	nfcont = readl(&nand->nfcont);
+
+	switch (chip) {
+	case -1:
+		nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+		break;
+	case 0:
+		nfcont &= ~NFCONT_NCE0;
+		break;
+	case 1:
+		nfcont &= ~NFCONT_NCE1;
+		break;
+	default:
+		return;
+	}
+
+	writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int

ctrl) +{

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

Quoted 35 lines

drivers/mtd/nand/s3c24xx_nand.c new file +30 -0

+#endif
+
+static void s3c_nand_select_chip(struct mtd_info *mtd, int chip)
+{
+	struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+	u_long nfcont;
+
+	nfcont = readl(&nand->nfcont);
+
+	switch (chip) {
+	case -1:
+		nfcont |= NFCONT_NCE1 | NFCONT_NCE0;
+		break;
+	case 0:
+		nfcont &= ~NFCONT_NCE0;
+		break;
+	case 1:
+		nfcont &= ~NFCONT_NCE1;
+		break;
+	default:
+		return;
+	}
+
+	writel(nfcont, &nand->nfcont);
+}
+
+/*
+ * Hardware specific access to control-lines function
+ */
+static void s3c_nand_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int

ctrl) +{

drivers/mtd/nand/s3c24xx_nand.c new file +3 -0

+	struct s3c24xx_nand *const nand = s3c24xx_get_base_nand();
+	struct nand_chip *this = mtd->priv;
+

drivers/mtd/nand/s3c24xx_nand.c line 78

Quoted 7 lines

drivers/mtd/nand/s3c24xx_nand.c new file +7 -0

+	if (ctrl & NAND_CTRL_CHANGE) {
+		if (ctrl & NAND_CLE)
+			this->IO_ADDR_W = &nand->nfcmmd;
+		else if (ctrl & NAND_ALE)
+			this->IO_ADDR_W = &nand->nfaddr;
+		else
+			this->IO_ADDR_W = &nand->nfdata;

Why don't you use local variable here?

drivers/mtd/nand/s3c24xx_nand.c line 80

Quoted 3 lines

drivers/mtd/nand/s3c24xx_nand.c new file +2 -0

+		if (ctrl & NAND_NCE)
+			s3c_nand_select_chip(mtd, *(int *)this->priv);

Uh, how's this *(int *) supposed to work?

Quoted 21 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

Quoted 3 lines

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

Quoted 25 lines · Jos? Miguel Gon?alves

Quoted 24 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

Quoted 4 lines

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Quoted 26 lines · Tom Rini

Quoted 25 lines · Jos? Miguel Gon?alves

Quoted 24 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

Quoted 8 lines

Quoted 4 lines

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl
   text       data        bss        dec        hex    filename
   3337          8        588       3933        f5d    u-boot-spl

$ size u-boot-spl-printf
   text       data        bss        dec        hex    filename
   7968          8        604       8580       2184 u-boot-spl-printf

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

Quoted 2 lines · Jos? Miguel Gon?alves

Quoted 1 line · Tom Rini

On Fri, Sep 14, 2012 at 07:45:40PM +0100, Jos? Miguel Gon?alves

Quoted 22 lines · wrote:

Quoted 22 lines

Quoted 22 lines · Marek Vasut

Quoted 21 lines

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least

in my

Quoted 1 line

Quoted 1 line

SPL series that's still enough to get you printf/puts (I believe

4kb was

Quoted 5 lines

Quoted 2 lines

the cutoff where that had to be dropped).

Barely:

Quoted 7 lines

$ size u-boot-spl
   text       data        bss        dec        hex    filename
   3337          8        588       3933        f5d    u-boot-spl

$ size u-boot-spl-printf
   text       data        bss        dec        hex    filename
   7968          8        604       8580       2184  

u-boot-spl-printf

Quoted 3 lines

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

There's two parts to this:
- What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and
SD
support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)
- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to
the
next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

Quoted 52 lines · Tom Rini

Quoted 24 lines · Jos? Miguel Gon?alves

Quoted 23 lines · Tom Rini

Quoted 22 lines · Jos? Miguel Gon?alves

Quoted 21 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

Review

Quoted 11 lines

(quoted diff) +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS    2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

Quoted 3 lines

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least

in my

Quoted 1 line

Quoted 1 line

SPL series that's still enough to get you printf/puts (I believe

4kb was

Quoted 9 lines

Quoted 2 lines

the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl
   text       data        bss        dec        hex    filename
   3337          8        588       3933        f5d    u-boot-spl

Quoted 6 lines

$ size u-boot-spl-printf
   text       data        bss        dec        hex    filename
   7968          8        604       8580       2184 u-boot-spl-printf

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

Quoted 14 lines

There's two parts to this:
- What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and SD support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)
- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to the next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

Weak symbols are not OK for configuring printf out of the SPL, as you'll still have all the format strings and caller code in the binary. It should be a macro (or an inline function that replaces the standard printf declaration), but it should be in a system header (not the CPU directory -- not sure what Marek meant there) and be based on an appropriate CONFIG symbol.

Quoted 1 line · Scott Wood

Weak symbols are not OK for configuring printf out of the SPL, as

you'll

Quoted 3 lines

still have all the format strings and caller code in the binary. It should be a macro (or an inline function that replaces the standard printf declaration), but it should be in a system header (not the

CPU

Quoted 2 lines

directory -- not sure what Marek meant there) and be based on an appropriate CONFIG symbol.

I'm a little leery of adding #if ... into <common.h> around printf. I'd
like to not worry about the branch/return bytes until we really really have to again but yes, the strings are more of a concern since they won't be collected out. Just top of my head thinking above.

Quoted 45 lines · Jos? Miguel Gon?alves

Quoted 26 lines · Tom Rini

Quoted 25 lines · Jos? Miguel Gon?alves

Quoted 24 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 4 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by Jos? Miguel Gon?alves <[email protected]>

[...]

drivers/mtd/nand/s3c24xx_nand.c line 37

Quoted 11 lines

drivers/mtd/nand/s3c24xx_nand.c new file +11 -0

+#include <common.h>
+#include <nand.h>
+#include <asm/io.h>
+#include <asm/arch/s3c24xx_cpu.h>
+#include <asm/errno.h>
+
+#define MAX_CHIPS	2
+static int nand_cs[MAX_CHIPS] = { 0, 1 };
+
+#ifdef CONFIG_SPL_BUILD
+#define printf(arg...) do {} while (0)

This doesn't seem quite right ...

1) this should be in CPU directory
2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set
3) should be inline function, not a macro

Quoted 8 lines

Quoted 4 lines

1) and 3) OK.
Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl
    text       data        bss        dec        hex    filename
    3337          8        588       3933        f5d    u-boot-spl

$ size u-boot-spl-printf
    text       data        bss        dec        hex    filename
    7968          8        604       8580       2184 u-boot-spl-printf

Quoted 2 lines

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

There's two parts to this:
- What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and SD support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)

- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to the next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

Quoted 45 lines · Tom Rini

Quoted 43 lines · Jos? Miguel Gon?alves

Quoted 23 lines · Tom Rini

Quoted 21 lines · Jos? Miguel Gon?alves

Quoted 20 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 5 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by: Jos? Miguel Gon?alves
<[email protected]>

[...]

Quoted 6 lines

+#include <common.h> +#include <nand.h> +#include <asm/io.h> +#include <asm/arch/s3c24xx_cpu.h> +#include
<asm/errno.h> + +#define MAX_CHIPS 2 +static int
nand_cs[MAX_CHIPS] = { 0, 1 }; + +#ifdef CONFIG_SPL_BUILD +#define printf(arg...) do {} while
(0)

This doesn't seem quite right ...

1) this should be in CPU directory 2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set 3) should be inline function, not a macro

Quoted 9 lines

Quoted 4 lines

1) and 3) OK. Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl text       data        bss        dec
hex    filename 3337          8        588       3933
f5d    u-boot-spl

$ size u-boot-spl-printf text       data        bss        dec
hex    filename 7968          8        604       8580
2184 u-boot-spl-printf

Quoted 8 lines

Quoted 3 lines

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

There's two parts to this: - What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and SD support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)

That's exactly what I've got in mind when I talked about a future expansion! Being able to boot also from an SD card. With only 8KB for .text and .data, I can not use printfs in the SPL for this platform (at least with the present printf support for SPL).

Quoted 9 lines

- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to the next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 54 lines · Jos? Miguel Gon?alves

Quoted 45 lines · Tom Rini

Quoted 43 lines · Jos? Miguel Gon?alves

Quoted 23 lines · Tom Rini

Quoted 21 lines · Jos? Miguel Gon?alves

Quoted 20 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 5 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by: Jos? Miguel Gon?alves
<[email protected]>

[...]

Quoted 6 lines

+#include <common.h> +#include <nand.h> +#include <asm/io.h> +#include <asm/arch/s3c24xx_cpu.h> +#include
<asm/errno.h> + +#define MAX_CHIPS 2 +static int
nand_cs[MAX_CHIPS] = { 0, 1 }; + +#ifdef CONFIG_SPL_BUILD +#define printf(arg...) do {} while
(0)

This doesn't seem quite right ...

1) this should be in CPU directory 2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set 3) should be inline function, not a macro

Quoted 9 lines

Quoted 4 lines

1) and 3) OK. Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl text       data        bss        dec
hex    filename 3337          8        588       3933
f5d    u-boot-spl

$ size u-boot-spl-printf text       data        bss        dec
hex    filename 7968          8        604       8580
2184 u-boot-spl-printf

Quoted 8 lines

Quoted 3 lines

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

There's two parts to this: - What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and SD support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)

Quoted 14 lines

That's exactly what I've got in mind when I talked about a future expansion! Being able to boot also from an SD card. With only 8KB for .text and .data, I can not use printfs in the SPL for this platform (at least with the present printf support for SPL).

Quoted 9 lines

- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to the next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

OK, so please take a stab at option two, on top of my SPL series, keeping in mind what Scott has said (which makes sense) because otherwise you'll be changing a lot of MMC files too to drop out printf :)

Quoted 72 lines · Tom Rini

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 54 lines · Jos? Miguel Gon?alves

Quoted 45 lines · Tom Rini

Quoted 43 lines · Jos? Miguel Gon?alves

Quoted 23 lines · Tom Rini

Quoted 21 lines · Jos? Miguel Gon?alves

Quoted 20 lines · Marek Vasut

Dear Jos? Miguel Gon?alves,

Quoted 5 lines

NAND Flash driver with HW ECC for the S3C24XX SoCs. Currently it only supports SLC NAND chips.

Signed-off-by: Jos? Miguel Gon?alves
<[email protected]>

[...]

Quoted 6 lines

+#include <common.h> +#include <nand.h> +#include <asm/io.h> +#include <asm/arch/s3c24xx_cpu.h> +#include
<asm/errno.h> + +#define MAX_CHIPS 2 +static int
nand_cs[MAX_CHIPS] = { 0, 1 }; + +#ifdef CONFIG_SPL_BUILD +#define printf(arg...) do {} while
(0)

This doesn't seem quite right ...

1) this should be in CPU directory 2) should be enabled only if CONFIG_SPL_SERIAL_SUPPORT is not set 3) should be inline function, not a macro

Quoted 9 lines

Quoted 4 lines

1) and 3) OK. Don't quite understand 2). I want to remove the printfs in the SPL build, as it would blown up the internal SoC RAM space available. So why add a condition with CONFIG_SPL_SERIAL_SUPPORT?

You've got 8KB, based on the final patch in the series. At least in my SPL series that's still enough to get you printf/puts (I believe 4kb was the cutoff where that had to be dropped).

Barely:

$ size u-boot-spl text       data        bss        dec
hex    filename 3337          8        588       3933
f5d    u-boot-spl

$ size u-boot-spl-printf text       data        bss        dec
hex    filename 7968          8        604       8580
2184 u-boot-spl-printf

Quoted 8 lines

Quoted 3 lines

The printf is not so important that justifies exhausting the IRAM space available and preventing any future SPL expansion...

There's two parts to this: - What else can you do in a single binary, in theory? Is there boot medium detection and you would want to have, for example, NAND and SD support in the same binary? I would say memory is meant for using, but this is a board maintainer decision and that's you :)

Quoted 14 lines

That's exactly what I've got in mind when I talked about a future expansion! Being able to boot also from an SD card. With only 8KB for .text and .data, I can not use printfs in the SPL for this platform (at least with the present printf support for SPL).

Quoted 9 lines

- We have a define today (CONFIG_SPL_LIBCOMMON_SUPPORT) that toggles printf or no printf. If we really need to say yes to LIBCOMMON_SUPPORT and no to printf, we need finer grained config options and then a do-nothing printf is used for SPL. Doing the opt-out driver by driver just punts this problem down the road to the next developer and that's not very nice (and adding CONFIG_SPL_PRINTF_SUPPORT shouldn't be a big patch, modify a few Makefiles, update a bunch of config files, add common/spl/dummy_funcs.c and a __weak printf).

Quoted 3 lines

OK, so please take a stab at option two, on top of my SPL series, keeping in mind what Scott has said (which makes sense) because otherwise you'll be changing a lot of MMC files too to drop out printf :)

The solution that I sorted out on the current SPL framework was to add this:

#ifdef CONFIG_SPL_BUILD
#define printf(arg...) do {} while (0)
#ifdef CONFIG_SPL_SERIAL_SUPPORT
#define puts(arg) serial_puts(arg)
#endif
#endif

on a CPU specific header. Marek told me to not use macros, but to use inline functions instead, but has I told earlier on this thread, I am unable to do that. Suggestions for doing this in a better way are welcome...