[U-Boot] [PATCH 06/10] ARM Add New Board GEC2410

[Wolfgang Denk]

Dear "Hui.Tang",

In message <28c8c0b4de93d8f48e61ff560c4537768b4e1455.1256898456.git.zetalabs at gmail.com> you wrote:
> GEC2410 Boot from NAND FLASH.
> 
> Signed-off-by: Hui.Tang <zetalabs at gmail.com>
> ---
>  cpu/arm920t/start.S |   36 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/cpu/arm920t/start.S b/cpu/arm920t/start.S
> index 114427a..d8b832a 100644
> --- a/cpu/arm920t/start.S
> +++ b/cpu/arm920t/start.S
> @@ -27,6 +27,10 @@
>  #include <common.h>
>  #include <config.h>
>  
> +#if !defined(CONFIG_ENABLE_MMU) && !defined(CONFIG_SYS_PHY_UBOOT_BASE)
> +#define CONFIG_SYS_PHY_UBOOT_BASE	CONFIG_SYS_UBOOT_BASE
> +#endif

Please don't add such defines to global code. Also, CONFIG_SYS_*
variables get set in board config files, not in some low level source
code.

> +/*
> + * Below variable is very important because we use MMU in U-Boot.

do we?

> + * Without it, we cannot run code correctly before MMU is ON.
> + * by scsuh.

What does "by scsuh." mean?

> +_TEXT_PHY_BASE:
> +	.word	CONFIG_SYS_PHY_UBOOT_BASE

Why would that be needed?

> +#ifndef CONFIG_NAND_SPL
>  	bl	coloured_LED_init
>  	bl	red_LED_on
> +#endif

in which way do LED's depend on NAND booting?

> +#ifndef CONFIG_NAND_SPL
>  #ifndef CONFIG_SKIP_RELOCATE_UBOOT
>  relocate:				/* relocate U-Boot to RAM	    */
>  	adr	r0, _start		/* r0 <- current position of code   */
> @@ -194,9 +216,13 @@ copy_loop:
>  	cmp	r0, r2			/* until source end addreee [r2]    */
>  	ble	copy_loop
>  #endif	/* CONFIG_SKIP_RELOCATE_UBOOT */
> +#endif	/* CONFIG_NAND_SPL */
>  
>  	/* Set up the stack						    */
>  stack_setup:
> +#ifdef CONFIG_MEMORY_UPPER_CODE
> +	ldr	sp, =(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE - 0xc)
> +#else

I don;t really like this growing amount of #ifdefs in this global
code. Isn't there some better way to implement this?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Far back in the mists of ancient time, in the great and glorious days
of the former Galactic Empire, life was wild, rich  and  largely  tax
free.         - Douglas Adams, _The Hitchhiker's Guide to the Galaxy_