ARM: mx31pdk: Use the new relocation scheme

RFC In review Fabio Estevam 5 participants 17 messages Jan 12, 2011 – Feb 11, 2011 Arm (32-bit)

lore

Share & tools

lore

Fabio Estevam

author

2011-01-12 12:49 UTC #0 lore raw

RFC 1/1 ARM: mx31pdk: Use the new relocation scheme

Hi,

I am trying to make the new relocation scheme to work on the mx31pdk board.

With this patch applied the mx31pdk build works, but not able to get a U-boot prompt yet.

I copied the nand_spl/board/karo/tx25/u-boot.lds to the mx31pdk one.

I am not certain whether the CONFIG_SYS_TEXT_BASE should be changed or not:

ifdef CONFIG_NAND_SPL
CONFIG_SYS_TEXT_BASE = 0x87ec0000
else
CONFIG_SYS_TEXT_BASE = 0x87f00000
endif

If anyone has any comments/suggestions I will appreciate it.

Thanks,

Fabio Estevam

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/cpu/arm1136/start.S                |    4 +-
 board/freescale/mx31pdk/mx31pdk.c           |   10 ++++-
 include/configs/mx31pdk.h                   |    6 +++
 nand_spl/board/freescale/mx31pdk/u-boot.lds |   59 +++++++++++++++++++++++---
 4 files changed, 68 insertions(+), 11 deletions(-)

arch/arm/cpu/arm1136/start.S +2 -2

index 9a6f6cb..85b5e44 100644
@@ -271,8 +271,8 @@ clbss_l:str	r2, [r0]		/* clear loop...		    */
 	ldr     r0, _nand_boot_ofs
 	adr	r1, _start
 	add	pc, r0, r1
-_nand_boot_ofs
-	: .word nand_boot - _start
+_nand_boot_ofs:	
+	.word nand_boot - _start
 #else
 jump_2_ram:
 	ldr	r0, _board_init_r_ofs

board/freescale/mx31pdk/mx31pdk.c +8 -2

index 9f47169..2756e5a 100644
@@ -33,10 +33,16 @@ DECLARE_GLOBAL_DATA_PTR;
 
 int dram_init(void)
 {
+	/* dram_init must store complete ramsize in gd->ram_size */
+	gd->ram_size = get_ram_size((volatile void *)CONFIG_SYS_SDRAM_BASE,
+				PHYS_SDRAM_1_SIZE);
+	return 0;
+}
+
+void dram_init_banksize(void)
+{
 	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
 	gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
-
-	return 0;
 }
 
 int board_init(void)

include/configs/mx31pdk.h +6 -0

index 3b90a01..e3f8b9f 100644
@@ -155,6 +155,12 @@
 #define PHYS_SDRAM_1		CSD0_BASE
 #define PHYS_SDRAM_1_SIZE	(128 * 1024 * 1024)
 
+#define CONFIG_SYS_SDRAM_BASE		PHYS_SDRAM_1
+#define CONFIG_SYS_INIT_RAM_ADDR	IRAM_BASE_ADDR
+#define CONFIG_SYS_INIT_RAM_SIZE	IRAM_SIZE
+#define CONFIG_SYS_GBL_DATA_OFFSET	(CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
+#define CONFIG_SYS_INIT_SP_ADDR	(CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_GBL_DATA_OFFSET)
+
 /*-----------------------------------------------------------------------
  * FLASH and environment organization
  */

nand_spl/board/freescale/mx31pdk/u-boot.lds +52 -7

73 lines not on this page · raw message

Signature · 1 line

1.6.0.4

Stefano Babic

2011-01-13 13:38 UTC #1 lore raw

Quoted 1 line · Fabio Estevam

Hi,

Hi Fabio,

Quoted 4 lines

I am trying to make the new relocation scheme to work on the mx31pdk board.
With this patch applied the mx31pdk build works, but not able to get a U-boot prompt yet.

I have tried to imagine some issues, but I cannot find a clear reason. Building with your patch I checked that nand_spl is still less than 2KB code, and that matches the NAND page, as required by i.MX31 - so it is not a problem.

