[U-Boot] [PATCH v2] omap3evm: Support for quick boot

[Premi, Sanjeev]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, October 27, 2010 7:19 PM
> To: Premi, Sanjeev
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v2] omap3evm: Support for quick boot
> 

[snip]...[snip]

> > +#undef CONFIG_QUICK_BOOT
> > +#undef CONFIG_QUICK_BOOT_MMC
> > +#undef CONFIG_QUICK_BOOT_NAND
> 
> If you use this here to show the options, then please move it into the
> comment above.

[sp] Will do.

> 
> Otherwise, it is just dead code as you undefine things that are not
> defiend anyway.  In any case, please remove the #undef's.
> 
> 
> > +#ifdef CONFIG_QUICK_BOOT
> > +	/*
> > +	 * Generic config options
> > +	 */
> > +	#ifndef CONFIG_SILENT_CONSOLE
> > +		#define CONFIG_SILENT_CONSOLE	1
> > +	#endif
> > +
> > +	#ifndef CONFIG_ENV_IS_NOWHERE
> > +		#define CONFIG_ENV_IS_NOWHERE	1
> > +	#endif
> 
> Style issue: Please always start preprocessor commands in column 1.
> 

[sp] I was following style from another file which uses nesting.
     Maybe, my choice of reference was incorect.
     Will make the change.

[snip]...[snip]

> > +	#undef CONFIG_CMD_USB
> 
> I do not like this approach at all. It will make the board config file
> basicly unreadable - it becomes a mess of defiunitions here and (even
> conditional) #undef's and re-definitions there.
> 
> I suggest you chose a different approach, for example move the common
> (always used) configuration settings into a separate header file, and
> then provide two separate board configurations (normal and quick boot)
> which include the common settings.

[sp] I did think about it, infact, I was initially planning to follow
     the config_cmd_default.h; because, same set of changes can apply
     across multiple boards. I did test the patch against Beagle board
     as well.

> 
> This has the additional benefit that you can actually build both
> configurations without the need to modify any source files.

[sp] Was only concerned about possibility of multiple configurations
     per platform. Keeping them in sync could be pain.

     Do you have suggestions on this?

> 
> 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
> Lack of skill dictates economy of style.                - Joey Ramone
>