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

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

[Guennadi Liakhovetski]

Hi,

a user is reporting, that setting "silent" and "baudrate" environment 
variables on ARM have no effect on the console bahaviour. I didn't test 
this but looking at the code it looks logical - the console is initialised 
before environment variables are read. Question - is this a bug that has 
to be fixed, or is there no good fix for this, because we really want to 
have console initialised ASAP - before the driver for the environment 
storage is initialised?

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

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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0901150827340.4456@axis700.grange> you wrote:
> 
> a user is reporting, that setting "silent" and "baudrate" environment 
> variables on ARM have no effect on the console bahaviour. I didn't test 
> this but looking at the code it looks logical - the console is initialised 
> before environment variables are read. Question - is this a bug that has 

This is obviously wrong, as  the  console  baudrate  is  set  *after*
reading  the  "baudrate"  environment  variable  -  at  least for all
conforming  ports.  There  may  be  broken   implementations   around
(especially  when booting from NAND), but these are just what I said:
broken.

> to be fixed, or is there no good fix for this, because we really want to 
> have console initialised ASAP - before the driver for the environment 
> storage is initialised?

Yes, this is a bug that has to be fixed, you obviously must be able
to access the environment before you can correctly configure the
console port.

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
If there was anything that depressed him more than his own  cynicism,
it was that quite often it still wasn't as cynical as real life.
                                 - Terry Pratchett, _Guards! Guards!_

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

[Ulf Samuelsson]

tor 2009-01-15 klockan 08:33 +0100 skrev Guennadi Liakhovetski:
> Hi,
> 
> a user is reporting, that setting "silent" and "baudrate" environment 
> variables on ARM have no effect on the console bahaviour. I didn't test 
> this but looking at the code it looks logical - the console is initialised 
> before environment variables are read. Question - is this a bug that has 
> to be fixed, or is there no good fix for this, because we really want to 
> have console initialised ASAP - before the driver for the environment 
> storage is initialised?
> 

Yes, I am aware of this.
I noticed this problems about six months ago.
The environment vairable "baudrate" is read
before SPI is initialized and that means
that you cannot get the value when
the environment is in a dataflash. 
I do not know if this affects the operation
if the environment is in normal flash.

> 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
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
-- 
Ulf Samuelsson <ulf.samuelsson@atmel.com>
Atmel Nordic AB

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

[Wolfgang Denk]

Dear Ulf Samuelsson,

In message <1232048568.4757.3.camel@aeglos.sweden.atmel.com> you wrote:
> 
> The environment vairable "baudrate" is read
> before SPI is initialized and that means
> that you cannot get the value when
> the environment is in a dataflash. 

That's a bug in the port, then.

Viele Gr??e,

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
There are very few personal problems that cannot be solved through  a
suitable application of high explosives.

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

[Guennadi Liakhovetski]

Hi,

below is a patch / rfc that allows me to read the envorinment from NAND 
early enough to be used in the console initialisation. It turns out this 
is not only an ARM problem, rather it is common to all platforms storing 
environment in NAND. Similarly, probably, env in SPI flash would have this 
problem on all platforms too. The patch is not meant for mainline in its 
present form because it only solves the problem for platforms, that not 
only have their env in NAND, but also boot from NAND (using nand_spl). 
OTOH, who would want to store environment in NAND if they didn't have to 
boot from it? Anyway, it lacks generality. Also, it contains a couple of 
clean ups, that actually would have to be submitted separately (removal 
of unused "total" variable, and a typo fix in a comment).

So, this is mostly as an inspiration for someone to develop a proper 
patch, or, if we do decide, that this approach is good enough, I'll split 
it up and submit properly.

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

diff --git a/common/env_nand.c b/common/env_nand.c
index a8f0de7..1261dd2 100644
--- a/common/env_nand.c
+++ b/common/env_nand.c
@@ -71,9 +71,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(CFG_ENV_IS_APPENDED)
+env_t *env_ptr = (env_t *)CFG_NAND_ENV_DST;
 #else /* ! ENV_IS_EMBEDDED */
 env_t *env_ptr = 0;
 #endif /* ENV_IS_EMBEDDED */