Have you tried to disable the relocation, just to check if the relocation is an issue with CONFIG_SKIP_RELOCATE_UBOOT ? This is not a solution, but only to check what happens.

Quoted 10 lines

I copied the nand_spl/board/karo/tx25/u-boot.lds to the mx31pdk one.
I am not certain whether the CONFIG_SYS_TEXT_BASE should be changed or not:
ifdef CONFIG_NAND_SPL
CONFIG_SYS_TEXT_BASE = 0x87ec0000
else
CONFIG_SYS_TEXT_BASE = 0x87f00000
endif

I think they are ok - both addresses are valid RAM addresses. The relocation will move then the code at the end of the RAM. And as I understood from i.MX31 manual, the NFC controller does not use the IRAM at all, so the area pointed by your starting stack pointer should not be overwritten.

Quoted 2 lines

If anyone has any comments/suggestions I will appreciate it.

No great idea, really. Probably you need to attach a JTAG debugger and see what happens.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Fabio Estevam

author

2011-01-14 18:33 UTC #2 lore raw

Hi Stefano,

Quoted 10 lines · Stefano Babic

Quoted 1 line · Fabio Estevam
Hi,
Hi Fabio,
Quoted 4 lines
I am trying to make the new relocation scheme to work on the mx31pdk board.
With this patch applied the mx31pdk build works, but not able to get a U-boot prompt yet.

Quoted 8 lines

I have tried to imagine some issues, but I cannot find a clear reason. Building with your patch I checked that nand_spl is still less than 2KB code, and that matches the NAND page, as required by i.MX31 - so it is not a problem.
Have you tried to disable the relocation, just to check if the relocation is an issue with CONFIG_SKIP_RELOCATE_UBOOT ? This is not a solution, but only to check what happens.

Yes, I tried to disable relocation and U-boot still fails to boot.

Quoted 18 lines

Quoted 10 lines
I copied the nand_spl/board/karo/tx25/u-boot.lds to the mx31pdk one.
I am not certain whether the CONFIG_SYS_TEXT_BASE should be changed or not:
ifdef CONFIG_NAND_SPL
CONFIG_SYS_TEXT_BASE = 0x87ec0000
else
CONFIG_SYS_TEXT_BASE = 0x87f00000
endif
I think they are ok - both addresses are valid RAM addresses. The relocation will move then the code at the end of the RAM. And as I understood from i.MX31 manual, the NFC controller does not use the IRAM at all, so the area pointed by your starting stack pointer should not be overwritten.
Quoted 1 line

Quoted 4 lines

Quoted 1 line
If anyone has any comments/suggestions I will appreciate it.
No great idea, really. Probably you need to attach a JTAG debugger and see what happens.

Yes, I am setting up a OpenOCD JTAG here to debug this.

Thanks,

Fabio Estevam

Magnus Lilja

2011-02-06 12:03 UTC #3 lore raw

RFC 1/1 ARM: mx31pdk: Use the new relocation scheme

Hi all

Quoted 21 lines · Fabio Estevam

Hi Stefano,
Quoted 18 lines · Stefano Babic
Quoted 1 line · Fabio Estevam
Hi,
Hi Fabio,
Quoted 4 lines
I am trying to make the new relocation scheme to work on the mx31pdk board.
With this patch applied the mx31pdk build works, but not able to get a U-boot prompt yet.
I have tried to imagine some issues, but I cannot find a clear reason. Building with your patch I checked that nand_spl is still less than 2KB code, and that matches the NAND page, as required by i.MX31 - so it is not a problem.
Have you tried to disable the relocation, just to check if the relocation is an issue with CONFIG_SKIP_RELOCATE_UBOOT ? This is not a solution, but only to check what happens.

I've done some testing now and ended up comparing start.S for ARM1136 with ARM926EJS since the latter is used on the Karo TX25 board which seems to have been updated and also boots from NAND.

The result is that applying the following patch on top of Fabios patch makes the i.MX31 PDK boot from NAND. Booting Linux from TFTP works nicely. However the first few lines of output from U-boot is missing, only the "NAND: 256 MB" line is shown. I haven't tried to understand how booting works nowadays and why these changes work, perhaps someone with more insight in the mechanisms can figure out the correct assembler code.

