[U-Boot] [PATCH] MIPS: start.S: unify and simplify reset vector handling

[Daniel Schwierzeck]

2013/1/31 Gabor Juhos <juhosg at openwrt.org>:
> Hi Daniel,
>
>> From: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>>
>> Adopt reset vector handling from Yamon.
>>
>> Signed-off-by: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>> ---
>>  arch/mips/cpu/mips32/start.S | 185 +++++++++----------------------------------
>>  arch/mips/cpu/mips64/start.S |  55 +++++++------
>>  2 files changed, 64 insertions(+), 176 deletions(-)
>>
>> diff --git a/arch/mips/cpu/mips32/start.S b/arch/mips/cpu/mips32/start.S
>> index 65acf7d..a7cd2c4 100644
>> --- a/arch/mips/cpu/mips32/start.S
>> +++ b/arch/mips/cpu/mips32/start.S
>> @@ -47,19 +47,16 @@
>>       .set    pop
>>       .endm
>>
>> -#define RVECENT(f,n) \
>> -   b f; nop
>> -#define XVECENT(f,bev) \
>> -   b f     ;           \
>> -   li k0,bev
>> -
>>       .set noreorder
>>
>>       .globl _start
>>       .text
>>  _start:
>> -     RVECENT(reset,0)                        # U-boot entry point
>> -     RVECENT(reset,1)                        # software reboot
>> +     /* U-boot entry point */
>> +     b       reset
>> +      nop
>> +      nop
>> +      nop
>>  #ifdef CONFIG_SYS_XWAY_EBU_BOOTCFG
>
> We should use '.org 0x010' here, that would make it it consistent with the rest
> of the code. Additionally, that would make it possible to remove the last two
> 'nop' instructions before the ifdef.

ok, fixed

>
>>       /*
>>        * Almost all Lantiq XWAY SoC devices have an external bus unit (EBU) to
>> @@ -71,139 +68,40 @@ _start:
>>       .word CONFIG_SYS_XWAY_EBU_BOOTCFG
>>       .word 0x00000000
>>  #else
>> -     RVECENT(romReserved,2)
>> +     .word 0xffffffff
>> +     .word 0xffffffff
>
> I assume that Yamon uses these two words to ensure that there will be no
> instructions at this address because this overlaps with the REVISION register on
> the MIPS boards. U-Boot does not support MIPS development boards yet, so
> strictly speaking we don't need this at the moment.

ok, I only init the words for Lantiq SoC's and let the assembler fill
the gap with zeroes

>
>>  #endif
>
> <...>
>
>> -     /*
>> -      * We hope there are no more reserved vectors!
>> -      * 128 * 8 == 1024 == 0x400
>> -      * so this is address R_VEC+0x400 == 0xbfc00400
>> -      */
>> +.org 0x200
>
> The indentation of this looks weird. All other pseudo op-codes are preceded by a
> tab. It would be more consistent to add a tab before each '.org'.

yes, fixed

>
>> +     /* TLB refill, 32 bit task */
>> +1:   b       1b
>> +      nop
>
> <...>
>
>> diff --git a/arch/mips/cpu/mips64/start.S b/arch/mips/cpu/mips64/start.S
>> index 1522594..0628211 100644
>> --- a/arch/mips/cpu/mips64/start.S
>> +++ b/arch/mips/cpu/mips64/start.S
>> @@ -52,40 +52,44 @@
>>       .globl _start
>>       .text
>>  _start:
>> -     .org 0x000
>> +     /* U-boot entry point */
>>       b       reset
>>        nop
>> -     .org 0x080
>> -     b       romReserved
>>        nop
>> -     .org 0x100
>> -     b       romReserved
>>        nop
>> -     .org 0x180
>> -     b       romReserved
>> +     .word 0xffffffff
>> +     .word 0xffffffff
>
> These two words and the preceding two nop instructions should be removed as well.
>
>> +
>> +.org 0x200
>> +     /* TLB refill, 32 bit task */
>> +1:   b       1b
>>        nop
>> -     .org 0x200
>
> The previous comment for the '.org' op-codes applies here as well. Even the
> patch would be a bit simpler if you would not remove the leading tabs.
>
> -Gabor

thanks for the comments

-- 
Best regards,
Daniel