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

[Guennadi Liakhovetski]

On Tue, 19 May 2009, Scott Wood wrote:

> 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?

Hm, interesting question. On the one hand, env_init() indeed just looks at 
(ulong)env_ptr + CONFIG_ENV_SIZE for a copy of the redundant environment, 
without even using CONFIG_ENV_OFFSET_REDUND. On the other hand, the 
saveenv() function on NAND exists in two versions depending on whether 
CONFIG_ENV_OFFSET_REDUND is defined or not, and only one of them really 
handles the redundant environment, and there it uses 
CONFIG_ENV_OFFSET_REDUND, and not (ulong)env_ptr + CONFIG_ENV_SIZE.

Ok, here's the ultimate answer: to use redundant environment you need the 
flags member in env_t, and that one is only present, if 
CONFIG_SYS_REDUNDAND_ENVIRONMENT is defined. And on NAND that one is only 
defined if CONFIG_ENV_OFFSET_REDUND is defined. So, no, you cannot use 
redundant environment without CONFIG_ENV_OFFSET_REDUND in NAND, and we 
better use CONFIG_ENV_OFFSET_REDUND in env_init() too...

> > +	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.

Right, but, I think, it just looks more uniform for handling the four 
[!]crc1_ok && [!]crc2_ok cases. Can remove it if you prefer, sure.

> > 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.

It is moved into a malloc()'ed buffer, I haven't changed 
env_relocate_spec(). As for other boards, they have to find a suitable 
location for CONFIG_NAND_ENV_DST themselves too, of course.

> > 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.

Hm, I cannot test any other NAND boot drivers, so, I would prefer to leave 
them to someone who actually can do that.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de