[U-Boot] [PATCH] S3C6400/SMDK6400: fix stack_setup in start.S

Minkyu Kang
2009-11-13T07:35:02Z

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 at 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 at 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 at 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