Re: [U-Boot] [PATCH 2/2] nand_spl: read environment early, when booting from NAND using nand_spl
On Mon, May 18, 2009 at 04:07:22PM +0200, Guennadi Liakhovetski wrote: > Currently, when booting from NAND using nand_spl, in the beginning the default > environment is used until later in boot process the dynamic environment is > read > out. This way environment variables that must be interpreted early, like the > baudrate or "silent", cannot be modified dynamically and remain at their > default values. Fix this problem by reading out main and redundand (if used) > copies of the environment in the nand_spl code. > > Signed-off-by: Guennadi Liakhovetski Applied to u-boot-nand-flash. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] nand_spl: read environment early, when booting from NAND using nand_spl
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: off...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/2] nand_spl: read environment early, when booting from NAND using nand_spl
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/2] nand_spl: read environment early, when booting from NAND using nand_spl
Currently, when booting from NAND using nand_spl, in the beginning the default
environment is used until later in boot process the dynamic environment is read
out. This way environment variables that must be interpreted early, like the
baudrate or "silent", cannot be modified dynamically and remain at their
default values. Fix this problem by reading out main and redundand (if used)
copies of the environment in the nand_spl code.
Signed-off-by: Guennadi Liakhovetski
---
README |6 ++
common/env_nand.c | 43 ++-
include/configs/smdk6400.h |3 +++
nand_spl/nand_boot.c | 10 ++
4 files changed, 49 insertions(+), 13 deletions(-)
diff --git a/README b/README
index 142dbcc..060b676 100644
--- a/README
+++ b/README
@@ -2422,6 +2422,12 @@ to save the current settings.
to a block boundary, and CONFIG_ENV_SIZE must be a multiple of
the NAND devices block size.
+- CONFIG_NAND_ENV_DST
+
+ Defines address in RAM to which the nand_spl code should copy the
+ environment. If redundant environment is used, it will be copied to
+ CONFIG_NAND_ENV_DST + CONFIG_ENV_SIZE.
+
- CONFIG_SYS_SPI_INIT_OFFSET
Defines offset to the initial SPI buffer area in DPRAM. The
diff --git a/common/env_nand.c b/common/env_nand.c
index 21bce25..90a1c45 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -68,9 +68,11 @@ extern int default_environment_size;
char * env_name_spec = "NAND";
-#ifdef ENV_IS_EMBEDDED
+#if defined(ENV_IS_EMBEDDED)
extern uchar environment[];
env_t *env_ptr = (env_t *)(&environment[0]);
+#elif defined(CONFIG_NAND_ENV_DST)
+env_t *env_ptr = (env_t *)CONFIG_NAND_ENV_DST;
#else /* ! ENV_IS_EMBEDDED */
env_t *env_ptr = 0;
#endif /* ENV_IS_EMBEDDED */
@@ -102,23 +104,33 @@ uchar env_get_char_spec (int index)
*/
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
+
+ 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) {
gd->env_valid = 1;
- else if(!crc1_ok && crc2_ok)
+ }
+#ifdef CONFIG_ENV_OFFSET_REDUND
+ else if (!crc1_ok && crc2_ok) {
gd->env_valid = 2;
- else {
+ } else {
/* both ok - check serial */
if(tmp_env1->flags == 255 && tmp_env2->flags == 0)
gd->env_valid = 2;
@@ -132,14 +144,19 @@ int env_init(void)
gd->env_valid = 1;
}
+ if (gd->env_valid == 2)
+ env_ptr = tmp_env2;
+ else
+#endif
if (gd->env_valid == 1)
env_ptr = tmp_env1;
- else if (gd->env_valid == 2)
- env_ptr = tmp_env2;
-#else /* ENV_IS_EMBEDDED */
+
+ gd->env_addr = (ulong)env_ptr->data;
+
+#else /* ENV_IS_EMBEDDED || CONFIG_NAND_ENV_DST */
gd->env_addr = (ulong)&default_environment[0];
gd->env_valid = 1;
-#endif /* ENV_IS_EMBEDDED */
+#endif /* ENV_IS_EMBEDDED || CONFIG_NAND_ENV_DST */
return (0);
}
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)
+
#ifdef CONFIG_ENABLE_MMU
#define CONFIG_SYS_MAPPED_RAM_BASE 0xc000
#define CONFIG_BOOTCOMMAND "nand read 0xc0018000 0x6 0x1c;" \
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_EN