@@ -105,23 +107,28 @@ uchar env_get_char_spec (int index)
  */
 int env_init(void)
 {
-#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;
 
-	total = CFG_ENV_SIZE;
+#ifdef CFG_REDUNDAND_ENVIRONMENT
+	env_t *tmp_env2;
 
-	tmp_env1 = env_ptr;
 	tmp_env2 = (env_t *)((ulong)env_ptr + CFG_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)
+	gd->env_addr = (ulong)env_ptr->data;
+
+	if (!crc1_ok && !crc2_ok) {
+		gd->env_addr  = 0;
 		gd->env_valid = 0;
-	else if(crc1_ok && !crc2_ok)
+	} else if(crc1_ok && !crc2_ok)
 		gd->env_valid = 1;
+#ifdef CFG_REDUNDAND_ENVIRONMENT
 	else if(!crc1_ok && crc2_ok)
 		gd->env_valid = 2;
 	else {
@@ -138,10 +145,13 @@ 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)&default_environment[0];
 	gd->env_valid = 1;
@@ -186,12 +196,10 @@ int writeenv(size_t offset, u_char *buf)
 #ifdef CFG_ENV_OFFSET_REDUND
 int saveenv(void)
 {
-	size_t total;
 	int ret = 0;
 	nand_erase_options_t nand_erase_options;
 
 	env_ptr->flags++;
-	total = CFG_ENV_SIZE;
 
 	nand_erase_options.length = CFG_ENV_RANGE;
 	nand_erase_options.quiet = 0;
@@ -229,7 +237,6 @@ int saveenv(void)
 #else /* ! CFG_ENV_OFFSET_REDUND */
 int saveenv(void)
 {
-	size_t total;
 	int ret = 0;
 	nand_erase_options_t nand_erase_options;
 
@@ -246,7 +253,6 @@ int saveenv(void)
 		return 1;
 
 	puts ("Writing to Nand... ");
-	total = CFG_ENV_SIZE;
 	if (writeenv(CFG_ENV_OFFSET, (u_char *) env_ptr)) {
 		puts("FAILED!\n");
 		return 1;
@@ -290,12 +296,9 @@ int readenv (size_t offset, u_char * buf)
 void env_relocate_spec (void)
 {
 #if !defined(ENV_IS_EMBEDDED)
-	size_t total;
 	int crc1_ok = 0, crc2_ok = 0;
 	env_t *tmp_env1, *tmp_env2;
 
-	total = CFG_ENV_SIZE;
-
 	tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE);
 	tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
 
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)
 
 /* NAND configuration */
 #define CFG_MAX_NAND_DEVICE	1
@@ -284,7 +287,8 @@
 */
 
 /* Settings as above boot configuration */
-#define CFG_ENV_IS_IN_NAND
+#define CFG_ENV_IS_IN_NAND	1
+#define CFG_ENV_IS_APPENDED	1
 #define CONFIG_BOOTARGS		"console=ttySAC,115200"
 
 #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 */
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
+
 	if (nand_chip.select_chip)
 		nand_chip.select_chip(&nand_info, -1);
 

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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902021725100.4218@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"

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

[Guennadi Liakhovetski]

On Mon, 2 Feb 2009, Wolfgang Denk wrote:

> Dear Guennadi Liakhovetski,
> 
> In message <Pine.LNX.4.64.0902021725100.4218@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?

Ok, agree, the name is badly chosen. The real restriction of this specific 
implementation is that the environment must be on the same NAND chip as 
the one we are booting from. Any location on that chip can be handled, I 
think.

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

The reason they share the same #if case here is that both get initialised 
early, not as was only possible until now on NAND - after all initcalls.

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

Ok, currently environment in NAND has two cases:

1. embedded, in this case the real environment is initialised at 
env_init() time, otherwise first default environment is used
2. other, env_init() initialises the default environment, the real (stored 
in NAND) environment is first activated at env_relocate() time.

With this patch we add one more case

3. environment, located on the same NAND chip as the boot NAND. in this 
case we can also let the nand_spl code load it for is in RAM and then we 
can use it in env_init().

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

This is smdk6400 header, on this board I chose to place the environment 
read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 
unused by u-boot. But I didn't find an easy way to pass a calculated value 
from u-boot proper to nand_spl or vice versa. As you know, these two 
objects use saparate linker scripts and are linked absolutely 
independently, and just cat'ed together in the end. So, I had to use a 
macro to specify the location of the environment copy in RAM instead of 
calculating it precisely.

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

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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902022141190.8602@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?
> 
> Ok, agree, the name is badly chosen. The real restriction of this specific 
> implementation is that the environment must be on the same NAND chip as 
> the one we are booting from. Any location on that chip can be handled, I 
> think.

Such an explanation belongs into the commit message.

And I'm not really sure why this file is affected, then.


> > > +#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.
> 
> The reason they share the same #if case here is that both get initialised 
> early, not as was only possible until now on NAND - after all initcalls.

But why does this matter here, and why do we have to change this file
for that?

> 1. embedded, in this case the real environment is initialised at 
> env_init() time, otherwise first default environment is used

First default environment? Do we have more than one default
environment?

> 2. other, env_init() initialises the default environment, the real (stored 
> in NAND) environment is first activated at env_relocate() time.
> 
> With this patch we add one more case
> 
> 3. environment, located on the same NAND chip as the boot NAND. in this 
> case we can also let the nand_spl code load it for is in RAM and then we 
> can use it in env_init().

I don't see why we need a special case here.

We have two situations only:

1) environment is on the same NAND chip as the U-Boot image; this is
   the case we can handle cleanly. But it does not play any role of
   the environment is embedded, or directly adjacent (either before
   or after) to the U-Boot image, or if it is on a completely
   different location on this NAND chip.

2) environment is on a different NAND chip than the U-Boot image; in
   this case we cannot access the environment as early as needed,
   i.e. proper operation of U-Boot is not possible.

To me this means that 1) is the case we implement, without additional
#ifdef'fery, and 2) is a configuration error for which we should  add
appropriate #error handling.