I have no idea if this breaks other boards but at least it's start for further work. I tried
it on the imx31 litekit (which boots from NOR) but that board doesn't seem to boot even before applying Fabios patch, I'll see if I can figure out why that board is broken now. Don't know when though.

Regards, Magnus Lilja

arch/arm/cpu/arm1136/start.S +12 -0

index 12545c2..7d75ecf 100644
@@ -163,6 +163,9 @@ call_board_init_f:
        bic     sp, sp, #7 /* 8-byte alignment for ABI compliance */
        ldr     r0,=0x00000000
 
+#if 1
+       bl      board_init_f
+#else
 #ifdef CONFIG_NAND_SPL
        bl      nand_boot
 #else

@@ -172,6 +175,7 @@ call_board_init_f:
        bl      board_init_f
 #endif /* CONFIG_ONENAND_IPL */
 #endif /* CONFIG_NAND_SPL */
+#endif
 
 /*------------------------------------------------------------------------------*/

@@ -266,11 +270,19 @@ clbss_l:str       r2, [r0]                /* clear loop...                    */
  * initialization, now running from RAM.
  */
 #ifdef CONFIG_NAND_SPL
+#if 1
+       ldr     r0, _nand_boot_ofs
+       mov     pc, r0
+
+_nand_boot_ofs:
+       .word nand_boot
+#else
        ldr     r0, _nand_boot_ofs
        adr     r1, _start
        add     pc, r0, r1
 _nand_boot_ofs:
        .word nand_boot - _start
+#endif
 #else
 jump_2_ram:
        ldr     r0, _board_init_r_ofs

Albert ARIBAUD

2011-02-06 15:07 UTC #4 lore raw

Le 06/02/2011 13:03, Magnus Lilja a ?crit :

Quoted 83 lines

Hi all
Quoted 21 lines · Fabio Estevam
Hi Stefano,
Quoted 18 lines · Stefano Babic
Quoted 1 line · Fabio Estevam
Hi,
Hi Fabio,
Quoted 4 lines
I am trying to make the new relocation scheme to work on the mx31pdk board.
With this patch applied the mx31pdk build works, but not able to get a U-boot prompt yet.
I have tried to imagine some issues, but I cannot find a clear reason. Building with your patch I checked that nand_spl is still less than 2KB code, and that matches the NAND page, as required by i.MX31 - so it is not a problem.
Have you tried to disable the relocation, just to check if the relocation is an issue with CONFIG_SKIP_RELOCATE_UBOOT ? This is not a solution, but only to check what happens.
I've done some testing now and ended up comparing start.S for ARM1136 with ARM926EJS since the latter is used on the Karo TX25 board which seems to have been updated and also boots from NAND.
The result is that applying the following patch on top of Fabios patch makes the i.MX31 PDK boot from NAND. Booting Linux from TFTP works nicely. However the first few lines of output from U-boot is missing, only the "NAND: 256 MB" line is shown. I haven't tried to understand how booting works nowadays and why these changes work, perhaps someone with more insight in the mechanisms can figure out the correct assembler code.
I have no idea if this breaks other boards but at least it's start for further work. I tried
it on the imx31 litekit (which boots from NOR) but that board doesn't seem to boot even before applying Fabios patch, I'll see if I can figure out why that board is broken now. Don't know when though.
Regards, Magnus Lilja
arch/arm/cpu/arm1136/start.S +12 -0
index 12545c2..7d75ecf 100644
@@ -163,6 +163,9 @@ call_board_init_f:
         bic     sp, sp, #7 /* 8-byte alignment for ABI compliance */
         ldr     r0,=0x00000000
 
+#if 1
+       bl      board_init_f
+#else
  #ifdef CONFIG_NAND_SPL
         bl      nand_boot
  #else
@@ -172,6 +175,7 @@ call_board_init_f:
         bl      board_init_f
  #endif /* CONFIG_ONENAND_IPL */
  #endif /* CONFIG_NAND_SPL */
