[U-Boot-Users] [PATCH]env_nand.c Added bad block management for environment variables

Hi All, This is my first attempt at submitting a change so please be patient and kind. This change allows for the environment variables to be stored in a rand of nand flash. If the first block is bad then the environment is stored in the next one, and so on. It introduces CFG_ENV_RANGE to define the size of the area that way contain the environment data. This will allow the environment to be loaded over an area with bad blocks from the factory without a problem.
diff --git a/common/env_nand.c b/common/env_nand.c index 49742f5..29aafa2 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -155,27 +155,53 @@ int env_init(void) int saveenv(void) { size_t total; + size_t offset; int ret = 0;
env_ptr->flags++; total = CFG_ENV_SIZE;
+ if (CFG_ENV_RANGE < CFG_ENV_SIZE) + return 1; if(gd->env_valid == 1) { - puts ("Erasing redundant Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET_REDUND, CFG_ENV_SIZE)) + puts ("Erasing redundant Nand...\n"); + for (offset = CFG_ENV_OFFSET_REDUND; offset < + CFG_ENV_OFFSET_REDUND + CFG_ENV_RANGE; ) + { + if (nand_erase(&nand_info[0], + offset, CFG_ENV_SIZE)) { + offset += CFG_ENV_SIZE; + } else { + break; + } + } + if (offset >= CFG_ENV_OFFSET_REDUND + CFG_ENV_RANGE) { + puts ("Redundant Nand area is completely bad!\n"); + gd->env_valid = 2; return 1; + } puts ("Writing to redundant Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, + ret = nand_write(&nand_info[0], offset, &total, (u_char*) env_ptr); } else { - puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET, CFG_ENV_SIZE)) + puts ("Erasing Nand...\n"); + for (offset = CFG_ENV_OFFSET; offset < + CFG_ENV_OFFSET + CFG_ENV_RANGE; ) + { + if (nand_erase(&nand_info[0], + offset, CFG_ENV_SIZE)) { + offset += CFG_ENV_SIZE; + } else { + break; + } + } + if (offset >= CFG_ENV_OFFSET + CFG_ENV_RANGE) { + puts ("Nand area is completely bad!\n"); + gd->env_valid = 1; return 1; - + } puts ("Writing to Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, + ret = nand_write(&nand_info[0], offset, &total, (u_char*) env_ptr); } if (ret || total != CFG_ENV_SIZE) @@ -189,15 +215,30 @@ int saveenv(void) int saveenv(void) { size_t total; + size_t offset; int ret = 0;
- puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_SIZE)) + if (CFG_ENV_RANGE < CFG_ENV_SIZE) return 1; + puts ("Erasing Nand...\n"); + for (offset = CFG_ENV_OFFSET; offset < + CFG_ENV_OFFSET + CFG_ENV_RANGE; ) + { + if (nand_erase(&nand_info[0], + offset, CFG_ENV_SIZE)) { + offset += CFG_ENV_SIZE; + } else { + break; + } + } + if (offset >= CFG_ENV_OFFSET + CFG_ENV_RANGE) { + puts ("Nand area is completely bad!\n"); + return 1; + }
puts ("Writing to Nand... "); total = CFG_ENV_SIZE; - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); + ret = nand_write(&nand_info[0], offset, &total, (u_char*)env_ptr); if (ret || total != CFG_ENV_SIZE) return 1;
@@ -212,6 +253,7 @@ void env_relocate_spec (void) { #if !defined(ENV_IS_EMBEDDED) size_t total; + size_t offset; int crc1_ok = 0, crc2_ok = 0; env_t *tmp_env1, *tmp_env2;
@@ -220,10 +262,25 @@ void env_relocate_spec (void) tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
- nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) tmp_env1); - nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) tmp_env2); + for (offset = CFG_ENV_OFFSET; offset < CFG_ENV_OFFSET + CFG_ENV_RANGE ; ) { + if (nand_block_isbad (&nand_info[0], offset)) { + offset += CFG_ENV_SIZE; + } else { + nand_read(&nand_info[0], offset, &total, + (u_char*) tmp_env1); + break; + } + } + for (offset = CFG_ENV_OFFSET_REDUND; offset < + CFG_ENV_OFFSET_REDUND + CFG_ENV_RANGE ; ) { + if (nand_block_isbad (&nand_info[0], offset)) { + offset += CFG_ENV_SIZE; + } else { + nand_read(&nand_info[0], offset, &total, + (u_char*) tmp_env2); + break; + } + }
crc1_ok = (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc); crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc);
-- Stuart Wood
Lab X Technologies, LLC 176 Anderson Ave. Suite 302 Rochester, NY 14607 Phone: (585) 271-7790 x207 Fax: (585) 473.4707

