[U-Boot] [PATCH] Marvell Sheevaplug Board support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1242937472-30778-2-git-send-email-prafulla at marvell.com> you wrote:
> References: http://plugcomputer.org/
> Serial console Setup
> http://openplug.org/plugwiki/index.php/Serial_terminal_program#Linux
> OpenOCD Setup
> http://openplug.org/plugwiki/index.php/Setting_Up_OpenOCD_Under_Linux
> 
> This patch is tested for-
> 1. Boot from DRAM/NAND flash
> 2. File transfer using tftp
> 3. NAND flash read/write/erase
...

> diff --git a/MAKEALL b/MAKEALL
> index 6719d7b..e3d33cb 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -144,6 +144,7 @@ LIST_8xx="		\
>  	RPXlite		\
>  	RPXlite_DW	\
>  	RRvision	\
> +	sheevaplug	\
>  	SM850		\

Why do you add an ARM board right in the middle of MPC8xx PowerPC
boards? You obviously never tested this.

Please do not submit untested code!!!

> @@ -1122,6 +1122,9 @@ RRvision_LCD_config:	unconfig
>  	@echo "#define CONFIG_SHARP_LQ104V7DS01" >>$(obj)include/config.h
>  	@$(MKCONFIG) -a RRvision ppc mpc8xx RRvision
>  
> +sheevaplug_config: unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood
> +
>  SM850_config	:	unconfig
>  	@$(MKCONFIG) $(@:_config=) ppc mpc8xx tqm8xx tqc

Why do you add an ARM board right in the middle of MPC8xx PowerPC
boards?

> @@ -2808,9 +2811,6 @@ lpd7a400_config \
>  lpd7a404_config:	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm lh7a40x lpd7a40x
>  
> -mv88f6281gtw_ge_config: unconfig
> -	@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood
> -

Why do you delete other boards?

> @@ -3110,6 +3110,8 @@ omap2420h4_config	: unconfig
>  qong_config		: unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm1136 qong davedenx mx31
>  
> +sheevapluge_config: unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm926ejs $(@:_config=) Marvell kirkwood

Why do you add the baord twice?

> +# (C) Copyright 2009
> +# Marvell Semiconductor <www.marvell.com>
> +# Prafulla Wadaskar <prafulla at marvell.com>

This makes no sense. Either Marvell holds the Copyright, or you do.
Same in other files.


> +/*
> + * For booting Linux, the board info and command line data
> + * have to be in the first 8 MB of memory, since this is
> + * the maximum mapped by the Linux kernel during initialization.
> + */
> +#define	CONFIG_BOOTMAPSZ	(8 << 20)	/* Initial Memmap for Linux */

Line too long.

> +#define CONFIG_MTDPARTS			"orion_nand:512k(uboot),"	\
> +	"1m at 4m(psm), 3m at 1m(kernel),13m at 5m(rootfs) rw\0"

This will not work. The embedded space is not allowed.

> +#ifdef CONFIG_CMD_NET
> +#define CONFIG_NETCONSOLE	/* include NetConsole support   */
> +#define CONFIG_NET_MULTI	/* specify more that one ports available */
> +#define	CONFIG_MII		/* expose smi ove miiphy interface */
> +#define CONFIG_KIRKWOOD_EGIGA	/* Enable kirkwood Gbe Controller Driver */

Line too long.


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
Marriage is the triumph  of  imagination  over  intelligence.  Second
marriage is the triumph of hope over experience.