[U-Boot] [PATCH] mx31ads: update board configuration

[Guennadi Liakhovetski]

On Thu, 13 Aug 2009, Wolfgang Denk wrote:

> Bring default environment more in line with other boards;
> fix address range for "mtest" command.
> 
> Signed-off-by: Wolfgang Denk <wd at denx.de>
> Cc: Guennadi Liakhovetski <lg at denx.de>

Having various board configurations look similar has, certainly, 
advantages, but I've got a couple of comments / questions:

> ---
>  include/configs/mx31ads.h |   62 ++++++++++++++++++++++++++-------------------
>  1 files changed, 36 insertions(+), 26 deletions(-)
> 
> diff --git a/include/configs/mx31ads.h b/include/configs/mx31ads.h
> index 363ea1b..e6c02c8 100644
> --- a/include/configs/mx31ads.h
> +++ b/include/configs/mx31ads.h
> @@ -56,7 +56,6 @@
>  /*
>   * Hardware drivers
>   */
> -

Is this removed empty line also to make it look more in line with others?

>  #define CONFIG_MXC_UART	1
>  #define CONFIG_SYS_MX31_UART1		1
>  
> @@ -87,27 +86,38 @@
>  #define CONFIG_CMD_SPI
>  #define CONFIG_CMD_DATE
>  
> -#define CONFIG_BOOTDELAY	3
> +#define CONFIG_BOOTDELAY	5

And this? do (most) other boards also have 5 seconds bootdelay? 
Personally, I find shorter delays better, they spare you a couple of 
seconds on default boots and usually hitting a key within a 2 or 3 second 
delay is not a problem either. Or is there a technical reason for this 
with this specific board?

>  
>  #define CONFIG_LOADADDR		0x80800000	/* loadaddr env var */
>  
>  #define	CONFIG_EXTRA_ENV_SETTINGS					\
>  	"netdev=eth0\0"							\
> -	"uboot_addr=0xa0000000\0"					\
> -	"uboot=mx31ads/u-boot.bin\0"					\
> +	"u-boot_addr=0xa0000000\0"					\
> +	"u-boot=mx31ads/u-boot.bin\0"					\
> +	"load=tftp ${loadaddr} ${u-boot}\0"				\
> +	"update=prot off ${u-boot_addr} +${filesize};"			\
> +		"era ${u-boot_addr} +${filesize};"			\
> +		"cp.b ${loadaddr} ${u-boot_addr} ${filesize};"		\
> +		"sete filesize;sete fileaddr;save\0"			\

Having abbreviated commands is good for typing, but IMHO in the 
environment, especially in the default environment, it is better to have 
complete commands. Besides, this seems inconsistent with the below:

>  	"kernel=mx31ads/uImage\0"					\
> -	"nfsroot=/opt/eldk/arm\0"					\
> -	"bootargs_base=setenv bootargs console=ttymxc0,115200\0"	\
> -	"bootargs_nfs=setenv bootargs ${bootargs} root=/dev/nfs "	\
> -		"ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0"	\
> -	"bootcmd=run bootcmd_net\0"					\
> -	"bootcmd_net=run bootargs_base bootargs_nfs; "			\
> -		"tftpboot ${loadaddr} ${kernel}; bootm\0"		\
> -	"prg_uboot=tftpboot ${loadaddr} ${uboot}; "			\
> -		"protect off ${uboot_addr} 0xa003ffff; "		\
> -		"erase ${uboot_addr} 0xa003ffff; "			\
> -		"cp.b ${loadaddr} ${uboot_addr} ${filesize}; "		\
> -		"setenv filesize; saveenv\0"
> +	"rootpath=/opt/eldk/armVFP\0"					\
> +	"netdev=eth0\0"							\
> +	"consdev=ttymxc0\0"						\
> +	"nfsargs=setenv bootargs root=/dev/nfs rw "			\
> +		"nfsroot=${serverip}:${rootpath}\0"			\
> +	"ramargs=setenv bootargs root=/dev/ram rw\0"			\
> +	"addip=setenv bootargs ${bootargs} "				\
> +		"ip=${ipaddr}:${serverip}:${gatewayip}:${netmask}"	\
> +		":${hostname}:${netdev}:off\0"				\
> +	"addcons=setenv bootargs ${bootargs} "				\
> +		"console=${consdev},${baudrate}\0"			\

Above "setenv"s are spelled out completely.

> +	"flash_nfs=run nfsargs addip addcons;"				\
> +		"bootm ${kernel_addr}\0"				\
> +	"flash_self=run ramargs addip addcons;"				\
> +		"bootm ${kernel_addr} ${ramdisk_addr}\0"		\
> +	"net_nfs=tftp 200000 ${bootfile};"				\
> +		"run nfsargs addip addcons;bootm\0"			\
> +	"bootcmd=run net_nfs\0"
>  
>  #define CONFIG_DRIVER_CS8900	1
>  #define CS8900_BASE		0xb4020300
> @@ -128,20 +138,20 @@
>  /*
>   * Miscellaneous configurable options
>   */
> -#define CONFIG_SYS_LONGHELP		/* undef to save memory */
> -#define CONFIG_SYS_PROMPT		"=> "
> -#define CONFIG_SYS_CBSIZE		256		/* Console I/O Buffer Size */
> +#define CONFIG_SYS_LONGHELP			/* undef to save memory */
> +#define CONFIG_SYS_PROMPT	"=> "
> +#define CONFIG_SYS_CBSIZE	256		/* Console I/O Buffer Size */
>  /* Print Buffer Size */
> -#define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
> -#define CONFIG_SYS_MAXARGS		16		/* max number of command args */
> -#define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE	/* Boot Argument Buffer Size */
> +#define CONFIG_SYS_PBSIZE	(CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
> +#define CONFIG_SYS_MAXARGS	32		/* max number of command args */
> +#define CONFIG_SYS_BARGSIZE	CONFIG_SYS_CBSIZE	/* Boot Argument Buffer Size */
>  
> -#define CONFIG_SYS_MEMTEST_START	0		/* memtest works on */
> -#define CONFIG_SYS_MEMTEST_END		0x10000
> +#define CONFIG_SYS_MEMTEST_START 0x80010000	/* memtest works on */
> +#define CONFIG_SYS_MEMTEST_END	0x87C00000	/* start+64k ... end-4MB */
>  
> -#define CONFIG_SYS_LOAD_ADDR		CONFIG_LOADADDR
> +#define CONFIG_SYS_LOAD_ADDR	CONFIG_LOADADDR
>  
> -#define CONFIG_SYS_HZ			1000
> +#define CONFIG_SYS_HZ		1000
>  
>  #define CONFIG_CMDLINE_EDITING	1
>  
> -- 
> 1.6.0.6
> 
> 
> 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de