On Tue, May 27, 2008 at 10:01:14AM -0400, Stuart Wood wrote:
Hi All, This is my first attempt at submitting a change so please be patient and kind. This change allows for the environment variables to be stored in a rand of nand flash. If the first block is bad then the environment is stored in the next one, and so on. It introduces CFG_ENV_RANGE to define the size of the area that way contain the environment data. This will allow the environment to be loaded over an area with bad blocks from the factory without a problem.
This will break any board that doesn't define CFG_ENV_RANGE... it should default to CFG_ENV_SIZE if unset.
if(gd->env_valid == 1) {
puts ("Erasing redundant Nand...");
if (nand_erase(&nand_info[0],
CFG_ENV_OFFSET_REDUND, CFG_ENV_SIZE))
puts ("Erasing redundant Nand...\n");
for (offset = CFG_ENV_OFFSET_REDUND; offset <
CFG_ENV_OFFSET_REDUND + CFG_ENV_RANGE; )
{
if (nand_erase(&nand_info[0],
offset, CFG_ENV_SIZE)) {
offset += CFG_ENV_SIZE;
} else {
break;
}
}
Please factor this code out instead of duplicating it for both environment instances.
It'd also be nicer as something like this:
offset = CFG_ENV_REDUND; end = offset + CFG_ENV_RANGE;
while (offset < end && nand_erase(&nand_info[0], offset, CFG_ENV_SIZE)) offset += CFG_ENV_SIZE;
@@ -189,15 +215,30 @@ int saveenv(void) int saveenv(void) { size_t total;
- size_t offset; int ret = 0;
- puts ("Erasing Nand...");
- if (nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_SIZE))
- if (CFG_ENV_RANGE < CFG_ENV_SIZE) return 1;
- puts ("Erasing Nand...\n");
- for (offset = CFG_ENV_OFFSET; offset <
CFG_ENV_OFFSET + CFG_ENV_RANGE; )
- {
if (nand_erase(&nand_info[0],
offset, CFG_ENV_SIZE)) {
offset += CFG_ENV_SIZE;
} else {
break;
}
- }
- if (offset >= CFG_ENV_OFFSET + CFG_ENV_RANGE) {
puts ("Nand area is completely bad!\n");
return 1;
- }
It'd be nice if we could reduce the duplication between the two versions of saveenv() as well.
- for (offset = CFG_ENV_OFFSET; offset < CFG_ENV_OFFSET + CFG_ENV_RANGE ; ) {
if (nand_block_isbad (&nand_info[0], offset)) {
offset += CFG_ENV_SIZE;
} else {
nand_read(&nand_info[0], offset, &total,
(u_char*) tmp_env1);
break;
}
- }
- for (offset = CFG_ENV_OFFSET_REDUND; offset <
CFG_ENV_OFFSET_REDUND + CFG_ENV_RANGE ; ) {
if (nand_block_isbad (&nand_info[0], offset)) {
offset += CFG_ENV_SIZE;
} else {
nand_read(&nand_info[0], offset, &total,
(u_char*) tmp_env2);
break;
}
- }
Same comments as saveenv().
-Scott

Scott,
Here is a new version of my patch to env_nand.c. Thanks for the good comments. Fixed a problem with the new code that allowed it to read a environment area even if it contained a bad block after the good environment data.
diff --git a/common/env_nand.c b/common/env_nand.c index 49742f5..3ee42e0 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -1,4 +1,7 @@ /* + * (C) Copywrite 2008 + * Stuart Wood, Lab X Technologies stuart.wood@labxtechnologies.com + * * (C) Copyright 2004 * Jian Zhang, Texas Instruments, jzhang@ti.com.
@@ -53,6 +56,10 @@ #error CONFIG_INFERNO not supported yet #endif
+#ifndef CFG_ENV_RANGE +#define CFG_ENV_RANGE CFG_ENV_SIZE +#endif + int nand_legacy_rw (struct nand_chip* nand, int cmd, size_t start, size_t len, size_t * retlen, u_char * buf); @@ -152,30 +159,57 @@ int env_init(void) * nand_dev_desc + 0. This is also the behaviour using the new NAND code. */ #ifdef CFG_ENV_OFFSET_REDUND +size_t erase_env(size_t offset) +{ + size_t end; + + end = offset + CFG_ENV_RANGE; + + while (offset < end && nand_erase(&nand_info[0],offset, CFG_ENV_SIZE)) + offset += CFG_ENV_SIZE; + + if (offset >= end) + return 0; + + return offset; +} + int saveenv(void) { size_t total; + size_t offset; int ret = 0;
env_ptr->flags++; total = CFG_ENV_SIZE;
+ if (CFG_ENV_RANGE < CFG_ENV_SIZE) + return 1; if(gd->env_valid == 1) { - puts ("Erasing redundant Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET_REDUND, CFG_ENV_SIZE)) + puts ("Erasing redundant Nand...\n"); + + offset = erase_env(CFG_ENV_OFFSET_REDUND); + if (offset == 0) { + puts ("Redundant Nand area is completely bad!\n"); + gd->env_valid = 2; return 1; + } + puts ("Writing to redundant Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, + ret = nand_write(&nand_info[0], offset, &total, (u_char*) env_ptr); } else { - puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET, CFG_ENV_SIZE)) + puts ("Erasing Nand...\n"); + + offset = erase_env(CFG_ENV_OFFSET); + if (offset == 0) { + puts ("Nand area is completely bad!\n"); + gd->env_valid = 1; return 1; + }
puts ("Writing to Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, + ret = nand_write(&nand_info[0], offset, &total, (u_char*) env_ptr); } if (ret || total != CFG_ENV_SIZE) @@ -189,15 +223,23 @@ int saveenv(void) int saveenv(void) { size_t total; + size_t offset; int ret = 0;
- puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_SIZE)) + if (CFG_ENV_RANGE < CFG_ENV_SIZE) return 1; + puts ("Erasing Nand...\n"); + + offset = erase_env(CFG_ENV_OFFSET); + + if (offset == 0) { + puts ("Nand area is completely bad!\n"); + return 1; + }
puts ("Writing to Nand... "); total = CFG_ENV_SIZE; - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); + ret = nand_write(&nand_info[0], offset, &total, (u_char*)env_ptr); if (ret || total != CFG_ENV_SIZE) return 1;
@@ -208,10 +250,26 @@ int saveenv(void) #endif /* CMD_SAVEENV */
#ifdef CFG_ENV_OFFSET_REDUND +int check_env_size (size_t offset) +{ + size_t end; + int ret_val = 0; + end = offset + CFG_ENV_SIZE; + + for (; offset < end; offset += nand_info[0].erasesize) { + if (nand_block_isbad(&nand_info[0],offset)) + ret_val = 1; + } + + return ret_val; +} + void env_relocate_spec (void) { #if !defined(ENV_IS_EMBEDDED) size_t total; + size_t offset; + size_t end; int crc1_ok = 0, crc2_ok = 0; env_t *tmp_env1, *tmp_env2;
@@ -220,10 +278,27 @@ void env_relocate_spec (void) tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
- nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) tmp_env1); - nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) tmp_env2); + offset = CFG_ENV_OFFSET; + end = offset + CFG_ENV_RANGE; + + while (offset < end && check_env_size(offset)) { + offset += CFG_ENV_SIZE; + } + if (offset >= end) + puts("No Valid Environment Area Found\n"); + else + nand_read(&nand_info[0], offset, &total, (u_char*) tmp_env1); + + offset = CFG_ENV_OFFSET_REDUND; + end = offset + CFG_ENV_RANGE; + + while (offset < end && check_env_size(offset)) { + offset += CFG_ENV_SIZE; + } + if (offset >= end) + puts("No Valid Reundant Environment Area Found\n"); + else + nand_read(&nand_info[0], offset, &total, (u_char*) tmp_env2);
crc1_ok = (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc); crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc);

