PATCH - Fix oob data copied into supplied buffer

This patch correctly sets the oobavail variable and fixes a bug where the oob data was not valid when there where multiple groups in oobfree.
It also defines a new variable so that you can tell which parts of a large page have ecc errors.

Signed-off-by Troy Kisky <[email protected]>

drivers/nand/nand_base.c +0 -0

index 151f535..45b8fb0 100644
@@ -931,7 +931,7 @@ static int nand_write_page (struct mtd_info *mtd,

struct nand_chip *this, int pa

drivers/nand/nand_base.c +0 -0

 			for (i = 0; i < eccbytes; i++, eccidx++)
 				oob_buf[oob_config[eccidx]] =

ecc_code[i];

drivers/nand/nand_base.c +0 -1

 			/* If the hardware ecc provides syndromes then
-			 * the ecc code must be written immidiately

after

drivers/nand/nand_base.c +1 -0

+			 * the ecc code must be written immediately

after

drivers/nand/nand_base.c +0 -0

 			 * the data bytes (words) */
 			if (this->options & NAND_HWECC_SYNDROME)
 				this->write_buf(mtd, ecc_code,

eccbytes);

drivers/nand/nand_base.c +0 -0

@@ -1092,7 +1092,7 @@ static int nand_read (struct mtd_info *mtd, loff_t

from, size_t len, size_t * re

drivers/nand/nand_base.c +2 -1

 	return nand_read_ecc (mtd, from, len, retlen, buf, NULL, NULL);
 }
 
-
+ulong eccReadMask;	/* bitmask off ecc err groups*/
 /**
  * nand_read_ecc - [MTD Interface] Read data with ECC
  * @mtd:	MTD device structure

@@ -1267,6 +1267,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
 loff_t from, size_t len,
 			ecc_code[j] = oob_data[oob_config[j]];
 
 		/* correct data, if neccecary */
+		eccReadMask = 0;
 		for (i = 0, j = 0, datidx = 0; i < this->eccsteps; i++,
 datidx += ecc) {

ecc_status = this->correct_data(mtd,
&data_poi[datidx], &ecc_code[j], &ecc_calc[j]);

drivers/nand/nand_base.c +1 -0

@@ -1283,6 +1284,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
 loff_t from, size_t len,
 			}
 
 			if (ecc_status == -1) {
+				eccReadMask |= (1<<i);
 				DEBUG (MTD_DEBUG_LEVEL0, "nand_read_ecc:
 " "Failed ECC read, page 0x%08x\n", page);

drivers/nand/nand_base.c +0 -1

 				ecc_failed++;
 			}

@@ -1299,7 +1301,7 @@ static int nand_read_ecc (struct mtd_info *mtd,
 loff_t from, size_t len,
 				for (i = 0, j = 0; j < mtd->oobavail;
 i++) {
 					int from =
 oobsel->oobfree[i][0];
 					int num = oobsel->oobfree[i][1];
-					memcpy(&oob_buf[oob],

&oob_data[from], num);

drivers/nand/nand_base.c +1 -0

+					memcpy(&oob_buf[oob+j],

&oob_data[from], num);

drivers/nand/nand_base.c +3 -1

 					j+= num;
 				}
 				oob += mtd->oobavail;

@@ -1644,8 +1646,10 @@ static int nand_write_ecc (struct mtd_info *mtd,
 loff_t to, size_t len,
 	this->select_chip(mtd, chipnr);
 
 	/* Check, if it is write protected */
-	if (nand_check_wp(mtd))
+	if (nand_check_wp(mtd)) {
+		printk (KERN_NOTICE "nand_write_ecc: Device is write
 protected\n");
 		goto out;
+	}
 

drivers/nand/nand_base.c +0 -0

 	/* if oobsel is NULL, use chip defaults */
 	if (oobsel == NULL)

@@ -2486,12 +2490,9 @@ int nand_scan (struct mtd_info *mtd, int
 maxchips)
 
 	/* The number of bytes available for the filesystem to place fs

dependend

drivers/nand/nand_base.c +5 -8

 	 * oob data */
-	if (this->options & NAND_BUSWIDTH_16) {
-		mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes
+ 2);
-		if (this->autooob->eccbytes & 0x01)
-			mtd->oobavail--;
-	} else
-		mtd->oobavail = mtd->oobsize - (this->autooob->eccbytes
+ 1);
+	mtd->oobavail = 0;
+	for (i=0; this->autooob->oobfree[i][1]; i++)
+		mtd->oobavail += this->autooob->oobfree[i][1];
 
 	/*
 	 * check ECC mode, default to software
 
------------------------------------------------------------------------
-

This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users at lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

On Thu, 20 Sep 2007, Troy Kisky wrote:

This doesn't look right to me.

First chunk fixes a minor spelling error. Then I can't understand why introduce a new variable. Everything works fine with existing structures and
it's been like this for ages, both in Linux kernel and U-Boot.

Quoted 6 lines

First chunk fixes a minor spelling error. Then I can't understand why introduce a new variable. Everything works fine with existing structures and
it's been like this for ages, both in Linux kernel and U-Boot.

If you look at the kernel, you'll see that oobavail is set there the way I did it. The other bug doesn't show up unless you have multiple oobfree groups. The new variable is used when reading or writing pages that an initial program loader reads. It's purpose is to correct pages without using a bad block table. I'll submit that code if this is approved.

Quoted 9 lines · In message <[email protected]> you wrote:

Quoted 5 lines

First chunk fixes a minor spelling error. Then I can't understand why introduce a new variable. Everything works fine with existing structures and
it's been like this for ages, both in Linux kernel and U-Boot.

If you look at the kernel, you'll see that oobavail is set there the way I did it. The other bug doesn't show up unless you have multiple oobfree groups. The new variable is used when reading or writing pages that an initial program loader reads. It's purpose is to correct pages without using a bad block table. I'll submit that code if this is approved.

kis's comments were legitimate.

Please add this variable only when adding code that uses it. I will always reject patches that attempt to add unused code or variables.

Also, please describe *all* your changes.

Best regards,

Wolfgang Denk

First segment fixes a typo
Second segment fixes a bug where oob data may be copied incorrectly. Third segment adds an error message when exiting due to write protect. Forth segment fixes a bug where oobavail may be set incorrectly.

All segments are independent and may be accepted or rejected as deemed appropriate. Do they need to be separate patches?

Signed-off-by Troy Kisky <[email protected]>