+#endif
 
  /*------------------------------------------------------------------------------*/
 
@@ -266,11 +270,19 @@ clbss_l:str       r2, [r0]                /* clear loop...                    */
   * initialization, now running from RAM.
   */
  #ifdef CONFIG_NAND_SPL
+#if 1
+       ldr     r0, _nand_boot_ofs
+       mov     pc, r0
+
+_nand_boot_ofs:
+       .word nand_boot
+#else
         ldr     r0, _nand_boot_ofs
         adr     r1, _start
         add     pc, r0, r1
  _nand_boot_ofs:
         .word nand_boot - _start
+#endif
  #else
  jump_2_ram:
         ldr     r0, _board_init_r_ofs

Quoted 4 lines

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

Can you repost the diff without those pesky '#if 1...#else...#endif' additions? Just take the current master code, do the changes needed to make it work without trying to keep the old lines around, then do the diff. I'll be easier for all to see the changes.

Amicalement,

Signature · 1 line

Albert.

Fabio Estevam

author

2011-02-07 11:32 UTC #5 lore raw

In reply to Magnus Lilja

Hi Magnus,

On 2/6/2011 10:03 AM, Magnus Lilja wrote:
...

Quoted 3 lines

I've done some testing now and ended up comparing start.S for ARM1136 with ARM926EJS since the latter is used on the Karo TX25 board which seems to have been updated and also boots from NAND.

Thanks for looking into this.

Quoted 5 lines

The result is that applying the following patch on top of Fabios patch makes the i.MX31 PDK boot from NAND. Booting Linux from TFTP works nicely. However the first few lines of output from U-boot is missing, only the "NAND: 256 MB" line is shown. I haven't tried to understand how booting works nowadays and why these changes work, perhaps someone with more insight in the mechanisms can figure out the correct assembler code.

I think that the missing lines can be fixed by using a similar approach as to this patch:
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=877eb0f91543dc5bca385bb6d22454b1d43f3e2d

Quoted 3 lines

I have no idea if this breaks other boards but at least it's start for further work. I tried
it on the imx31 litekit (which boots from NOR) but that board doesn't seem to boot even before applying Fabios patch, I'll see if I can figure out why that board is broken now. Don't know when though.

I will let others comment about the changes you did in arch/arm/cpu/arm1136/start.S Will also try it later today.

Thanks,

Fabio Estevam

Quoted 45 lines

Regards, Magnus Lilja

arch/arm/cpu/arm1136/start.S +12 -0

index 12545c2..7d75ecf 100644
@@ -163,6 +163,9 @@ call_board_init_f:
        bic     sp, sp, #7 /* 8-byte alignment for ABI compliance */
        ldr     r0,=0x00000000
 
+#if 1
+       bl      board_init_f
+#else
 #ifdef CONFIG_NAND_SPL
        bl      nand_boot
 #else

@@ -172,6 +175,7 @@ call_board_init_f:
        bl      board_init_f
 #endif /* CONFIG_ONENAND_IPL */
 #endif /* CONFIG_NAND_SPL */
+#endif
 
 /*------------------------------------------------------------------------------*/

@@ -266,11 +270,19 @@ clbss_l:str       r2, [r0]                /* clear loop...                    */
  * initialization, now running from RAM.
  */
 #ifdef CONFIG_NAND_SPL
+#if 1
+       ldr     r0, _nand_boot_ofs
+       mov     pc, r0
+
+_nand_boot_ofs:
+       .word nand_boot
+#else
        ldr     r0, _nand_boot_ofs
        adr     r1, _start
        add     pc, r0, r1
 _nand_boot_ofs:
        .word nand_boot - _start
+#endif
 #else
 jump_2_ram:
        ldr     r0, _board_init_r_ofs

Stefano Babic

2011-02-07 14:13 UTC #6 lore raw

Quoted 4 lines · Fabio Estevam

Hi Magnus,
On 2/6/2011 10:03 AM, Magnus Lilja wrote: ...

Hi,

Quoted 8 lines