Stuart Wood wrote:
Scott,
Here is a new version of my patch to env_nand.c. Thanks for the good comments. Fixed a problem with the new code that allowed it to read a environment area even if it contained a bad block after the good environment data.
Please put comments such as these that don't belong in the commit message below a --- line, with the commit message and a signed-off-by above it.
diff --git a/common/env_nand.c b/common/env_nand.c index 49742f5..3ee42e0 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -1,4 +1,7 @@ /*
- (C) Copywrite 2008
- Stuart Wood, Lab X Technologies stuart.wood@labxtechnologies.com
- (C) Copyright 2004
It's spelled "Copyright" (as in the "right" to "copy"). Just like the one below it. :-)
- Jian Zhang, Texas Instruments, jzhang@ti.com.
@@ -53,6 +56,10 @@ #error CONFIG_INFERNO not supported yet #endif
+#ifndef CFG_ENV_RANGE +#define CFG_ENV_RANGE CFG_ENV_SIZE +#endif
int nand_legacy_rw (struct nand_chip* nand, int cmd, size_t start, size_t len, size_t * retlen, u_char * buf); @@ -152,30 +159,57 @@ int env_init(void)
- nand_dev_desc + 0. This is also the behaviour using the new NAND code.
*/ #ifdef CFG_ENV_OFFSET_REDUND +size_t erase_env(size_t offset) +{
erase_env() should not be within this ifdef.
- size_t end;
- end = offset + CFG_ENV_RANGE;
- while (offset < end && nand_erase(&nand_info[0],offset, CFG_ENV_SIZE))
Spaces after commas.
offset += CFG_ENV_SIZE;
- if (offset >= end)
return 0;
- return offset;
+}
What if the offset is zero? Should return -1 or something similar on error.
@@ -208,10 +250,26 @@ int saveenv(void) #endif /* CMD_SAVEENV */
#ifdef CFG_ENV_OFFSET_REDUND +int check_env_size (size_t offset) +{
What about the non-redundant version of env_relocate_spec? Also, this isn't checking the size, so it's not really an appropriate name.
- size_t end;
- int ret_val = 0;
- end = offset + CFG_ENV_SIZE;
- for (; offset < end; offset += nand_info[0].erasesize) {
if (nand_block_isbad(&nand_info[0],offset))
ret_val = 1;
- }
- return ret_val;
size_t end = offset + CFG_ENV_SIZE;
while (offset < end) if (nand_block_isbad(&nand_info[0], offset)) return 1;
return 0;
@@ -220,10 +278,27 @@ void env_relocate_spec (void) tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
- nand_read(&nand_info[0], CFG_ENV_OFFSET, &total,
(u_char*) tmp_env1);
- nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total,
(u_char*) tmp_env2);
- offset = CFG_ENV_OFFSET;
- end = offset + CFG_ENV_RANGE;
- while (offset < end && check_env_size(offset)) {
offset += CFG_ENV_SIZE;
- }
- if (offset >= end)
puts("No Valid Environment Area Found\n");
- else
nand_read(&nand_info[0], offset, &total, (u_char*) tmp_env1);
If the environment can span multiple blocks, we should be able to skip blocks internally rather than requiring contiguous good blocks.
-Scott

