[U-Boot] [ARM] Environment variables not available during console initialisation?

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902021725100.4218 at axis700.grange> you wrote:
> 
> -#ifdef ENV_IS_EMBEDDED
> +#if defined(ENV_IS_EMBEDDED)
>  extern uchar environment[];
>  env_t *env_ptr = (env_t *)(&environment[0]);
> +#elif defined(CFG_ENV_IS_APPENDED)

What makes you think an "appended" environment is so special that it
is worth a separate #ifdef here? What about a prepended environment?
Or one two sectors apart?

> -#if defined(ENV_IS_EMBEDDED)
> -	size_t total;
> +#if defined(ENV_IS_EMBEDDED) || defined(CFG_ENV_IS_APPENDED)
>  	int crc1_ok = 0, crc2_ok = 0;
> -	env_t *tmp_env1, *tmp_env2;
> +	env_t *tmp_env1;
>  

Note that there is a fundamental difference between embedded and
non-embedded (in whatever which way) environments, but I can see no
significant difference between an "appended" or a "prepended" or
otherwise separate environment.

Maybe you try to explain what you have in mind with this
special-casing here?

And I don't understand the rest of your implementation either. Please
explain...


> diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h
> index 1ee4191..3acf7cd 100644
> --- a/include/configs/smdk6400.h
> +++ b/include/configs/smdk6400.h
> @@ -223,6 +224,8 @@
>  #define CFG_UBOOT_BASE		(CFG_MAPPED_RAM_BASE + 0x07e00000)
>  
>  #define CFG_ENV_OFFSET		0x0040000
> +/* Leave enough space for bss, currently __bss_end == 0x57e74800 */
> +#define CFG_NAND_ENV_DST	(CFG_UBOOT_BASE + 0x80000)

???? What's that? What has BSS to do with the environment storage ???

>  #if !defined(CONFIG_ENABLE_MMU)
> diff --git a/lib_arm/board.c b/lib_arm/board.c
> index a093860..2dadfce 100644
> --- a/lib_arm/board.c
> +++ b/lib_arm/board.c
> @@ -282,7 +282,7 @@ init_fnc_t *init_sequence[] = {
>  	board_init,		/* basic board dependent setup */
>  	interrupt_init,		/* set up exceptions */
>  	env_init,		/* initialize environment */
> -	init_baudrate,		/* initialze baudrate settings */
> +	init_baudrate,		/* initialize baudrate settings */
>  	serial_init,		/* serial communications setup */
>  	console_init_f,		/* stage 1 init of console */
>  	display_banner,		/* say that we are here */

Please never, never ever mix unrelated changes into one patch. Always
keep patches orthogonal.

> diff --git a/nand_spl/nand_boot.c b/nand_spl/nand_boot.c
> index 16d128f..1fe10d2 100644
> --- a/nand_spl/nand_boot.c
> +++ b/nand_spl/nand_boot.c
> @@ -248,6 +248,11 @@ void nand_boot(void)
>  	ret = nand_load(&nand_info, CFG_NAND_U_BOOT_OFFS, CFG_NAND_U_BOOT_SIZE,
>  			(uchar *)CFG_NAND_U_BOOT_DST);
>  
> +#ifdef CFG_ENV_IS_APPENDED
> +	nand_load(&nand_info, CFG_ENV_OFFSET, CFG_ENV_SIZE,
> +		  (uchar *)CFG_NAND_ENV_DST);
> +#endif

What is so special here with "appended" compared to any other location
of the environment in the NAND?

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
...though his invention worked superbly -- his theory was a crock  of
sewage from beginning to end.         - Vernor Vinge, "The Peace War"