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