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

[Prafulla Wadaskar]

Dear Wolfgang Denk

Thanks for your comments..

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, May 20, 2009 2:51 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; 
> Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH v9] Marvell MV88F6281GTW_GE Board support
> 
> 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.
I agree, even 4kb is sufficient for me
but if I keep it less than a sector size it gives me bad CRC warning at boot up even though I do saveenv
Hence I kept it equal to sector size
This may be a bug...??
 
> 
> 
> > + * 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.
Corrected....

> 
> > +#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.
I changed it to 128KB refering some other similar boards..

Regards..
Prafulla ..