Re: [U-Boot] [PATCH] S3C6400/SMDK6400: fix stack_setup in start.S
Dear Minkyu Kang,
2009/11/2 Minkyu Kang promsoft@gmail.com:
Dear Seunghyeon Rhee
2009/10/31 "Seunghyeon Rhee (이승현)" seunghyeon@lpmtec.com:
stack_setup is modified to initialize the stack on the correct address in DRAM accroding to the typical memory configuration described in README and the related CONFIG_* macro definitions. This makes macro CONFIG_MEMORY_UPPER_CODE no longer necessry. This was introduced and used only by this board for some unclear reason. The definition of this macro is removed because it's not referenced elsewhere.
Signed-off-by: Seunghyeon Rhee seunghyeon@lpmtec.com
cpu/arm1176/start.S | 7 +------ include/configs/smdk6400.h | 2 -- 2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index cb891df..1ecb3b9 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -241,16 +241,11 @@ mmu_enable: skip_hw_init: /* Set up the stack */ stack_setup: -#ifdef CONFIG_MEMORY_UPPER_CODE
- ldr sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc)
-#else
- ldr r0, _TEXT_BASE /* upper 128 KiB: relocated uboot */
- ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in
DRAM */
this change is unnecessary, TEXT_BASE and CONFIG_SYS_UBOOT_BASE must
be same.
That's true for the body of U-Boot but not for SPL, where TEXT_BASE is defined to be '0'. Please refer to board/samsung/smdk6400/config.mk. On the other hand, CONFIG_SYS_UBOOT_BASE is always dependent on DRAM's base. In SPL, the base of the code should be '0' (the steppingstone memory) and then the stack is located below '0' - not a valid area. If SPL itself requires no stack, it should be no problem. But start.S calls nand_boot function right after the stack is badly set up in the air.
My test results are like the following: with CONFIG_MEMORY_UPPER_CODE defined : OK with CONFIG_MEMORY_UPPER_CODE undefined : - SPL bypassed (U-Boot downloaded to DRAM directly by USB monitor program) : OK - through SPL : Not OK (seems to fail for SPL downloading the code to DRAM)
I think CONFIG_MEMORY_UPPER_CODE was tested for the case it's defined but not enough for the case it's not defined. Would you check it again?
btw, is there need CONFIG_SYS_UBOOT_BASE define?
If you are not sure, why did you use CONFIG_SYS_UBOOT_BASE for the case CONFIG_MEMORY_UPPER_CODE is defined while you use TEXT_BASE otherwise? I think the unnecessary macro definition here is not CONFIG_..._BASE but CONFIG_..._CODE. TEXT_BASE and CONFIG_..._BASE have their own meanings and so both are necessary.
Best regards, Seunghyeon
sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* mallocarea */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack */
-#endif
clear_bss: ldr r0, _bss_start /* find start of bss segment */ ldr r1, _bss_end /* stop here */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f6e1221..f644cd2 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -49,8 +49,6 @@ #define CONFIG_ENABLE_MMU #endif
-#define CONFIG_MEMORY_UPPER_CODE
#define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_CMDLINE_TAG
#define CONFIG_INITRD_TAG
1.6.2.5
-- Seunghyeon Rhee, Ph.D. / Director LPM Technology Inc. T +82-70-8255-6007 F +82-2-6442-6462 M +82-10-2790-0657 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks for patch :) Minkyu Kang -- from. prom. www.promsoft.net _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Seunghyeon Rhee,
2009/11/2 "Seunghyeon Rhee (이승현)" seunghyeon@lpmtec.com:
Dear Minkyu Kang,
2009/11/2 Minkyu Kang promsoft@gmail.com:
Dear Seunghyeon Rhee
2009/10/31 "Seunghyeon Rhee (이승현)" seunghyeon@lpmtec.com:
stack_setup is modified to initialize the stack on the correct address in DRAM accroding to the typical memory configuration described in README and the related CONFIG_* macro definitions. This makes macro CONFIG_MEMORY_UPPER_CODE no longer necessry. This was introduced and used only by this board for some unclear reason. The definition of this macro is removed because it's not referenced elsewhere.
Signed-off-by: Seunghyeon Rhee seunghyeon@lpmtec.com
cpu/arm1176/start.S | 7 +------ include/configs/smdk6400.h | 2 -- 2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index cb891df..1ecb3b9 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -241,16 +241,11 @@ mmu_enable: skip_hw_init: /* Set up the stack */ stack_setup: -#ifdef CONFIG_MEMORY_UPPER_CODE
- ldr sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc)
-#else
- ldr r0, _TEXT_BASE /* upper 128 KiB: relocated uboot */
- ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in
DRAM */
this change is unnecessary, TEXT_BASE and CONFIG_SYS_UBOOT_BASE must
be same.
That's true for the body of U-Boot but not for SPL, where TEXT_BASE is defined to be '0'. Please refer to board/samsung/smdk6400/config.mk. On the other hand, CONFIG_SYS_UBOOT_BASE is always dependent on DRAM's base. In SPL, the base of the code should be '0' (the steppingstone memory) and then the stack is located below '0' - not a valid area. If SPL itself requires no stack, it should be no problem. But start.S calls nand_boot function right after the stack is badly set up in the air.
My test results are like the following: with CONFIG_MEMORY_UPPER_CODE defined : OK with CONFIG_MEMORY_UPPER_CODE undefined : - SPL bypassed (U-Boot downloaded to DRAM directly by USB monitor program) : OK - through SPL : Not OK (seems to fail for SPL downloading the code to DRAM)
I think CONFIG_MEMORY_UPPER_CODE was tested for the case it's defined but not enough for the case it's not defined. Would you check it again?
btw, is there need CONFIG_SYS_UBOOT_BASE define?
If you are not sure, why did you use CONFIG_SYS_UBOOT_BASE for the case CONFIG_MEMORY_UPPER_CODE is defined while you use TEXT_BASE otherwise? I think the unnecessary macro definition here is not CONFIG_..._BASE but CONFIG_..._CODE. TEXT_BASE and CONFIG_..._BASE have their own meanings and so both are necessary.
Best regards, Seunghyeon
sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* malloc area */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack */
-#endif
clear_bss: ldr r0, _bss_start /* find start of bss segment */ ldr r1, _bss_end /* stop here */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f6e1221..f644cd2 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -49,8 +49,6 @@ #define CONFIG_ENABLE_MMU #endif
-#define CONFIG_MEMORY_UPPER_CODE
#define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_CMDLINE_TAG #define CONFIG_INITRD_TAG -- 1.6.2.5
-- Seunghyeon Rhee, Ph.D. / Director LPM Technology Inc. T +82-70-8255-6007 F +82-2-6442-6462 M +82-10-2790-0657 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks for patch :) Minkyu Kang -- from. prom. www.promsoft.net _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Seunghyeon Rhee, Ph.D. / Director LPM Technology Inc. T +82-70-8255-6007 F +82-2-6442-6462 M +82-10-2790-0657 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Tested on the NCP board (s3c6410), and It works fine. I'll wait few days more for this patch's review. Thanks!
Tested-by: Minkyu Kang mk7.kang@samsung.com
Minkyu Kang
Dear Seunghyeon Rhee,
2009/11/10 Minkyu Kang promsoft@gmail.com:
Dear Seunghyeon Rhee,
2009/11/2 "Seunghyeon Rhee (이승현)" seunghyeon@lpmtec.com:
Dear Minkyu Kang,
2009/11/2 Minkyu Kang promsoft@gmail.com:
Dear Seunghyeon Rhee
2009/10/31 "Seunghyeon Rhee (이승현)" seunghyeon@lpmtec.com:
stack_setup is modified to initialize the stack on the correct address in DRAM accroding to the typical memory configuration described in README and the related CONFIG_* macro definitions. This makes macro CONFIG_MEMORY_UPPER_CODE no longer necessry. This was introduced and used only by this board for some unclear reason. The definition of this macro is removed because it's not referenced elsewhere.
Signed-off-by: Seunghyeon Rhee seunghyeon@lpmtec.com
cpu/arm1176/start.S | 7 +------ include/configs/smdk6400.h | 2 -- 2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index cb891df..1ecb3b9 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -241,16 +241,11 @@ mmu_enable: skip_hw_init: /* Set up the stack */ stack_setup: -#ifdef CONFIG_MEMORY_UPPER_CODE
- ldr sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc)
-#else
- ldr r0, _TEXT_BASE /* upper 128 KiB: relocated uboot */
- ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in
DRAM */
this change is unnecessary, TEXT_BASE and CONFIG_SYS_UBOOT_BASE must
be same.
That's true for the body of U-Boot but not for SPL, where TEXT_BASE is defined to be '0'. Please refer to board/samsung/smdk6400/config.mk. On the other hand, CONFIG_SYS_UBOOT_BASE is always dependent on DRAM's base. In SPL, the base of the code should be '0' (the steppingstone memory) and then the stack is located below '0' - not a valid area. If SPL itself requires no stack, it should be no problem. But start.S calls nand_boot function right after the stack is badly set up in the air.
My test results are like the following: with CONFIG_MEMORY_UPPER_CODE defined : OK with CONFIG_MEMORY_UPPER_CODE undefined : - SPL bypassed (U-Boot downloaded to DRAM directly by USB monitor program) : OK - through SPL : Not OK (seems to fail for SPL downloading the code to DRAM)
I think CONFIG_MEMORY_UPPER_CODE was tested for the case it's defined but not enough for the case it's not defined. Would you check it again?
btw, is there need CONFIG_SYS_UBOOT_BASE define?
If you are not sure, why did you use CONFIG_SYS_UBOOT_BASE for the case CONFIG_MEMORY_UPPER_CODE is defined while you use TEXT_BASE otherwise? I think the unnecessary macro definition here is not CONFIG_..._BASE but CONFIG_..._CODE. TEXT_BASE and CONFIG_..._BASE have their own meanings and so both are necessary.
Best regards, Seunghyeon
sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* malloc area */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack */
-#endif
clear_bss: ldr r0, _bss_start /* find start of bss segment */ ldr r1, _bss_end /* stop here */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f6e1221..f644cd2 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -49,8 +49,6 @@ #define CONFIG_ENABLE_MMU #endif
-#define CONFIG_MEMORY_UPPER_CODE
#define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_CMDLINE_TAG #define CONFIG_INITRD_TAG -- 1.6.2.5
-- Seunghyeon Rhee, Ph.D. / Director LPM Technology Inc. T +82-70-8255-6007 F +82-2-6442-6462 M +82-10-2790-0657 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks for patch :) Minkyu Kang -- from. prom. www.promsoft.net _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
-- Seunghyeon Rhee, Ph.D. / Director LPM Technology Inc. T +82-70-8255-6007 F +82-2-6442-6462 M +82-10-2790-0657 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Tested on the NCP board (s3c6410), and It works fine. I'll wait few days more for this patch's review. Thanks!
Tested-by: Minkyu Kang mk7.kang@samsung.com
Minkyu Kang
from. prom. www.promsoft.net
your patch is line wrapped and the tabs are stripped out. please resend the patch
Thanks Minkyu Kang
Fix stack_setup to place the stack on the correct address in DRAM accroding to U-Boot standard and remove conditional compilation by CONFIG_MEMORY_UPPER_CODE macro that is not necessry. This macro was introduced and used only by this board for some unclear reason.
The definition of this macro is also removed because it's not referenced elsewhere.
Signed-off-by: Seunghyeon Rhee seunghyeon@lpmtec.com --- cpu/arm1176/start.S | 7 +------ include/configs/smdk6400.h | 2 -- 2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index cb891df..1ecb3b9 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -241,16 +241,11 @@ mmu_enable: skip_hw_init: /* Set up the stack */ stack_setup: -#ifdef CONFIG_MEMORY_UPPER_CODE - ldr sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc) -#else - ldr r0, _TEXT_BASE /* upper 128 KiB: relocated uboot */ + ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in DRAM */ sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* malloc area */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack */
-#endif - clear_bss: ldr r0, _bss_start /* find start of bss segment */ ldr r1, _bss_end /* stop here */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f6e1221..f644cd2 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -49,8 +49,6 @@ #define CONFIG_ENABLE_MMU #endif
-#define CONFIG_MEMORY_UPPER_CODE - #define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_CMDLINE_TAG #define CONFIG_INITRD_TAG
Dear Seunghyeon Rhee,
2009/11/13 이승현 rhee4j1@gmail.com:
Fix stack_setup to place the stack on the correct address in DRAM accroding to U-Boot standard and remove conditional compilation by CONFIG_MEMORY_UPPER_CODE macro that is not necessry. This macro was introduced and used only by this board for some unclear reason.
The definition of this macro is also removed because it's not referenced elsewhere.
Signed-off-by: Seunghyeon Rhee seunghyeon@lpmtec.com
cpu/arm1176/start.S | 7 +------ include/configs/smdk6400.h | 2 -- 2 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/cpu/arm1176/start.S b/cpu/arm1176/start.S index cb891df..1ecb3b9 100644 --- a/cpu/arm1176/start.S +++ b/cpu/arm1176/start.S @@ -241,16 +241,11 @@ mmu_enable: skip_hw_init: /* Set up the stack */ stack_setup: -#ifdef CONFIG_MEMORY_UPPER_CODE
- ldr sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc)
-#else
- ldr r0, _TEXT_BASE /* upper 128 KiB: relocated uboot */
- ldr r0, =CONFIG_SYS_UBOOT_BASE /* base of copy in DRAM */
sub r0, r0, #CONFIG_SYS_MALLOC_LEN /* malloc area */ sub r0, r0, #CONFIG_SYS_GBL_DATA_SIZE /* bdinfo */ sub sp, r0, #12 /* leave 3 words for abort-stack */
-#endif
clear_bss: ldr r0, _bss_start /* find start of bss segment */ ldr r1, _bss_end /* stop here */ diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h index f6e1221..f644cd2 100644 --- a/include/configs/smdk6400.h +++ b/include/configs/smdk6400.h @@ -49,8 +49,6 @@ #define CONFIG_ENABLE_MMU #endif
-#define CONFIG_MEMORY_UPPER_CODE
#define CONFIG_SETUP_MEMORY_TAGS #define CONFIG_CMDLINE_TAG #define CONFIG_INITRD_TAG -- 1.6.2.5
applied to u-boot-samsung
At next time, if you resend the patch, please send next version patch.
Thanks Minkyu Kang
participants (3)
-
"Seunghyeon Rhee (이승현)" -
Minkyu Kang -
이승현