[U-Boot] [PATCH v9] Marvell MV88F6281GTW_GE Board support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1242763432-13693-1-git-send-email-prafulla at marvell.com> you wrote:
> 
> This is Marvell's 88F6281_A0 based custom board developed
> for wireless access point product
...
> +/*
> + *  Environment variables configurations
> + */
> +#ifdef CONFIG_SPI_FLASH
> +#define CONFIG_ENV_IS_IN_SPI_FLASH	1
> +#define CONFIG_ENV_SIZE			0x10000	/* spi flash block (64k) */
> +#define CONFIG_ENV_SECT_SIZE		0x10000	/* _64K */
> +#else
> +#define CONFIG_ENV_IS_NOWHERE		1	/* if env in SDRAM */
> +#define CONFIG_ENV_SIZE			0x20000	/* default 128k */

Just a  question...  Do  you  really  NEED  64  kB  or  even  128  kB
environement size? In my experience, 16 kB is almost always more than
sufficient.  Keep  in  mind  that the environment size can be smaller
than the sector size which stores the environment,  and  that  a  big
enviroment size adds to the boot delay, as the whole environment size
needs to be CRC32 checked.


> + * Default environment variables
> + */
> +#define CONFIG_BOOTCOMMAND		"${x_bootcmd_kernel}; setenv bootargs " \
> +	"${x_bootargs} ${x_bootargs_root}; bootm 0x6400000;"
> +
> +#define CONFIG_MTDPARTS			"spi0.0:512k(uboot),512k at 512k(psm), "	\
> +	"2m at 1m(kernel),13m at 3m(rootfs)\0"

Lines too long.

> +#define CONFIG_EXTRA_ENV_SETTINGS	"x_bootargs=console=ttyS0,115200 " \
> +	"mtdparts="CONFIG_MTDPARTS	\
> +	"x_bootcmd_kernel=cp.b 0xf8100000 0x6400000 0x200000\0" \
> +	"x_bootargs_root=root=/dev/mtdblock3 ro rootfstype=squashfs\0"
> +
> +/*
> + * Size of malloc() pool
> + */
> +#define CONFIG_SYS_MALLOC_LEN	0x00400000	/* 4M */
> +/* size in bytes reserved for initial data */
> +#define CONFIG_SYS_GBL_DATA_SIZE	128
> +
> +/*
> + * Other required minimal configurations
> + */
> +#define CONFIG_CONSOLE_INFO_QUIET	/* some code reduction */

Hm... you reserve  4  MB  malloc  space,  but  then  suppress  useful
information  to  save  a  few bytes? To me this seems inconsistent. I
recommend to check if this really makes sense.


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
C makes it easy for you to shoot yourself in the foot. C++ makes that
harder, but when you do, it blows away your whole leg.
                                                 -- Bjarne Stroustrup