> > >  #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 ???
> 
> This is smdk6400 header, on this board I chose to place the environment 
> read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 

What has BSS to do with the environment storage ???

BSS does not take any space in the U-Boot image, so why leave a
(eventually huge) gap unused?

> unused by u-boot. But I didn't find an easy way to pass a calculated value 
> from u-boot proper to nand_spl or vice versa. As you know, these two 
> objects use saparate linker scripts and are linked absolutely 
> independently, and just cat'ed together in the end. So, I had to use a 
> macro to specify the location of the environment copy in RAM instead of 
> calculating it precisely.

What has the location in RAM to do with the image structure on NAND?
We don't need to allocate any space for BSS on the NAND device, right?

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
On a clear disk you can seek forever.

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

[Guennadi Liakhovetski]

On Mon, 2 Feb 2009, Wolfgang Denk wrote:

> Dear Guennadi Liakhovetski,
> 
> In message <Pine.LNX.4.64.0902022141190.8602@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?
> > 
> > Ok, agree, the name is badly chosen. The real restriction of this specific 
> > implementation is that the environment must be on the same NAND chip as 
> > the one we are booting from. Any location on that chip can be handled, I 
> > think.
> 
> Such an explanation belongs into the commit message.

Sure, _if_ we decide, that this is the correct direction to fix this 
problem.

> And I'm not really sure why this file is affected, then.

This file provides the env_init() function, which now has to become aware 
of the new possibility - proper (not default) environment already in RAM 
(after being loaded by nand_spl).

> > > > +#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.
> > 
> > The reason they share the same #if case here is that both get initialised 
> > early, not as was only possible until now on NAND - after all initcalls.
> 
> But why does this matter here, and why do we have to change this file
> for that?

I haven't found a better way to do this. Maybe there is one. I understood, 
that the proper place to provide an early copy of the dynamic (not 
default / static) environment is env_init(), a copy of which is present in 
each of env_{flash,nand,nvram,onenand,dataflash,sf}.c.

> > 1. embedded, in this case the real environment is initialised at 
> > env_init() time, otherwise first default environment is used
> 
> First default environment? Do we have more than one default
> environment?

s/first/at first/ (in the beginning, before env_relocate()).

> > 2. other, env_init() initialises the default environment, the real (stored 
> > in NAND) environment is first activated at env_relocate() time.
> > 
> > With this patch we add one more case
> > 
> > 3. environment, located on the same NAND chip as the boot NAND. in this 
> > case we can also let the nand_spl code load it for is in RAM and then we 
> > can use it in env_init().
> 
> I don't see why we need a special case here.
> 
> We have two situations only:
> 
> 1) environment is on the same NAND chip as the U-Boot image; this is
>    the case we can handle cleanly. But it does not play any role of
>    the environment is embedded, or directly adjacent (either before
>    or after) to the U-Boot image, or if it is on a completely
>    different location on this NAND chip.