Quoted 5 lines
I have no idea if this breaks other boards but at least it's start for further work. I tried it on the imx31 litekit (which boots from NOR) but that board doesn't seem to boot even before applying Fabios patch, I'll see if I can figure out why that board is broken now. Don't know when though.
I will let others comment about the changes you did in arch/arm/cpu/arm1136/start.S Will also try it later today.

Anyway, arch/arm/cpu/arm1136/start.S cannot be compiled without the changes suggested by Magnus due to the following lines:

_nand_boot_ofs
: .word nand_boot - _start

":" seems to me wrong in any case and should be fixed. Is this syntax accepted by newer compiler releases ?

IMHO board_init_f must be called absolutely before nand_boot. I have tested the changes suggested by Magnus on a i.MX35 and the board boots from NAND. However, I do not see the problems with the console, and I can think they are not related to this issue.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Magnus Lilja

2011-02-07 19:48 UTC #7 lore raw

RFC 1/1 ARM: mx31pdk: Use the new relocation scheme

Quoted 15 lines · Stefano Babic

Quoted 8 lines
Quoted 5 lines
I have no idea if this breaks other boards but at least it's start for further work. I tried it on the imx31 litekit (which boots from NOR) but that board doesn't seem to boot even before applying Fabios patch, I'll see if I can figure out why that board is broken now. Don't know when though.
I will let others comment about the changes you did in arch/arm/cpu/arm1136/start.S Will also try it later today.
Anyway, arch/arm/cpu/arm1136/start.S cannot be compiled without the changes suggested by Magnus due to the following lines:
_nand_boot_ofs
: .word nand_boot - _start

Quoted 7 lines

":" seems to me wrong in any case and should be fixed. Is this syntax accepted by newer compiler releases ?
IMHO board_init_f must be called absolutely before nand_boot. I have tested the changes suggested by Magnus on a i.MX35 and the board boots from NAND. However, I do not see the problems with the console, and I can think they are not related to this issue. >

I agree on that last statement, the console thing must be something else.

Here's a somewhat cleaner version of my patch. Hope the mail looks ok, I'm having internet connectivity issues this evening so I'm using a different installation of Thunderbird than usual.

Regards, Magnus

arch/arm/cpu/arm1136/start.S +0 -8

index 12545c2..bab2868 100644
@@ -163,15 +163,7 @@ call_board_init_f:
         bic     sp, sp, #7 /* 8-byte alignment for ABI compliance */
         ldr     r0,=0x00000000
 
-#ifdef CONFIG_NAND_SPL
-       bl      nand_boot
-#else
-#ifdef CONFIG_ONENAND_IPL
-       bl      start_oneboot
-#else
         bl      board_init_f
-#endif /* CONFIG_ONENAND_IPL */
-#endif /* CONFIG_NAND_SPL */

/*------------------------------------------------------------------------------*/

@@ -267,10 +259,10 @@ clbss_l:str r2, [r0] /* clear
loop... */
*/
#ifdef CONFIG_NAND_SPL
ldr r0, _nand_boot_ofs

-       adr     r1, _start
-       add     pc, r0, r1

-_nand_boot_ofs:
- .word nand_boot - _start
+ mov pc, r0
+
+_nand_boot_ofs:
+ .word nand_boot
#else
jump_2_ram:
ldr r0, _board_init_r_ofs

Fabio Estevam

author

2011-02-08 17:09 UTC #8 lore raw

Hi Stefano,

On 2/7/2011 5:48 PM, Magnus Lilja wrote:
...

Quoted 31 lines

Here's a somewhat cleaner version of my patch. Hope the mail looks ok, I'm having internet connectivity issues this evening so I'm using a different installation of Thunderbird than usual.

Regards, Magnus

arch/arm/cpu/arm1136/start.S +0 -9

index 12545c2..bab2868 100644
@@ -163,15 +163,7 @@ call_board_init_f:
        bic     sp, sp, #7 /* 8-byte alignment for ABI compliance */
        ldr     r0,=0x00000000
 
-#ifdef CONFIG_NAND_SPL
-       bl      nand_boot
-#else
-#ifdef CONFIG_ONENAND_IPL
-       bl      start_oneboot
-#else
        bl      board_init_f
