[U-Boot,v9,05/10] nds32/ag101: lowlevel_init.S of ag101

lowlevel_init.S is a peripheral initial procedure of ag101.
It configures onboard dram, clock, and power settings.
It also prepars the dram environment before moving u-boot
from rom and flash into dram.

This version of lowlevel_init.S also replace hardcode value
by MARCO defines from the GPL version andesboot for better
code quality.

Signed-off-by: Macpaul Lin <macpaul@andestech.com>

Changes from v1-v4:
  - Code clean up and formatting style.
Changes from v5-v6
  - Change hard code value into MARCO definitions.
  - ftsmc010
    - Fix FTSMC020_TPR_AT2 from 1 to 3 (0xff3ff)
  - ftsdmc021
    - Fix hardcoded address of CR1, CR2, TR1, TR2, BANK0 registers.
    - Fix the default configuration value of FTSDMC and FTSMC controller.
  - Remove some ftpmu010 and flash probe code to C functions.
Changes for v7-v9:
  - No change.
---
 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

Dear Macpaul Lin,

In message <1304342712-17120-5-git-send-email-macpaul@andestech.com> you wrote:
> lowlevel_init.S is a peripheral initial procedure of ag101.
> It configures onboard dram, clock, and power settings.
> It also prepars the dram environment before moving u-boot
> from rom and flash into dram.
> 
> This version of lowlevel_init.S also replace hardcode value
> by MARCO defines from the GPL version andesboot for better
> code quality.
> 
> Signed-off-by: Macpaul Lin <macpaul@andestech.com>
> 
> Changes from v1-v4:
>   - Code clean up and formatting style.
> Changes from v5-v6
>   - Change hard code value into MARCO definitions.
>   - ftsmc010
>     - Fix FTSMC020_TPR_AT2 from 1 to 3 (0xff3ff)
>   - ftsdmc021
>     - Fix hardcoded address of CR1, CR2, TR1, TR2, BANK0 registers.
>     - Fix the default configuration value of FTSDMC and FTSMC controller.
>   - Remove some ftpmu010 and flash probe code to C functions.
> Changes for v7-v9:
>   - No change.
> ---

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

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

...
> +	/*
> +	 * 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, ...).

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

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.