From mboxrd@z Thu Jan  1 00:00:00 1970
From: Macpaul Lin <macpaul@gmail.com>
Date: Mon, 2 May 2011 23:37:49 +0800
Subject: [U-Boot] [PATCH v9 05/10] nds32/ag101: lowlevel_init.S of ag101
In-Reply-To: <20110502152700.53274BB0D9@gemini.denx.de>
References: <1302180333-25372-1-git-send-email-macpaul@andestech.com>
	<1304342712-17120-5-git-send-email-macpaul@andestech.com>
	<20110502152700.53274BB0D9@gemini.denx.de>
Message-ID: <BANLkTimuJunzvthhx6+s7FRL-9PW8WM1rA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Wolfgang,

> Patch changelog belongs _below_ the "---" line, not above.

Sorry for this mistaken, will fix in Patch v10

>> ?arch/nds32/cpu/n1213/ag101/lowlevel_init.S | ?160 ++++++++++++++++++++++++++++
>> ?1 files changed, 160 insertions(+), 0 deletions(-)
>> ?create mode 100644 arch/nds32/cpu/n1213/ag101/lowlevel_init.S
>
> This file alone makes no sense. ?Pluease submit as a single commit
> together with the other board code.

The single file is intend to be reviewed and fix up easyly because I
know there must be some problem of this lowlevel_init.S.
I think this should go with the SoC. So I will merge it into ag101 SoC
level related code.

> ...
>> + ? ? /*
>> + ? ? ?* relocation, copy ROM code to SDRAM (current at 0x10000000)
>> + ? ? ?*/
>> + ? ? li ? ? ?$r4, CONFIG_SYS_RELO_ADDR ? ? ? ? ? ? ? ! 0x10000000
>> + ? ? li ? ? ?$r5, 0x0
>> + ? ? la ? ? ?$r1, relo_base
>> + ? ? sub ? ? $r2, $r0, $r1
>> + ? ? sethi ? $r6, hi20(andesboot_end)
>> + ? ? ori ? ? $r6, $r6, lo12(andesboot_end)
>> + ? ? add ? ? $r6, $r6, $r2
>> +1:
>> + ? ? lwi ? ? $r7, [$r5]
>> + ? ? swi ? ? $r7, [$r4]
>> + ? ? addi ? ?$r5, $r5, #4
>> + ? ? addi ? ?$r4, $r4, #4
>> + ? ? blt ? ? $r5, $r6, 1b
>
> You are using ?non-standard and completely undocumented CONFIG option
> here, CONFIG_SYS_RELO_ADDR. ?Please don't. ?I think when adding new
> architecture support we should avaid errors done with other
> architectures in the past (see ARM, MIPS, ...).

Sure.

> Please implement proper relocation to a dynamically determined
> address at the upper end of RAM. Keep in mind that the intention is
> to have only a single lib/board.c isnatead of a collection of
> slightly different arch-specific arch/*/board.c versions.
>
> Best regards,
>
> Wolfgang Denk

Will try to fix it soon.
Thanks.

-- 
Best regards,
Macpaul Lin