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

[Wolfgang Denk]

Dear Sanjeev Premi,

In message <1288186611-30656-1-git-send-email-premi at ti.com> you wrote:
> Default configuration enables multiple features
> that are great for intial development work. But
> many of them are not useful when time taken to
> boot kernel is important - and uboot is frozen.
> 
> This patch attempts to reduce the size of u-boot
> binary by excluding unused/not-so-commonly used
> features/commands. It allows initialization of
> unused devices to be skipped.
> 
> Less time to load the binary and selective
> initialization results in considerable reduction
> in the boot time.
> 
> Signed-off-by: Sanjeev Premi <premi at ti.com>
> ---
> v2: Replaced occurences of "fast" with "quick"
>     to avoid any confusion with "fastboot" in
>     Android.
> 
>  include/configs/omap3_evm.h |  138 +++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 138 insertions(+), 0 deletions(-)
> 
> diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
> index 271c985..ce681b1 100644
> --- a/include/configs/omap3_evm.h
> +++ b/include/configs/omap3_evm.h
> @@ -378,4 +378,142 @@ extern unsigned int boot_flash_type;
>  #define CONFIG_BOOTP_HOSTNAME		0x00000004
>  #define CONFIG_BOOTP_BOOTPATH		0x00000010
>  
> +/*
> + * Support for quick boot
> + *
> + * In default build, the feature is disabled. To use the feature:
> + * 1) Define CONFIG_QUICK_BOOT
> + * 2) Select the device used as boot medium, by defining one of:
> + *    - CONFIG_QUICK_BOOT_MMC
> + *    - CONFIG_QUICK_BOOT_NAND
> + */
> +#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.

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.

> +	/*
> +	 * Exclude unused/rarely used features.
> +	 */
> +	#undef CONFIG_SYS_LONGHELP
> +	#undef CONFIG_SYS_HUSH_PARSER
> +
> +	#undef CONFIG_REVISION_TAG
> +
> +	#undef CONFIG_MD5
> +	#undef CONFIG_SHA1
> +	#undef CONFIG_BZIP2
> +	#undef CONFIG_LZMA
> +
> +	#undef CONFIG_CMD_BDI
> +	#undef CONFIG_CMD_BOOTD
> +	#undef CONFIG_CMD_CONSOLE
> +	#undef CONFIG_CMD_ECHO
> +	#undef CONFIG_CMD_EDITENV
> +	#undef CONFIG_CMD_FPGA
> +	#undef CONFIG_CMD_IMI
> +	#undef CONFIG_CMD_ITEST
> +	#undef CONFIG_CMD_LOADB
> +	#undef CONFIG_CMD_LOADS
> +	#undef CONFIG_CMD_NET
> +	#undef CONFIG_CMD_NFS
> +	#undef CONFIG_CMD_SETGETDCR
> +	#undef CONFIG_CMD_SOURCE
> +	#undef CONFIG_CMD_XIMG
> +	#undef CONFIG_CMD_FPGA
> +	#undef CONFIG_CMD_IMLS
> +	#undef CONFIG_CMD_FLASH
> +	#undef CONFIG_CMD_EXT2
> +	#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.

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

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