Well, there is a difference. If the environment is embedded, it is loaded 
by nand_spl automatically with the image with a single nand_load() (that's 
how it worked until now, I think). If the environment is not embedded but 
on the same chip, we still can handle it, but we need an additional 
nand_load(). And the copy of the environment from NAND as loaded into RAM 
will be at a different location. Maybe env_nand.c doesn't have to know 
about this difference, but I haven't gone that far to unify these two 
cases.

> 2) environment is on a different NAND chip than the U-Boot image; in
>    this case we cannot access the environment as early as needed,
>    i.e. proper operation of U-Boot is not possible.
> 
> To me this means that 1) is the case we implement, without additional
> #ifdef'fery, and 2) is a configuration error for which we should  add
> appropriate #error handling.

Hm, as I briefly looked through other env_*.c, it looks like a few of them 
resort to the default environment until env_relocate(). So, we can either 
accept that and say "on those media you can store environment, but it will 
be only available late in boot process," or we can start fixing them all 
by shifting hardware initialisation before env_init()... Or are you 
suggesting to declare them all as broken?

> > > >  #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 ???
> > 
> > This is smdk6400 header, on this board I chose to place the environment 
> > read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 
> 
> What has BSS to do with the environment storage ???
> 
> BSS does not take any space in the U-Boot image, so why leave a
> (eventually huge) gap unused?

I need a location in RAM for nand_spl to put a copy of the environment 
into. I know that bss doesn't take space in the u-boot image. But you 
cannot put environment there, because it will be nullified by u-boot 
proper.

> > unused by u-boot. But I didn't find an easy way to pass a calculated value 
> > from u-boot proper to nand_spl or vice versa. As you know, these two 
> > objects use saparate linker scripts and are linked absolutely 
> > independently, and just cat'ed together in the end. So, I had to use a 
> > macro to specify the location of the environment copy in RAM instead of 
> > calculating it precisely.
> 
> What has the location in RAM to do with the image structure on NAND?
> We don't need to allocate any space for BSS on the NAND device, right?

1. nand_spl copies u-boot proper at a location in RAM

2. nand_spl copies environment at another location in RAM

3. nand_spl jumps to u-boot proper

4. u-boot nullifies bss

5. u-boot looks for environment - either embedded, or default, or copied 
by nand_spl in 2.

ideally you would tell nand_spl to put environment directly after bss, but 
I haven't find a way (without adding some ugly scripts) to do this.

Notice, 1) I am not an expert in U-Boot environment operation, so, there 
well might be issues I'm overseeing, 2) redundant environment is not 
handled yet, 3) would be nice to hear comments from nand(_spl) 
maintainers.

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

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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902022310070.8602@axis700.grange> you wrote:
> 
> > Such an explanation belongs into the commit message.
> 
> Sure, _if_ we decide, that this is the correct direction to fix this 
> problem.

We're iterating here to make this a usable patch, right? Such
description should be added to the next version.

> > We have two situations only:
> > 
> > 1) environment is on the same NAND chip as the U-Boot image; this is
> >    the case we can handle cleanly. But it does not play any role of
> >    the environment is embedded, or directly adjacent (either before
> >    or after) to the U-Boot image, or if it is on a completely
> >    different location on this NAND chip.
> 
> Well, there is a difference. If the environment is embedded, it is loaded 
> by nand_spl automatically with the image with a single nand_load() (that's 
> how it worked until now, I think). If the environment is not embedded but 
> on the same chip, we still can handle it, but we need an additional 
> nand_load(). And the copy of the environment from NAND as loaded into RAM 
> will be at a different location. Maybe env_nand.c doesn't have to know 
> about this difference, but I haven't gone that far to unify these two 
> cases.

Why not? The only thing env_nand.c needs to know is the address of the
environment in memory - it does not care if it's embedded or
elsewhere.

I think we should not artifically split this into two cases when it's
really one. There is two things that need to be done: (1) nand_spl must
locate a valid copy of the environment and load it into RAM, and (2)
the address must be passed to env_nand.c, but this is probably easy as
it can be statically defined, I think.