-#endif /* CONFIG_ONENAND_IPL */
-#endif /* CONFIG_NAND_SPL */
 
 
 /*------------------------------------------------------------------------------*/

@@ -267,10 +259,10 @@ clbss_l:str       r2, [r0]                /* clear loop...                    */
  */
 #ifdef CONFIG_NAND_SPL
        ldr     r0, _nand_boot_ofs
-       adr     r1, _start

arch/arm/cpu/arm1136/start.S clbss_l:str r2, [r0] /* clear loop... */ · line 268

Quoted 10 lines

arch/arm/cpu/arm1136/start.S +4 -3

-       add     pc, r0, r1
-_nand_boot_ofs:
-       .word nand_boot - _start
+       mov     pc, r0
+
+_nand_boot_ofs:
+       .word nand_boot
 #else
 jump_2_ram:
        ldr     r0, _board_init_r_ofs

I confirmed that by applying my original patch of this thread plus Magnus? patch above I can get MX31PDK to boot.

Please let me know how you want me to proceed.

Thanks,

Fabio Estevam

stefano babic

2011-02-08 17:50 UTC #9 lore raw

Am 08.02.2011 18:09, schrieb Fabio Estevam:

Quoted 2 lines

I confirmed that by applying my original patch of this thread plus Magnus? patch above I can get MX31PDK to boot.

Fine.

Quoted 2 lines

Please let me know how you want me to proceed.

I think the correct way is that Magnus adds his Signed-off-by to his patch, repushing the patch to the list. Please put Albert in CC, as this file is competence of the ARM maintainer (well, we tested only on i.MX, I see...). I will take your patch for the mx31pdk and I will merge it on u-boot-imx (and including it in the next pull request as well).

Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Magnus Lilja

2011-02-08 19:26 UTC #10 lore raw

Quoted 7 lines

Quoted 1 line
Please let me know how you want me to proceed.
I think the correct way is that Magnus adds his Signed-off-by to his patch, repushing the patch to the list. Please put Albert in CC, as this file is competence of the ARM maintainer (well, we tested only on i.MX, I see...). I will take your patch for the mx31pdk and I will merge it on u-boot-imx (and including it in the next pull request as well).

Patch reposted as a separate mail a couple of minutes ago.

As I mention in the patch I think Fabio's patch has to be applied first. Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files. I cannot work on that before Thursday or Friday though.

Regards, Magnus Lilja

stefano babic

2011-02-08 20:18 UTC #11 lore raw

Am 08.02.2011 20:26, schrieb Magnus Lilja:

Quoted 3 lines

Patch reposted as a separate mail a couple of minutes ago.
As I mention in the patch I think Fabio's patch has to be applied first.

I think your patch is ok - Fabio fixed the syntax error as you do. We need only one of them.

Quoted 3 lines

Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files.

IMHO this is the preferred way, because the two issues are orthogonal. Your patch fixes booting from NAND for ARM11, and Fabio's patch fix the mx31pdk board only.

Regards,
Stefano

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

Albert ARIBAUD

2011-02-08 20:50 UTC #12 lore raw

Le 08/02/2011 21:18, stefano babic a ?crit :

Quoted 8 lines

Am 08.02.2011 20:26, schrieb Magnus Lilja:
Quoted 3 lines
Patch reposted as a separate mail a couple of minutes ago.
As I mention in the patch I think Fabio's patch has to be applied first.
I think your patch is ok - Fabio fixed the syntax error as you do. We need only one of them.

Quoted 7 lines

Quoted 3 lines
Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files.
IMHO this is the preferred way, because the two issues are orthogonal. Your patch fixes booting from NAND for ARM11, and Fabio's patch fix the mx31pdk board only.

Agreed.

Note also that there was a recent patch to ARM926's start.S (replacing 'adr r1, _start' with 'ldr r1, _TEXT_BASE' at line 284). The same should be done on arm1136.

Quoted 2 lines

Regards,
Stefano

Amicalement,

Signature · 1 line

Albert.

Fabio Estevam

author

2011-02-09 11:45 UTC #13 lore raw

Hi Albert,