Scott Wood wrote:
- size_t end;
- int ret_val = 0;
- end = offset + CFG_ENV_SIZE;
- for (; offset < end; offset += nand_info[0].erasesize) {
if (nand_block_isbad(&nand_info[0],offset))
ret_val = 1;
- }
- return ret_val;
size_t end = offset + CFG_ENV_SIZE;
while (offset < end) if (nand_block_isbad(&nand_info[0], offset)) return 1;
return 0;
Err, with an offset increment in the loop, of course.
-Scott

Modified to check for bad blocks and to skipping over them when CFG_ENV_RANGE has been defined. CFG_ENV_RANGE must be larger than CFG_ENV_SIZE and aligned to the NAND flash block size.
signed off by Stuart Wood stuart.wood@labxtechnologies.com ---
diff --git a/common/env_nand.c b/common/env_nand.c index 49742f5..d5f330c 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -1,4 +1,7 @@ /* + * (C) Copyright 2008 + * Stuart Wood, Lab X Technologies stuart.wood@labxtechnologies.com + * * (C) Copyright 2004 * Jian Zhang, Texas Instruments, jzhang@ti.com.
@@ -53,6 +56,10 @@ #error CONFIG_INFERNO not supported yet #endif
+#ifndef CFG_ENV_RANGE +#define CFG_ENV_RANGE CFG_ENV_SIZE +#endif + int nand_legacy_rw (struct nand_chip* nand, int cmd, size_t start, size_t len, size_t * retlen, u_char * buf); @@ -151,6 +158,34 @@ int env_init(void) * The legacy NAND code saved the environment in the first NAND device i.e., * nand_dev_desc + 0. This is also the behaviour using the new NAND code. */ +int writeenv(size_t offset, u_char * buf) +{ + size_t end = offset + CFG_ENV_RANGE; + size_t amount_saved = 0; + size_t blocksize; + + u_char *char_ptr; + + blocksize = nand_info[0].erasesize; + + while (amount_saved < CFG_ENV_SIZE && offset < end) { + if (nand_block_isbad(&nand_info[0], offset)) { + offset += blocksize; + } else { + char_ptr = &buf[amount_saved]; + if (nand_write(&nand_info[0], offset, &blocksize, char_ptr)) { + return 1; + } else { + offset += blocksize; + amount_saved += blocksize; + } + } + } + if (amount_saved != CFG_ENV_SIZE) + return 1; + + return 0; +} #ifdef CFG_ENV_OFFSET_REDUND int saveenv(void) { @@ -160,26 +195,25 @@ int saveenv(void) env_ptr->flags++; total = CFG_ENV_SIZE;
+ if (CFG_ENV_RANGE < CFG_ENV_SIZE) + return 1; if(gd->env_valid == 1) { - puts ("Erasing redundant Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET_REDUND, CFG_ENV_SIZE)) - return 1; + puts ("Erasing redundant Nand...\n"); + nand_erase(&nand_info[0], CFG_ENV_OFFSET_REDUND, CFG_ENV_RANGE); + puts ("Writing to redundant Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) env_ptr); + ret = writeenv(CFG_ENV_OFFSET_REDUND, env_ptr); } else { - puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET, CFG_ENV_SIZE)) - return 1; + puts ("Erasing Nand...\n"); + nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_RANGE);
puts ("Writing to Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) env_ptr); + ret = writeenv(CFG_ENV_OFFSET, env_ptr); } - if (ret || total != CFG_ENV_SIZE) + if (ret || total != CFG_ENV_SIZE) { + puts("FAILED!\n"); return 1; + }
puts ("done\n"); gd->env_valid = (gd->env_valid == 2 ? 1 : 2); @@ -191,15 +225,18 @@ int saveenv(void) size_t total; int ret = 0;
- puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_SIZE)) + if (CFG_ENV_RANGE < CFG_ENV_SIZE) return 1; + puts ("Erasing Nand...\n"); + nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_RANGE);
puts ("Writing to Nand... "); total = CFG_ENV_SIZE; - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); - if (ret || total != CFG_ENV_SIZE) + ret = writeenv(CFG_ENV_OFFSET, env_ptr); + if (ret || total != CFG_ENV_SIZE) { + puts("FAILED!\n"); return 1; + }
puts ("done\n"); return ret; @@ -207,11 +244,39 @@ int saveenv(void) #endif /* CFG_ENV_OFFSET_REDUND */ #endif /* CMD_SAVEENV */
+int readenv (size_t offset, u_char * buf) +{ + size_t end = offset + CFG_ENV_RANGE; + size_t amount_loaded = 0; + size_t blocksize; + + u_char *char_ptr; + + blocksize = nand_info[0].erasesize; + + while (amount_loaded < CFG_ENV_SIZE && offset < end) { + if (nand_block_isbad(&nand_info[0], offset)) { + offset += blocksize; + } else { + char_ptr = &buf[amount_loaded]; + nand_read(&nand_info[0], offset, &blocksize, char_ptr); + offset += blocksize; + amount_loaded += blocksize; + } + } + if (amount_loaded != CFG_ENV_SIZE) + return -1; + + return 0; +} + #ifdef CFG_ENV_OFFSET_REDUND void env_relocate_spec (void) { #if !defined(ENV_IS_EMBEDDED) size_t total; + size_t offset; + size_t end; int crc1_ok = 0, crc2_ok = 0; env_t *tmp_env1, *tmp_env2;
@@ -220,10 +285,10 @@ void env_relocate_spec (void) tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
- nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) tmp_env1); - nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) tmp_env2); + if (readenv(CFG_ENV_OFFSET, (u_char *) tmp_env1)) + puts("No Valid Environment Area Found\n"); + if (readenv(CFG_ENV_OFFSET_REDUND, (u_char *) tmp_env2)) + puts("No Valid Reundant Environment Area Found\n");
crc1_ok = (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc); crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc); @@ -272,7 +337,7 @@ void env_relocate_spec (void) int ret;
total = CFG_ENV_SIZE; - ret = nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); + ret = readenv(CFG_ENV_OFFSET, env_ptr); if (ret || total != CFG_ENV_SIZE) return use_default();

