Skip to contentSkip to conversation

env_nand.c Added bad block management for environment variables

PATCH New Stuart Wood 2 participants 9 messages May 27, 2008 – Jun 2, 2008

lore
Share & tools
lore

Stuart Wood

author
#0 lore raw

PATCH 1/1 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.

common/env_nand.c +73 -16
index 49742f5..29aafa2 100644
@@ -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

#1 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

Quoted 7 lines · Stuart Wood

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.

Quoted 5 lines
common/env_nand.c +1 -3
 	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");

common/env_nand.c int env_init(void) · line 177

Quoted 10 lines
common/env_nand.c +10 -0
+		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;
Quoted 16 lines
common/env_nand.c +7 -2
@@ -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],

common/env_nand.c int saveenv(void) · line 237

Quoted 10 lines
common/env_nand.c +10 -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.

Quoted 9 lines
common/env_nand.c +9 -0
+	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;
+		}
+	}

common/env_nand.c void env_relocate_spec (void) · line 283

Quoted 10 lines
common/env_nand.c +10 -0
+	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

author
#2 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

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.

common/env_nand.c +90 -15
index 49742f5..3ee42e0 100644
@@ -1,4 +1,7 @@
 /*
+ * (C) Copywrite 2008
+ * Stuart Wood, Lab X Technologies <[email protected]>
+ *
  * (C) Copyright 2004
  * Jian Zhang, Texas Instruments, jzhang at 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);
Signature · 8 lines
Stuart Wood

Lab X Technologies, LLC
176 Anderson Ave.
Suite 302
Rochester, NY 14607
Phone: (585) 271-7790 x207
Fax: (585) 473.4707
#3 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

Quoted 6 lines · Stuart Wood

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.

common/env_nand.c line 5

Quoted 10 lines
common/env_nand.c +3 -0
index 49742f5..3ee42e0 100644
@@ -1,4 +1,7 @@
 /*
+ * (C) Copywrite 2008
+ * Stuart Wood, Lab X Technologies <[email protected]>
+ *
  * (C) Copyright 2004

It's spelled "Copyright" (as in the "right" to "copy"). Just like the one below it. :-)

Quoted 14 lines
common/env_nand.c +4 -0
  * Jian Zhang, Texas Instruments, jzhang at 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);

common/env_nand.c int env_init(void) · line 163

Quoted 5 lines
common/env_nand.c +2 -0
@@ -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.

common/env_nand.c int env_init(void) · line 168

Quoted 5 lines
common/env_nand.c +5 -0
+	size_t end;
+
+	end = offset + CFG_ENV_RANGE;
+
+	while (offset < end && nand_erase(&nand_info[0],offset, CFG_ENV_SIZE))

Spaces after commas.

common/env_nand.c int env_init(void) · line 175

Quoted 7 lines
common/env_nand.c +7 -0
+		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.

common/env_nand.c int saveenv(void) · line 254

Quoted 6 lines
common/env_nand.c +2 -0
@@ -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.

common/env_nand.c int saveenv(void) · line 264

Quoted 10 lines
common/env_nand.c +10 -0
+	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;

Quoted 8 lines
common/env_nand.c +0 -4
@@ -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);

common/env_nand.c void env_relocate_spec (void) · line 290

Quoted 10 lines
common/env_nand.c +10 -0
+	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

#4 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

Quoted 13 lines · Scott Wood

common/env_nand.c int saveenv(void) · line 264

Quoted 10 lines
common/env_nand.c +10 -0
+	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;

Quoted 5 lines

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

author
#5 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

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 <[email protected]>
---

common/env_nand.c +87 -22
index 49742f5..d5f330c 100644
@@ -1,4 +1,7 @@
 /*
+ * (C) Copyright 2008
+ * Stuart Wood, Lab X Technologies <[email protected]>
+ *
  * (C) Copyright 2004
  * Jian Zhang, Texas Instruments, jzhang at 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();
#6 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

Quoted 1 line · Stuart Wood

+int writeenv(size_t offset, u_char * buf)

No space after '*'.

common/env_nand.c int env_init(void) · line 176

Quoted 2 lines
common/env_nand.c +2 -0
+			char_ptr = &buf[amount_saved];
+			if (nand_write(&nand_info[0], offset, &blocksize, char_ptr)) {

Wrap long line.

common/env_nand.c int env_init(void) · line 178

Quoted 2 lines
common/env_nand.c +2 -0
+				return 1;
+			} else {

The "else" is superfluous.

common/env_nand.c int saveenv(void) · line 203

Quoted 3 lines
common/env_nand.c +3 -0
+		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.

common/env_nand.c int saveenv(void) · line 216

Quoted 5 lines
common/env_nand.c +3 -1
-	if (ret || total != CFG_ENV_SIZE)
+	if (ret || total != CFG_ENV_SIZE) {
+		puts("FAILED!\n");
 		return 1;
+	}

total is no longer referenced anywhere but here and the initialization; remove it.

common/env_nand.c int saveenv(void) · line 262

Quoted 6 lines
common/env_nand.c +6 -0
+	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

author
#7 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

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 <[email protected]>
---

common/env_nand.c +99 -20
index 49742f5..508d7a4 100644
@@ -1,4 +1,7 @@
 /*
+ * (C) Copyright 2008
+ * Stuart Wood, Lab X Technologies <[email protected]>
+ *
  * (C) Copyright 2004
  * Jian Zhang, Texas Instruments, jzhang at 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();
#8 lore raw

PATCH 1/1 env_nand.c Added bad block management for environment variables

Quoted 5 lines · Stuart Wood

Scott, I this this one is it, and thnaks for pointing out the nand_erase_opts() function.

Stuart

Quoted 7 lines

---
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 <[email protected]>

Applied to u-boot-nand-flash.

-Scott