Skip to contentSkip to conversation

AT91: Defer Dataflash access to env_relocate_spec

PATCH New Hong Xu 4 participants 5 messages Aug 3, 2011 – Aug 24, 2011

lore
Share & tools
lore

Hong Xu

author
#0 lore raw

PATCH 1/1 AT91: Defer Dataflash access to env_relocate_spec

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)

---
 common/env_dataflash.c |   83 ++++++++++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 38 deletions(-)
common/env_dataflash.c +45 -38
index 1d57079..55534a5 100644
@@ -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;
 }
Signature · 1 line
1.7.3.3
#1 lore raw

PATCH 1/1 AT91: Defer Dataflash access to env_relocate_spec

Dear Hong Xu,

Quoted 9 lines

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)

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

author
#2 lore raw

PATCH 1/1 AT91: Defer Dataflash access to env_relocate_spec

Hi Reinhard,

Quoted 9 lines · Reinhard Meyer

Dear Hong Xu,

Quoted 8 lines

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)

---
common/env_dataflash.c | 83

Quoted 6 lines

++++++++++++++++++++++++++----------------------

Quoted 1 line
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

Quoted 8 lines

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

#3 lore raw

PATCH 1/1 AT91: Defer Dataflash access to env_relocate_spec

Quoted 19 lines · Hong Xu

Hi Reinhard,

Quoted 9 lines · Reinhard Meyer

Dear Hong Xu,

Quoted 8 lines

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)

---

 common/env_dataflash.c | 83
Quoted 6 lines

++++++++++++++++++++++++++----------------------

Quoted 1 line
 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)?

Quoted 7 lines

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,

Signature · 1 line
Albert.
#4 lore raw

PATCH 1/1 AT91: Defer Dataflash access to env_relocate_spec

In reply to Hong Xu

Dear Hong Xu,

Quoted 6 lines · In message <[email protected]> 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)

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

Signature · 5 lines
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