Quoted 13 lines · Albert ARIBAUD

Le 08/02/2011 21:18, stefano babic a ?crit :
Quoted 12 lines
Am 08.02.2011 20:26, schrieb Magnus Lilja:
Quoted 3 lines
Patch reposted as a separate mail a couple of minutes ago.
As I mention in the patch I think Fabio's patch has to be applied first.
I think your patch is ok - Fabio fixed the syntax error as you do. We need only one of them.
Quoted 3 lines
Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files.

Quoted 5 lines

Quoted 3 lines
IMHO this is the preferred way, because the two issues are orthogonal. Your patch fixes booting from NAND for ARM11, and Fabio's patch fix the mx31pdk board only.
Agreed.

Ok, I have just submitted the patch series treating one issue at the time.

Quoted 1 line

Note also that there was a recent patch to ARM926's start.S (replacing 'adr r1, _start' with 'ldr r1, _TEXT_BASE' at line 284). The same should be done on arm1136.

Ok, I haven?t added this change to my patch series yet, but I can do it on a separate patch after my original patch series is applied, if this is OK with you.

I have tested this recommended change and it worked fine on my mx31pdk board.

Thanks,

Fabio Estevam

Aneesh V

2011-02-11 10:51 UTC #14 lore raw

In reply to Albert ARIBAUD

Hi Albert,

Quoted 17 lines · Albert ARIBAUD

Le 08/02/2011 21:18, stefano babic a ?crit :
Quoted 8 lines
Am 08.02.2011 20:26, schrieb Magnus Lilja:
Quoted 3 lines
Patch reposted as a separate mail a couple of minutes ago.
As I mention in the patch I think Fabio's patch has to be applied first.
I think your patch is ok - Fabio fixed the syntax error as you do. We need only one of them.
Quoted 7 lines
Quoted 3 lines
Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files.
IMHO this is the preferred way, because the two issues are orthogonal. Your patch fixes booting from NAND for ARM11, and Fabio's patch fix the mx31pdk board only.

Quoted 5 lines

Agreed.
Note also that there was a recent patch to ARM926's start.S (replacing 'adr r1, _start' with 'ldr r1, _TEXT_BASE' at line 284). The same should be done on arm1136.