> > To me this means that 1) is the case we implement, without additional
> > #ifdef'fery, and 2) is a configuration error for which we should  add
> > appropriate #error handling.
> 
> Hm, as I briefly looked through other env_*.c, it looks like a few of them 
> resort to the default environment until env_relocate(). So, we can either 

Which ones? I think this is almost certainly broken, then.

> accept that and say "on those media you can store environment, but it will 
> be only available late in boot process," or we can start fixing them all 
> by shifting hardware initialisation before env_init()... Or are you 
> suggesting to declare them all as broken?

Not declare them as broken - just state what they are.

But that wa snot what I suggested above. I wrote that we should  bail
out  with  an  error  message  if  somebody  attempts  to  compile  a
configuration where the environment is on a different NAND chip  than
the U-Boot image - it's a non-supported configuration.

> > > > >  #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 ???
> > > 
> > > This is smdk6400 header, on this board I chose to place the environment 
> > > read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 
> > 
> > What has BSS to do with the environment storage ???
> > 
> > BSS does not take any space in the U-Boot image, so why leave a
> > (eventually huge) gap unused?
> 
> I need a location in RAM for nand_spl to put a copy of the environment 
> into. I know that bss doesn't take space in the u-boot image. But you 
> cannot put environment there, because it will be nullified by u-boot 
> proper.

I think there is some problem with the memory map on this system. It's
next to impossible to understand it's design from the config file.
Maybe you should document this in the first place. No, please deleted
the "maybe".

Your patch refers to CFG_MAPPED_RAM_BASE  which  is  not  present  in
mainline. Then you define CFG_UBOOT_BASE. Using a fixed offset, i. e.
probably without dynamic memory sizing? Grrrgh. Ah, right, that's ARM
where this is broken anyway.

Well, I think it is fundamentally important here that you take off the
ARM goggles here. Please be aware that such code must run on PPC as
well, i. e. it must be usable on systems that do proper auto-sizing
etc.

If you need space for the environment copy, you  must  not  just  use
some  area  somewhere  "behind"  U-Boot,  where  it  will most likely
collide with protected ram, shared log buffer,  frame  buffer  memory
etc. Instead, you must design a place for it in the memory map.

Thus please start by documenting the intended memory map.

> > What has the location in RAM to do with the image structure on NAND?
> > We don't need to allocate any space for BSS on the NAND device, right?
> 
> 1. nand_spl copies u-boot proper at a location in RAM
> 
> 2. nand_spl copies environment at another location in RAM
> 
> 3. nand_spl jumps to u-boot proper
> 
> 4. u-boot nullifies bss
> 
> 5. u-boot looks for environment - either embedded, or default, or copied 
> by nand_spl in 2.

Agreed. I don't see any mention of BSS in your description, though -
as expected.

> ideally you would tell nand_spl to put environment directly after bss, but 
> I haven't find a way (without adding some ugly scripts) to do this.

No, that is definitely not "ideally", that's most likely buggy, as it
messes with the memory map we're using so far.

> Notice, 1) I am not an expert in U-Boot environment operation, so, there 
> well might be issues I'm overseeing, 2) redundant environment is not 
> handled yet, 3) would be nice to hear comments from nand(_spl) 
> maintainers.

I don't see what nand(_spl) maintainers should comment here  -  their
code  is  so  far  completley  unaffected.  We just agreed on needing
another call to the load routine.

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
If A equals success, then the formula is A = X + Y + Z. X is work.  Y
is play. Z is keep your mouth shut.                 - Albert Einstein

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

[Guennadi Liakhovetski]

Morning Wolfgang

On Tue, 3 Feb 2009, Wolfgang Denk wrote:

> Why not? The only thing env_nand.c needs to know is the address of the
> environment in memory - it does not care if it's embedded or
> elsewhere.
> 
> I think we should not artifically split this into two cases when it's
> really one. There is two things that need to be done: (1) nand_spl must
> locate a valid copy of the environment and load it into RAM, and (2)
> the address must be passed to env_nand.c, but this is probably easy as
> it can be statically defined, I think.