On Fri, May 30, 2008 at 11:14:10AM -0400, Stuart Wood wrote:
+int writeenv(size_t offset, u_char * buf)
No space after '*'.
char_ptr = &buf[amount_saved];
if (nand_write(&nand_info[0], offset, &blocksize, char_ptr)) {
Wrap long line.
return 1;
} else {
The "else" is superfluous.
puts ("Erasing redundant Nand...\n");
nand_erase(&nand_info[0], CFG_ENV_OFFSET_REDUND, CFG_ENV_RANGE);
If erasing fails for reasons other than a bad block, we should abort.
If erasing fails due to a bad block, we should skip past it; however, the current erase code fails and does not try to erase any further blocks. Use nand_erase_opts() instead.
- if (ret || total != CFG_ENV_SIZE)
- if (ret || total != CFG_ENV_SIZE) {
return 1;puts("FAILED!\n");
- }
total is no longer referenced anywhere but here and the initialization; remove it.
- while (amount_loaded < CFG_ENV_SIZE && offset < end) {
if (nand_block_isbad(&nand_info[0], offset)) {
offset += blocksize;
} else {
char_ptr = &buf[amount_loaded];
nand_read(&nand_info[0], offset, &blocksize, char_ptr);
Please check the return value of nand_read.
-Scott

Scott, I this this one is it, and thnaks for pointing out the nand_erase_opts() function.
Stuart
--- Modified to check for bad blocks and to skipping over them when CFG_ENV_RANGE has been defined. CFG_ENV_RANGE must be larger than CFG_ENV_SIZE and aligned to the NAND flash block size.
signed off by Stuart Wood stuart.wood@labxtechnologies.com ---
diff --git a/common/env_nand.c b/common/env_nand.c index 49742f5..508d7a4 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -1,4 +1,7 @@ /* + * (C) Copyright 2008 + * Stuart Wood, Lab X Technologies stuart.wood@labxtechnologies.com + * * (C) Copyright 2004 * Jian Zhang, Texas Instruments, jzhang@ti.com.
@@ -53,6 +56,10 @@ #error CONFIG_INFERNO not supported yet #endif
+#ifndef CFG_ENV_RANGE +#define CFG_ENV_RANGE CFG_ENV_SIZE +#endif + int nand_legacy_rw (struct nand_chip* nand, int cmd, size_t start, size_t len, size_t * retlen, u_char * buf); @@ -151,35 +158,71 @@ int env_init(void) * The legacy NAND code saved the environment in the first NAND device i.e., * nand_dev_desc + 0. This is also the behaviour using the new NAND code. */ +int writeenv(size_t offset, u_char *buf) +{ + size_t end = offset + CFG_ENV_RANGE; + size_t amount_saved = 0; + size_t blocksize; + + u_char *char_ptr; + + blocksize = nand_info[0].erasesize; + + while (amount_saved < CFG_ENV_SIZE && offset < end) { + if (nand_block_isbad(&nand_info[0], offset)) { + offset += blocksize; + } else { + char_ptr = &buf[amount_saved]; + if (nand_write(&nand_info[0], offset, &blocksize, + char_ptr)) + return 1; + offset += blocksize; + amount_saved += blocksize; + } + } + if (amount_saved != CFG_ENV_SIZE) + return 1; + + return 0; +} #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; + nand_erase_options.jffs2 = 0; + nand_erase_options.scrub = 0; + + if (CFG_ENV_RANGE < CFG_ENV_SIZE) + return 1; if(gd->env_valid == 1) { - puts ("Erasing redundant Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET_REDUND, CFG_ENV_SIZE)) + puts ("Erasing redundant Nand...\n"); + nand_erase_options.offset = CFG_ENV_OFFSET_REDUND; + if (nand_erase_opts(&nand_info[0], &nand_erase_options)) return 1; + puts ("Writing to redundant Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) env_ptr); + ret = writeenv(CFG_ENV_OFFSET_REDUND, (u_char *) env_ptr); } else { - puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], - CFG_ENV_OFFSET, CFG_ENV_SIZE)) + puts ("Erasing Nand...\n"); + nand_erase_options.offset = CFG_ENV_OFFSET; + if (nand_erase_opts(&nand_info[0], &nand_erase_options)) return 1;
puts ("Writing to Nand... "); - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) env_ptr); + ret = writeenv(CFG_ENV_OFFSET, (u_char *) env_ptr); } - if (ret || total != CFG_ENV_SIZE) + if (ret) { + puts("FAILED!\n"); return 1; + }
puts ("done\n"); gd->env_valid = (gd->env_valid == 2 ? 1 : 2); @@ -191,15 +234,24 @@ int saveenv(void) size_t total; int ret = 0;
- puts ("Erasing Nand..."); - if (nand_erase(&nand_info[0], CFG_ENV_OFFSET, CFG_ENV_SIZE)) + nand_erase_options.length = CFG_ENV_RANGE; + nand_erase_options.quiet = 0; + nand_erase_options.jffs2 = 0; + nand_erase_options.scrub = 0; + nand_erase_options.offset = CFG_ENV_OFFSET; + + if (CFG_ENV_RANGE < CFG_ENV_SIZE) + return 1; + puts ("Erasing Nand...\n"); + if (nand_erase_opts(&nand_info[0], &nand_erase_options)) return 1;
puts ("Writing to Nand... "); total = CFG_ENV_SIZE; - ret = nand_write(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); - if (ret || total != CFG_ENV_SIZE) + if (writeenv(CFG_ENV_OFFSET, env_ptr)) { + puts("FAILED!\n"); return 1; + }
puts ("done\n"); return ret; @@ -207,6 +259,33 @@ int saveenv(void) #endif /* CFG_ENV_OFFSET_REDUND */ #endif /* CMD_SAVEENV */
+int readenv (size_t offset, u_char * buf) +{ + size_t end = offset + CFG_ENV_RANGE; + size_t amount_loaded = 0; + size_t blocksize; + + u_char *char_ptr; + + blocksize = nand_info[0].erasesize; + + while (amount_loaded < CFG_ENV_SIZE && offset < end) { + if (nand_block_isbad(&nand_info[0], offset)) { + offset += blocksize; + } else { + char_ptr = &buf[amount_loaded]; + if (nand_read(&nand_info[0], offset, &blocksize, char_ptr)) + return 1; + offset += blocksize; + amount_loaded += blocksize; + } + } + if (amount_loaded != CFG_ENV_SIZE) + return 1; + + return 0; +} + #ifdef CFG_ENV_OFFSET_REDUND void env_relocate_spec (void) { @@ -220,10 +299,10 @@ void env_relocate_spec (void) tmp_env1 = (env_t *) malloc(CFG_ENV_SIZE); tmp_env2 = (env_t *) malloc(CFG_ENV_SIZE);
- nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, - (u_char*) tmp_env1); - nand_read(&nand_info[0], CFG_ENV_OFFSET_REDUND, &total, - (u_char*) tmp_env2); + if (readenv(CFG_ENV_OFFSET, (u_char *) tmp_env1)) + puts("No Valid Environment Area Found\n"); + if (readenv(CFG_ENV_OFFSET_REDUND, (u_char *) tmp_env2)) + puts("No Valid Reundant Environment Area Found\n");
crc1_ok = (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc); crc2_ok = (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc); @@ -272,7 +351,7 @@ void env_relocate_spec (void) int ret;
total = CFG_ENV_SIZE; - ret = nand_read(&nand_info[0], CFG_ENV_OFFSET, &total, (u_char*)env_ptr); + ret = readenv(CFG_ENV_OFFSET, env_ptr); if (ret || total != CFG_ENV_SIZE) return use_default();

On Fri, May 30, 2008 at 04:05:28PM -0400, Stuart Wood wrote:
Scott, I this this one is it, and thnaks for pointing out the nand_erase_opts() function.
Stuart
Modified to check for bad blocks and to skipping over them when CFG_ENV_RANGE has been defined. CFG_ENV_RANGE must be larger than CFG_ENV_SIZE and aligned to the NAND flash block size.
signed off by Stuart Wood stuart.wood@labxtechnologies.com
Applied to u-boot-nand-flash.
-Scott
participants (2)
-
Scott Wood
-
Stuart Wood