[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[Hong Xu]

When env_init is called, the SPI is not actually initialized in U-Boot.
So that we can not read Dataflash for its content.
We simply mark it OK for now, and defer the real work to
`env_relocate_spec'. (Idealy from env_nand.c)

Signed-off-by: Hong Xu <hong.xu@atmel.com>
---
 common/env_dataflash.c |   83 ++++++++++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/common/env_dataflash.c b/common/env_dataflash.c
index 1d57079..55534a5 100644
--- a/common/env_dataflash.c
+++ b/common/env_dataflash.c
@@ -21,6 +21,7 @@
 #include <command.h>
 #include <environment.h>
 #include <linux/stddef.h>
+#include <malloc.h>
 #include <dataflash.h>
 #include <search.h>
 #include <errno.h>
@@ -50,11 +51,46 @@ uchar env_get_char_spec(int index)
 
 void env_relocate_spec(void)
 {
-	char buf[CONFIG_ENV_SIZE];
+	ulong old_crc, new_crc = 0;
+	char *buf;
 
-	read_dataflash(CONFIG_ENV_ADDR, CONFIG_ENV_SIZE, buf);
+	gd->env_valid = 0;
 
-	env_import(buf, 1);
+	buf = (char *)malloc(CONFIG_ENV_SIZE);
+	if (buf == NULL) {
+		error("Can not allocate memory for env.\n");
+		goto err_mem;
+	}
+
+	AT91F_DataflashInit();
+
+	if (read_dataflash(CONFIG_ENV_ADDR + offsetof(env_t, crc),
+		sizeof(ulong), (char *)&old_crc) != DATAFLASH_OK) {
+		error("Dataflash: Failed to read original 4-bytes CRC\n");
+		goto err;
+	}
+
+	if (read_dataflash(CONFIG_ENV_ADDR + offsetof(env_t, data),
+		ENV_SIZE, buf) != DATAFLASH_OK) {
+		error("Dataflash: Failed to read env string.\n");
+		goto err;
+	}
+
+	new_crc = crc32(new_crc, (uchar *)buf, ENV_SIZE);
+
+	if (old_crc == new_crc) {
+		gd->env_addr  = offsetof(env_t, data);
+		gd->env_valid = 1;
+		env_import(buf, 0);
+		goto out;
+	}
+
+err:
+	set_default_env("!bad CRC");
+out:
+	free(buf);
+err_mem:
+	return;
 }
 
 #ifdef CONFIG_ENV_OFFSET_REDUND
@@ -83,44 +119,15 @@ int saveenv(void)
 /*
  * Initialize environment use
  *
- * We are still running from ROM, so data use is limited.
- * Use a (moderately small) buffer on the stack
+ * When env_init is called, the SPI is not actually initialized in U-Boot.
+ * So that we can not read Dataflash for its content.
+ * We simply mark it OK for now, and defer the real work to
+ * `env_relocate_spec'. (Idealy from env_nand.c)
  */
 int env_init(void)
 {
-	ulong crc, len, new;
-	unsigned off;
-	uchar buf[64];
-
-	if (gd->env_valid)
-		return 0;
-
-	AT91F_DataflashInit();	/* prepare for DATAFLASH read/write */
-
-	/* read old CRC */
-	read_dataflash(CONFIG_ENV_ADDR + offsetof(env_t, crc),
-		sizeof(ulong), (char *)&crc);
-
-	new = 0;
-	len = ENV_SIZE;
-	off = offsetof(env_t,data);
-	while (len > 0) {
-		int n = (len > sizeof(buf)) ? sizeof(buf) : len;
-
-		read_dataflash(CONFIG_ENV_ADDR + off, n, (char *)buf);
-
-		new = crc32 (new, buf, n);
-		len -= n;
-		off += n;
-	}
-
-	if (crc == new) {
-		gd->env_addr  = offsetof(env_t,data);
-		gd->env_valid = 1;
-	} else {
-		gd->env_addr  = (ulong)&default_environment[0];
-		gd->env_valid = 0;
-	}
+	gd->env_addr  = (ulong)&default_environment[0];
+	gd->env_valid = 1;
 
 	return 0;
 }
-- 
1.7.3.3

[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[Reinhard Meyer]

Dear Hong Xu,
> When env_init is called, the SPI is not actually initialized in U-Boot.
> So that we can not read Dataflash for its content.
> We simply mark it OK for now, and defer the real work to
> `env_relocate_spec'. (Idealy from env_nand.c)
>
> Signed-off-by: Hong Xu<hong.xu@atmel.com>
> ---
>   common/env_dataflash.c |   83 ++++++++++++++++++++++++++----------------------
>   1 files changed, 45 insertions(+), 38 deletions(-)

I cannot really decide if that is a good approach. Where would be
the issue if SPI/dataflash were initialized at this point (before
relocation)?
Same works well for example for I2C.

If Wolfgang is OK with it, I can pick up this patch (once
it is agreed on the way to solve the issue).

Best Regards,
Reinhard

[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[Hong Xu]

Hi Reinhard,

On 08/03/2011 03:20 PM, Reinhard Meyer wrote:
> Dear Hong Xu,
>  > When env_init is called, the SPI is not actually initialized in U-Boot.
>  > So that we can not read Dataflash for its content.
>  > We simply mark it OK for now, and defer the real work to
>  > `env_relocate_spec'. (Idealy from env_nand.c)
>  >
>  > Signed-off-by: Hong Xu<hong.xu@atmel.com>
>  > ---
>  > common/env_dataflash.c | 83
> ++++++++++++++++++++++++++----------------------
>  > 1 files changed, 45 insertions(+), 38 deletions(-)
>
> I cannot really decide if that is a good approach. Where would be
> the issue if SPI/dataflash were initialized at this point (before
> relocation)?

Currently the SPI is initialized in board_init which is called in 
board_init_r, but env_init is called in board_init_f. So actually the 
original code needs the SPI to be initialized before env_init, not 
before relocation.

An alternative way is to put SPI initialization code in 
board_early_init_f. But I'm not sure if it's the correct way.

BR,
Eric

> Same works well for example for I2C.
>
> If Wolfgang is OK with it, I can pick up this patch (once
> it is agreed on the way to solve the issue).
>
> Best Regards,
> Reinhard
>

[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[Albert ARIBAUD]

On 03/08/2011 09:36, Hong Xu wrote:
> Hi Reinhard,
>
> On 08/03/2011 03:20 PM, Reinhard Meyer wrote:
>> Dear Hong Xu,
>>   >  When env_init is called, the SPI is not actually initialized in U-Boot.
>>   >  So that we can not read Dataflash for its content.
>>   >  We simply mark it OK for now, and defer the real work to
>>   >  `env_relocate_spec'. (Idealy from env_nand.c)
>>   >
>>   >  Signed-off-by: Hong Xu<hong.xu@atmel.com>
>>   >  ---
>>   >  common/env_dataflash.c | 83
>> ++++++++++++++++++++++++++----------------------
>>   >  1 files changed, 45 insertions(+), 38 deletions(-)
>>
>> I cannot really decide if that is a good approach. Where would be
>> the issue if SPI/dataflash were initialized at this point (before
>> relocation)?
>
> Currently the SPI is initialized in board_init which is called in
> board_init_r, but env_init is called in board_init_f. So actually the
> original code needs the SPI to be initialized before env_init, not
> before relocation.
>
> An alternative way is to put SPI initialization code in
> board_early_init_f. But I'm not sure if it's the correct way.

I guess this is a general issue: if some driver is needed to get access 
to environment, then it must be initialized before relocation. But not 
all boards need initialization this early, and conversively, for each 
board-specific case where environment reading requires driver X, this 
adds a *general* constraint of initializing X before relocation -- and 
we'll end up initializing just about anything before reloc on boards 
that do not actually need initializations this early.

Now this may not be feasible, but I think the idea is more "initialize 
what is needed for relocation, then relocate, then initialize the rest.

Does anyone else here see this problem as I see it, or am I just being 
over-nit-picking?

If it *is* a problem, then the only way I see to solve this is, for each 
driver that *might* be initialized before reloc, to provide a 
configuration option to select between initializing before or after 
relocation (possibly with variations in the driver initialization code 
based on this config option).

Or am I also over-engineering here?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] AT91: Defer Dataflash access to env_relocate_spec

[Wolfgang Denk]

Dear Hong Xu,

In message <1312354896-24951-1-git-send-email-hong.xu@atmel.com> you wrote:
> When env_init is called, the SPI is not actually initialized in U-Boot.
> So that we can not read Dataflash for its content.
> We simply mark it OK for now, and defer the real work to
> `env_relocate_spec'. (Idealy from env_nand.c)
> 
> Signed-off-by: Hong Xu <hong.xu@atmel.com>

How do you then get access for example to the "baudrate" environment
variable, which is needed for initializing the console interface,
which happens long before relocation?

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
"All my life I wanted to be someone; I guess I should have been  more
specific."                                              - Jane Wagner