for (1) - "valid copy" - you're not suggesting to calculate crc32 in 
nand_spl to decide upon env validity, are you? I think, we should add as 
little as possible to nand_spl, remember, it has to fit in a very limited 
size like 16, or 4, or even 2K. So, probably, just add one (or two for 
redundant env) nand_load calls and let env_init() evaluate them? Or is 
this what you meant?

> > > To me this means that 1) is the case we implement, without additional
> > > #ifdef'fery, and 2) is a configuration error for which we should  add
> > > appropriate #error handling.
> > 
> > Hm, as I briefly looked through other env_*.c, it looks like a few of them 
> > resort to the default environment until env_relocate(). So, we can either 
> 
> Which ones? I think this is almost certainly broken, then.

Correct me if I'm wrong, but I think code like

int env_init(void)
{
	/* SPI flash isn't usable before relocation */
	gd->env_addr = (ulong)&default_environment[0];
	gd->env_valid = 1;

	return 0;
}

does mean the specific environment limits itself to default until 
env_relocate()? Then, this snippet os from env_sf.c, env_onenand.c does 
exactly the same, env_nand.c if env is not embedded. The rest are actually 
reading the medium.

> > accept that and say "on those media you can store environment, but it will 
> > be only available late in boot process," or we can start fixing them all 
> > by shifting hardware initialisation before env_init()... Or are you 
> > suggesting to declare them all as broken?
> 
> Not declare them as broken - just state what they are.
> 
> But that wa snot what I suggested above. I wrote that we should  bail
> out  with  an  error  message  if  somebody  attempts  to  compile  a
> configuration where the environment is on a different NAND chip  than
> the U-Boot image - it's a non-supported configuration.

Why? Maybe there are users out there ATM that have environment in NAND and 
boot from another storage and are reasonably happy being only able to 
affect late configuration dynamically (e.g., Linux boot procedure) and 
having to recompile u-boot to change early configuration (console 
baudrate, etc.)?

> > > > > >  #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 ???
> > > > 
> > > > This is smdk6400 header, on this board I chose to place the environment 
> > > > read in by nand_spl above bss, because, AFAIU, the area above __bss_end is 
> > > 
> > > What has BSS to do with the environment storage ???
> > > 
> > > BSS does not take any space in the U-Boot image, so why leave a
> > > (eventually huge) gap unused?
> > 
> > I need a location in RAM for nand_spl to put a copy of the environment 
> > into. I know that bss doesn't take space in the u-boot image. But you 
> > cannot put environment there, because it will be nullified by u-boot 
> > proper.
> 
> I think there is some problem with the memory map on this system. It's
> next to impossible to understand it's design from the config file.
> Maybe you should document this in the first place. No, please deleted
> the "maybe".
> 
> Your patch refers to CFG_MAPPED_RAM_BASE  which  is  not  present  in
> mainline. Then you define CFG_UBOOT_BASE. Using a fixed offset, i. e.
> probably without dynamic memory sizing? Grrrgh. Ah, right, that's ARM
> where this is broken anyway.
> 
> Well, I think it is fundamentally important here that you take off the
> ARM goggles here. Please be aware that such code must run on PPC as
> well, i. e. it must be usable on systems that do proper auto-sizing
> etc.
> 
> If you need space for the environment copy, you  must  not  just  use
> some  area  somewhere  "behind"  U-Boot,  where  it  will most likely
> collide with protected ram, shared log buffer,  frame  buffer  memory
> etc. Instead, you must design a place for it in the memory map.
> 
> Thus please start by documenting the intended memory map.

This all sounds good, but, as I see it, this memory map, and the location 
of the env copies read out by nand_spl are platform-dependent, so, every 
implementor will just have to take care of the specific configuration. 
besides, this location only has to survive until env_relocate(), where it 
anyway will be copied to a malloc()ed buffer. Although, if we reuse the 
ENV_IS_EMBEDDED case then we can keep the buffers and avoid the extra 
malloc() / memcpy(). But documenting is always good, sure:-)

> > > What has the location in RAM to do with the image structure on NAND?
> > > We don't need to allocate any space for BSS on the NAND device, right?
> > 
> > 1. nand_spl copies u-boot proper at a location in RAM
> > 
> > 2. nand_spl copies environment at another location in RAM
> > 
> > 3. nand_spl jumps to u-boot proper
> > 
> > 4. u-boot nullifies bss
> > 
> > 5. u-boot looks for environment - either embedded, or default, or copied 
> > by nand_spl in 2.
> 
> Agreed. I don't see any mention of BSS in your description, though -
> as expected.

