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

[Minkyu Kang]

Dear Seunghyeon Rhee,

2009/11/10 Minkyu Kang <promsoft at gmail.com>:
> Dear Seunghyeon Rhee,
>
> 2009/11/2 "Seunghyeon Rhee (이승현)" <seunghyeon at lpmtec.com>:
>> Dear Minkyu Kang,
>>
>> 2009/11/2 Minkyu Kang <promsoft at gmail.com>:
>>> Dear Seunghyeon Rhee
>>>
>>> 2009/10/31 "Seunghyeon Rhee (이승현)" <seunghyeon at 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 at 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 at 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
-- 
from. prom.
www.promsoft.net