[U-Boot] [PATCH 2/2] nand_spl: read environment early, when booting from NAND using nand_spl

[Scott Wood]

On Mon, May 18, 2009 at 04:07:22PM +0200, Guennadi Liakhovetski wrote:
>  int env_init(void)
>  {
> -#if defined(ENV_IS_EMBEDDED)
> +#if defined(ENV_IS_EMBEDDED) || defined(CONFIG_NAND_ENV_DST)
>  	int crc1_ok = 0, crc2_ok = 0;
> -	env_t *tmp_env1, *tmp_env2;
> +	env_t *tmp_env1;
> +
> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	env_t *tmp_env2;
>  
> -	tmp_env1 = env_ptr;
>  	tmp_env2 = (env_t *)((ulong)env_ptr + CONFIG_ENV_SIZE);
> +	crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc);
> +#endif

Are there any existing boards that use a redundant embedded environment
without defining CONFIG_ENV_OFFSET_REDUND, since it seems it was done
unconditionally before?

> +	tmp_env1 = env_ptr;
>  
>  	crc1_ok = (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc);
> -	crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc);
>  
> -	if (!crc1_ok && !crc2_ok)
> +	if (!crc1_ok && !crc2_ok) {
> +		gd->env_addr  = 0;
>  		gd->env_valid = 0;
> -	else if(crc1_ok && !crc2_ok)
> +
> +		return 0;
> +	} else if (crc1_ok && !crc2_ok) {

No need for "else" after return.

> diff --git a/include/configs/smdk6400.h b/include/configs/smdk6400.h
> index cac58cf..018f576 100644
> --- a/include/configs/smdk6400.h
> +++ b/include/configs/smdk6400.h
> @@ -209,6 +209,9 @@
>  /* total memory available to uboot */
>  #define CONFIG_SYS_UBOOT_SIZE		(1024 * 1024)
>  
> +/* Put environment copies after the end of U-Boot owned RAM */
> +#define CONFIG_NAND_ENV_DST	(CONFIG_SYS_UBOOT_BASE + CONFIG_SYS_UBOOT_SIZE)
> +

This is the only board where I see CONFIG_SYS_UBOOT_SIZE defined.  What
would other boards supply here?  How would they make sure that u-boot
doesn't clobber the RAM environment (the u-boot image itself relocates,
avoiding this problem)?  Perhaps we should move the environment when
relocating.

> diff --git a/nand_spl/nand_boot.c b/nand_spl/nand_boot.c
> index c7eadad..be2e69c 100644
> --- a/nand_spl/nand_boot.c
> +++ b/nand_spl/nand_boot.c
> @@ -246,6 +246,16 @@ void nand_boot(void)
>  	ret = nand_load(&nand_info, CONFIG_SYS_NAND_U_BOOT_OFFS, CONFIG_SYS_NAND_U_BOOT_SIZE,
>  			(uchar *)CONFIG_SYS_NAND_U_BOOT_DST);
>  
> +#ifdef CONFIG_NAND_ENV_DST
> +	nand_load(&nand_info, CONFIG_ENV_OFFSET, CONFIG_ENV_SIZE,
> +		  (uchar *)CONFIG_NAND_ENV_DST);
> +
> +#ifdef CONFIG_ENV_OFFSET_REDUND
> +	nand_load(&nand_info, CONFIG_ENV_OFFSET_REDUND, CONFIG_ENV_SIZE,
> +		  (uchar *)CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE);
> +#endif
> +#endif

Don't forget the other NAND boot drivers... perhaps we should factor out
the nand_load calls into something common.

-Scott