in 4. - as a reason why you have to skip it.

> > ideally you would tell nand_spl to put environment directly after bss, but 
> > I haven't find a way (without adding some ugly scripts) to do this.
> 
> No, that is definitely not "ideally", that's most likely buggy, as it
> messes with the memory map we're using so far.
> 
> > Notice, 1) I am not an expert in U-Boot environment operation, so, there 
> > well might be issues I'm overseeing, 2) redundant environment is not 
> > handled yet, 3) would be nice to hear comments from nand(_spl) 
> > maintainers.
> 
> I don't see what nand(_spl) maintainers should comment here  -  their
> code  is  so  far  completley  unaffected.  We just agreed on needing
> another call to the load routine.

We still have the other possibility - properly initialise NAND before 
calling env_init() and then read out env? This would be a preferred 
solution IMHO, because it leverages restrictions like "only the boot 
NAND," simplifies the choice of a location in RAM, leaves nand_spl code 
intact, thus avoiding even minimal size increment. NAND maintainer's 
opinion as to how feasible / difficult this would be would be appreciated. 
Or even better - a patch:-) I think, we could even move mem_malloc_init() 
before the initcall loop and use a malloc()ed buffer just as 
env_relocate() would do.

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

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

[Wolfgang Denk]

Dear Guennadi Liakhovetski,

In message <Pine.LNX.4.64.0902030842390.4902@axis700.grange> you wrote:
> 
> > I think we should not artifically split this into two cases when it's
> > really one. There is two things that need to be done: (1) nand_spl must
> > locate a valid copy of the environment and load it into RAM, and (2)
> > the address must be passed to env_nand.c, but this is probably easy as
> > it can be statically defined, I think.
> 
> for (1) - "valid copy" - you're not suggesting to calculate crc32 in 
> nand_spl to decide upon env validity, are you? I think, we should add as 
> little as possible to nand_spl, remember, it has to fit in a very limited 
> size like 16, or 4, or even 2K. So, probably, just add one (or two for 
> redundant env) nand_load calls and let env_init() evaluate them? Or is 
> this what you meant?

I enjoy the position of taking a higher level view without caring
about implementation details :-)

Technically, you have to check which of the possibly N copies of  the
environment  in NAND are valid, and which of them is the most current
copy. With NAND based storage this always means you have to load  all
the data in RAM anyway, so it seems OK with me to do just the loading
in  the  NAND  bootstrap loader and leave the rest of the work to the
full U-Boot initialization.

But as mentioned before: that's an implementation details.


> > > Hm, as I briefly looked through other env_*.c, it looks like a few of them 
> > > resort to the default environment until env_relocate(). So, we can either 
> > 
> > Which ones? I think this is almost certainly broken, then.
> 
> Correct me if I'm wrong, but I think code like
> 
> int env_init(void)
> {
> 	/* SPI flash isn't usable before relocation */
> 	gd->env_addr = (ulong)&default_environment[0];
> 	gd->env_valid = 1;
> 
> 	return 0;
> }
> 
> does mean the specific environment limits itself to default until 
> env_relocate()? Then, this snippet os from env_sf.c, env_onenand.c does 
> exactly the same, env_nand.c if env is not embedded. The rest are actually 
> reading the medium.

You are right - all the examples you listed are basicly broken and
should be fixed.

> > But that wa snot what I suggested above. I wrote that we should  bail
> > out  with  an  error  message  if  somebody  attempts  to  compile  a
> > configuration where the environment is on a different NAND chip  than
> > the U-Boot image - it's a non-supported configuration.
> 
> Why? Maybe there are users out there ATM that have environment in NAND and 
> boot from another storage and are reasonably happy being only able to 
> affect late configuration dynamically (e.g., Linux boot procedure) and 
> having to recompile u-boot to change early configuration (console 
> baudrate, etc.)?

Why should we add additional complexity to the  code  for  a  unknown
(but in any case tiny) number of systems with an exotic setup? So far
no  such  need  is known, and nobody in a sane state of mind would do
this. If we clearly state that  such  a  mode  of  operation  is  not
supported,  we also have a clear position if somebody asks about such
a design.

