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 <[email protected]>
> Cc: Guennadi Liakhovetski <[email protected]>

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: [email protected]
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to