Is this going to happen for armv7 too? What is the real reason behind this proposal. What is the case when _start is not same as _TEXT_BASE(I looked at the archives but couldn't filter out the original discussion on this)

I see a problem with that. _TEXT_BASE is based on
CONFIG_SYS_TEXT_BASE. In our SPL's case CONFIG_SYS_TEXT_BASE indicates the TEXT_BASE for u-boot and *CONFIG_SYS_SPL_TEXT_BASE* indicates the TEXT_BASE for SPL. Both are defined and useful in SPL because one is used for linking SPL while the other is used while loading u-boot from MMC. So, CONFIG_SYS_TEXT_BASE used in the start.S of SPL will not be correct.

In the worst case we need to define yet another label in the linker scripts like __text_base. But I was wondering if we could maintain the status quo for armv7: that is 'adr r1, _start'

Best regards,
Aneesh

Albert ARIBAUD

2011-02-11 12:46 UTC #15 lore raw

Le 11/02/2011 11:51, Aneesh V a ?crit :

Quoted 23 lines

Hi Albert,
Quoted 20 lines · Albert ARIBAUD
Le 08/02/2011 21:18, stefano babic a ?crit :
Quoted 9 lines
Am 08.02.2011 20:26, schrieb Magnus Lilja:
Quoted 4 lines
Patch reposted as a separate mail a couple of minutes ago.
As I mention in the patch I think Fabio's patch has to be applied first.
I think your patch is ok - Fabio fixed the syntax error as you do. We need only one of them.
Quoted 7 lines
Quoted 3 lines
Another solution would be to change my patch somewhat to apply it first and then update Fabios patch to only touch the i.MX31-PDK specific files.
IMHO this is the preferred way, because the two issues are orthogonal. Your patch fixes booting from NAND for ARM11, and Fabio's patch fix the mx31pdk board only.
Agreed.

Quoted 8 lines

Quoted 3 lines
Note also that there was a recent patch to ARM926's start.S (replacing 'adr r1, _start' with 'ldr r1, _TEXT_BASE' at line 284). The same should be done on arm1136.
Is this going to happen for armv7 too? What is the real reason behind this proposal. What is the case when _start is not same as _TEXT_BASE(I looked at the archives but couldn't filter out the original discussion on this)

The difference is that _TEXT_BASE always contains the link-time address of _start, whereas references to _start may contain a different value if the code is executed somewhere else than at the link-time address.

/Normally/, u-boot should always execute first at the link-time address -- that's a base constraint.

/But/ this change makes it more resilient to out-of-link-time-address execution, and I want, at some time in the future, to find a way for u-boot to be able to start anywhere -- within reasonable limits:
anywhere in NOR for a NOR-based U-boot, anywhere in RAM for a RAM-based U-boot, but I am not talking about a generic, run-in-RAM-or-NOR-or-anywhere, binary.

Yet. :)

Quoted 7 lines

I see a problem with that. _TEXT_BASE is based on CONFIG_SYS_TEXT_BASE. In our SPL's case CONFIG_SYS_TEXT_BASE indicates the TEXT_BASE for u-boot and *CONFIG_SYS_SPL_TEXT_BASE* indicates the TEXT_BASE for SPL. Both are defined and useful in SPL because one is used for linking SPL while the other is used while loading u-boot from MMC. So, CONFIG_SYS_TEXT_BASE used in the start.S of SPL will not be correct.

The change I indicate is under the #else of a #ifdef CONFIG_NAND_SPL, so it will not apply to SPL. Does that still cause an issue with armv7?

Quoted 3 lines

In the worst case we need to define yet another label in the linker scripts like __text_base. But I was wondering if we could maintain the status quo for armv7: that is 'adr r1, _start'

As long as you run the u-boot start code at the link-time address, there will be no difference except the code is more correct with respect to what it should do; and if you run it elsewhere, which you should not, you have slightly better chances that it manages to survive.

Quoted 2 lines

Best regards,
Aneesh

Amicalement,

Signature · 1 line

Albert.

Aneesh V

2011-02-11 13:49 UTC #16 lore raw

On Friday 11 February 2011 06:16 PM, Albert ARIBAUD wrote:
[snip...]

Quoted 34 lines

Quoted 9 lines
Quoted 4 lines
Note also that there was a recent patch to ARM926's start.S (replacing 'adr r1, _start' with 'ldr r1, _TEXT_BASE' at line 284). The same should be done on arm1136.
Is this going to happen for armv7 too? What is the real reason behind this proposal. What is the case when _start is not same as _TEXT_BASE(I looked at the archives but couldn't filter out the original discussion on this)
The difference is that _TEXT_BASE always contains the link-time address of _start, whereas references to _start may contain a different value if the code is executed somewhere else than at the link-time address.
/Normally/, u-boot should always execute first at the link-time address -- that's a base constraint.
/But/ this change makes it more resilient to out-of-link-time-address execution, and I want, at some time in the future, to find a way for u-boot to be able to start anywhere -- within reasonable limits:
anywhere in NOR for a NOR-based U-boot, anywhere in RAM for a RAM-based U-boot, but I am not talking about a generic, run-in-RAM-or-NOR-or-anywhere, binary.
Yet. :)
Quoted 7 lines
I see a problem with that. _TEXT_BASE is based on CONFIG_SYS_TEXT_BASE. In our SPL's case CONFIG_SYS_TEXT_BASE indicates the TEXT_BASE for u-boot and *CONFIG_SYS_SPL_TEXT_BASE* indicates the TEXT_BASE for SPL. Both are defined and useful in SPL because one is used for linking SPL while the other is used while loading u-boot from MMC. So, CONFIG_SYS_TEXT_BASE used in the start.S of SPL will not be correct.

Quoted 2 lines

The change I indicate is under the #else of a #ifdef CONFIG_NAND_SPL, so it will not apply to SPL. Does that still cause an issue with armv7?

No. It doesn't. I am fine with this change if it applies only to u-boot.

br,
Aneesh