However, if we allow for this now, and add all the complexity to  the
implementation, you can be sure that soon somebody will pop up who is
stupid  enough  to  actually  use this, so we never can get rid of it
again, not even after realizing how stupid it actually was.

Let's do it right, especially when doing it right results in clean and
simple code that works as expected.


> > Thus please start by documenting the intended memory map.
> 
> This all sounds good, but, as I see it, this memory map, and the location 
> of the env copies read out by nand_spl are platform-dependent, so, every 
> implementor will just have to take care of the specific configuration. 

No. It is architecture dependent (unforatunately, becaus ethe initial
armboot port tried to "simplify" the design we had in place for PPC),
but it is in no way board dependent.

Yes, certain areas may or may not be present  for  a  specific  board
(there  may or may not be pram or a shared log buffer or video memory
or ...), but the relative location of all these areas is exactly  the
same for all boards of the same architecture.

> besides, this location only has to survive until env_relocate(), where it 
> anyway will be copied to a malloc()ed buffer. Although, if we reuse the 
> ENV_IS_EMBEDDED case then we can keep the buffers and avoid the extra 
> malloc() / memcpy(). But documenting is always good, sure:-)

You don't want to keep all the buffers. See above - you will probably
read N sectors of redundant environment, and in the end you will  use
just a part of one of these sectors.

> > > 1. nand_spl copies u-boot proper at a location in RAM
> > > 
> > > 2. nand_spl copies environment at another location in RAM
> > > 
> > > 3. nand_spl jumps to u-boot proper
> > > 
> > > 4. u-boot nullifies bss
> > > 
> > > 5. u-boot looks for environment - either embedded, or default, or copied 
> > > by nand_spl in 2.
> > 
> > Agreed. I don't see any mention of BSS in your description, though -
> > as expected.
> 
> in 4. - as a reason why you have to skip it.

Actually your sequence is wrong. U-Boot begins accessing the
environment before setting up the bss. And it seems obvious that you
must not meddle with any of the areas used by U-Boot - bss is not
special in this respect. Stack area, malloc area, pram, shared log
buffer, video memory etc. - all have to be considered. 

See above: start with designing the memory map for this usage case.

> We still have the other possibility - properly initialise NAND before 
> calling env_init() and then read out env? This would be a preferred 

You are missing a very basic point of U-Boot design: please see
http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#6_Keep_it_Debuggable

The more code we move before the intialization of the  console  port,
the  less  easy  we can debug U-Boot. That's why I will always resist
against movin code towards "earlier".

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
"Maintain an awareness for contribution --  to  your  schedule,  your
project, our company."                         - A Group of Employees

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

[Scott Wood]

Guennadi Liakhovetski wrote:
> On Tue, 3 Feb 2009, Wolfgang Denk wrote:
>> I don't see what nand(_spl) maintainers should comment here  -  their
>> code  is  so  far  completley  unaffected.  We just agreed on needing
>> another call to the load routine.
> 
> We still have the other possibility - properly initialise NAND before 
> calling env_init() and then read out env? This would be a preferred 
> solution IMHO, because it leverages restrictions like "only the boot 
> NAND," simplifies the choice of a location in RAM, leaves nand_spl code 
> intact, thus avoiding even minimal size increment. NAND maintainer's 
> opinion as to how feasible / difficult this would be would be appreciated. 
> Or even better - a patch:-) I think, we could even move mem_malloc_init() 
> before the initcall loop and use a malloc()ed buffer just as 
> env_relocate() would do.

Besides Wolfgang's comment that this would preclude serial output during 
NAND init, this won't work if you're booting from NOR flash and thus 
don't have RAM initialized yet.  And even with NAND boot, this is prior 
to relocation to the top of RAM, which might complicate things.

We could leave nand_spl code completely untouched if we pad the NAND 
image to a block boundary, and then append the environment.  nand_spl 
would just see a larger image.

-Scott

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

[Derek Ou]

Is this a catch 22?  If console is initialized after loading environment 
variables, this
means NAND (or other memories) driver are loaded before console being 
available
and there is no console output to debug NAND driver initialization.  If 
console is
initialized before loading NAND driver, environment variable is 
available after the
console is configured?

Derek