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;
- 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@denx.de