[PATCH 0/2] mtd: oobtest: Add parameter to ignore bitflip errors within specified limit

[PATCH 0/2] mtd: oobtest: Add parameter to ignore bitflip errors within specified limit

[Roger Quadros]

Hi,

The oobtest case uses raw NAND read/writes to OOB area bypassing the error correction
mechanism and hence is bound to be affected by bitflip errors which are normal
in NAND memories. (e.g. we can never get DRA7-evm's NAND to fully pass
mtd_oobtest).

In these patches we add a module parameter "bitflip_limit" to specify how many
bitflips per page are tolerable. Not specifiing the parameter defaults to old
behaviour (i.e. zero bitflips tolerated).
Specifying bitflip_limit=1 makes us pass on DRA7-evm with 0 errors.

Introduce a new memcmpshow() function that shows the data byte where comparison failed.
This is useful for debugging. The same function is also used to calculate number of
bitflip errors over the data block.

cheers,
-roger

Roger Quadros (2):
  mtd: mtd_oobtest: Show the verification error location and data
  mtd: mtd_oobtest: add bitflip_limit parameter

 drivers/mtd/tests/oobtest.c | 77 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 64 insertions(+), 13 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] mtd: mtd_oobtest: Show the verification error location and data

[Roger Quadros]

Add a function memcmpshow() that compares the 2 data buffers
and shows the address:offset and data bytes on comparison failure.
This function  does not break at a comparison failure but runs the
check for the whole data buffer.

Use memcmpshow() instead of memcmp() for all the verification paths.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/mtd/tests/oobtest.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index dc4f960..edd859d 100644
--- a/drivers/mtd/tests/oobtest.c
+++ b/drivers/mtd/tests/oobtest.c
@@ -115,6 +115,26 @@ static int write_whole_device(void)
 	return 0;
 }
 
+/* Display the address, offset and data bytes at comparison failure */
+static int memcmpshow(loff_t addr, const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res;
+	int ret = 0;
+	size_t i = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--, i++) {
+		res = *su1 ^ *su2;
+		if (res) {
+			pr_info("error @addr[0x%lx:0x%x] 0x%x -> 0x%x diff 0x%x\n",
+				(unsigned long)addr, i, *su1, *su2, res);
+			ret = 1;
+		}
+	}
+
+	return ret;
+}
+
 static int verify_eraseblock(int ebnum)
 {
 	int i;
@@ -139,8 +159,9 @@ static int verify_eraseblock(int ebnum)
 			errcnt += 1;
 			return err ? err : -1;
 		}
-		if (memcmp(readbuf, writebuf + (use_len_max * i) + use_offset,
-			   use_len)) {
+		if (memcmpshow(addr, readbuf,
+			       writebuf + (use_len_max * i) + use_offset,
+			       use_len)) {
 			pr_err("error: verify failed at %#llx\n",
 			       (long long)addr);
 			errcnt += 1;
@@ -167,9 +188,9 @@ static int verify_eraseblock(int ebnum)
 				errcnt += 1;
 				return err ? err : -1;
 			}
-			if (memcmp(readbuf + use_offset,
-				   writebuf + (use_len_max * i) + use_offset,
-				   use_len)) {
+			if (memcmpshow(addr, readbuf + use_offset,
+				       writebuf + (use_len_max * i) + use_offset,
+				       use_len)) {
 				pr_err("error: verify failed at %#llx\n",
 						(long long)addr);
 				errcnt += 1;
@@ -233,7 +254,7 @@ static int verify_eraseblock_in_one_go(int ebnum)
 		errcnt += 1;
 		return err ? err : -1;
 	}
-	if (memcmp(readbuf, writebuf, len)) {
+	if (memcmpshow(addr, readbuf, writebuf, len)) {
 		pr_err("error: verify failed at %#llx\n",
 		       (long long)addr);
 		errcnt += 1;
@@ -610,7 +631,8 @@ static int __init mtd_oobtest_init(void)
 		err = mtd_read_oob(mtd, addr, &ops);
 		if (err)
 			goto out;
-		if (memcmp(readbuf, writebuf, mtd->ecclayout->oobavail * 2)) {
+		if (memcmpshow(addr, readbuf, writebuf,
+			       mtd->ecclayout->oobavail * 2)) {
 			pr_err("error: verify failed at %#llx\n",
 			       (long long)addr);
 			errcnt += 1;
-- 
1.8.3.2

[PATCH 2/2] mtd: mtd_oobtest: add bitflip_limit parameter

[Roger Quadros]

It is common for NAND devices to have bitflip errors.
Add a bitflip_limit parameter to specify how many bitflips per
page we can tolerate without flagging an error.
By default zero bitflips are tolerated.

Signed-off-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/mtd/tests/oobtest.c | 65 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 47 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
index edd859d..1fd2cda 100644
--- a/drivers/mtd/tests/oobtest.c
+++ b/drivers/mtd/tests/oobtest.c
@@ -34,8 +34,11 @@
 #include "mtd_test.h"
 
 static int dev = -EINVAL;
+static int bitflip_limit;
 module_param(dev, int, S_IRUGO);
 MODULE_PARM_DESC(dev, "MTD device number to use");
+module_param(bitflip_limit, int, S_IRUGO);
+MODULE_PARM_DESC(bitflip_limit, "Max. allowed bitflips per page");
 
 static struct mtd_info *mtd;
 static unsigned char *readbuf;
@@ -115,24 +118,27 @@ static int write_whole_device(void)
 	return 0;
 }
 
-/* Display the address, offset and data bytes at comparison failure */
-static int memcmpshow(loff_t addr, const void *cs, const void *ct, size_t count)
+/*
+ * Display the address, offset and data bytes at comparison failure.
+ * Return number of bitflips encountered.
+ */
+static size_t memcmpshow(loff_t addr, const void *cs, const void *ct, size_t count)
 {
 	const unsigned char *su1, *su2;
 	int res;
-	int ret = 0;
 	size_t i = 0;
+	size_t bitflips = 0;
 
 	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--, i++) {
 		res = *su1 ^ *su2;
 		if (res) {
 			pr_info("error @addr[0x%lx:0x%x] 0x%x -> 0x%x diff 0x%x\n",
 				(unsigned long)addr, i, *su1, *su2, res);
-			ret = 1;
+			bitflips += hweight8(res);
 		}
 	}
 
-	return ret;
+	return bitflips;
 }
 
 static int verify_eraseblock(int ebnum)
@@ -141,6 +147,7 @@ static int verify_eraseblock(int ebnum)
 	struct mtd_oob_ops ops;
 	int err = 0;
 	loff_t addr = (loff_t)ebnum * mtd->erasesize;
+	size_t bitflips;
 
 	prandom_bytes_state(&rnd_state, writebuf, use_len_max * pgcnt);
 	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
@@ -159,9 +166,11 @@ static int verify_eraseblock(int ebnum)
 			errcnt += 1;
 			return err ? err : -1;
 		}
-		if (memcmpshow(addr, readbuf,
-			       writebuf + (use_len_max * i) + use_offset,
-			       use_len)) {
+
+		bitflips = memcmpshow(addr, readbuf,
+				      writebuf + (use_len_max * i) + use_offset,
+				      use_len);
+		if (bitflips > bitflip_limit) {
 			pr_err("error: verify failed at %#llx\n",
 			       (long long)addr);
 			errcnt += 1;
@@ -169,7 +178,10 @@ static int verify_eraseblock(int ebnum)
 				pr_err("error: too many errors\n");
 				return -1;
 			}
+		} else if (bitflips) {
+			pr_info("ignoring error as within bitflip_limit\n");
 		}
+
 		if (use_offset != 0 || use_len < mtd->ecclayout->oobavail) {
 			int k;
 
@@ -188,9 +200,10 @@ static int verify_eraseblock(int ebnum)
 				errcnt += 1;
 				return err ? err : -1;
 			}
-			if (memcmpshow(addr, readbuf + use_offset,
-				       writebuf + (use_len_max * i) + use_offset,
-				       use_len)) {
+			bitflips = memcmpshow(addr, readbuf + use_offset,
+					      writebuf + (use_len_max * i) + use_offset,
+					      use_len);
+			if (bitflips > bitflip_limit) {
 				pr_err("error: verify failed at %#llx\n",
 						(long long)addr);
 				errcnt += 1;
@@ -198,7 +211,10 @@ static int verify_eraseblock(int ebnum)
 					pr_err("error: too many errors\n");
 					return -1;
 				}
+			} else if (bitflips) {
+				pr_info("ignoring error as within bitflip_limit\n");
 			}
+
 			for (k = 0; k < use_offset; ++k)
 				if (readbuf[k] != 0xff) {
 					pr_err("error: verify 0xff "
@@ -237,6 +253,9 @@ static int verify_eraseblock_in_one_go(int ebnum)
 	int err = 0;
 	loff_t addr = (loff_t)ebnum * mtd->erasesize;
 	size_t len = mtd->ecclayout->oobavail * pgcnt;
+	size_t oobavail = mtd->ecclayout->oobavail;
+	size_t bitflips;
+	int i;
 
 	prandom_bytes_state(&rnd_state, writebuf, len);
 	ops.mode      = MTD_OPS_AUTO_OOB;
@@ -247,6 +266,8 @@ static int verify_eraseblock_in_one_go(int ebnum)
 	ops.ooboffs   = 0;
 	ops.datbuf    = NULL;
 	ops.oobbuf    = readbuf;
+
+	/* read entire block's OOB at one go */
 	err = mtd_read_oob(mtd, addr, &ops);
 	if (err || ops.oobretlen != len) {
 		pr_err("error: readoob failed at %#llx\n",
@@ -254,13 +275,21 @@ static int verify_eraseblock_in_one_go(int ebnum)
 		errcnt += 1;
 		return err ? err : -1;
 	}
-	if (memcmpshow(addr, readbuf, writebuf, len)) {
-		pr_err("error: verify failed at %#llx\n",
-		       (long long)addr);
-		errcnt += 1;
-		if (errcnt > 1000) {
-			pr_err("error: too many errors\n");
-			return -1;
+
+	/* verify one page OOB at a time for bitflip per page limit check */
+	for (i = 0; i < pgcnt; ++i, addr += mtd->writesize) {
+		bitflips = memcmpshow(addr, readbuf + (i * oobavail),
+				      writebuf + (i * oobavail), oobavail);
+		if (bitflips > bitflip_limit) {
+			pr_err("error: verify failed at %#llx\n",
+			       (long long)addr);
+			errcnt += 1;
+			if (errcnt > 1000) {
+				pr_err("error: too many errors\n");
+				return -1;
+			}
+		} else if (bitflips) {
+			pr_info("ignoring error as within bitflip_limit\n");
 		}
 	}
 
-- 
1.8.3.2

Re: [PATCH 0/2] mtd: oobtest: Add parameter to ignore bitflip errors within specified limit

[Roger Quadros]

Hi,

On 10/21/2014 04:53 PM, Roger Quadros wrote:
> Hi,
> 
> The oobtest case uses raw NAND read/writes to OOB area bypassing the error correction
> mechanism and hence is bound to be affected by bitflip errors which are normal
> in NAND memories. (e.g. we can never get DRA7-evm's NAND to fully pass
> mtd_oobtest).
> 
> In these patches we add a module parameter "bitflip_limit" to specify how many
> bitflips per page are tolerable. Not specifiing the parameter defaults to old
> behaviour (i.e. zero bitflips tolerated).
> Specifying bitflip_limit=1 makes us pass on DRA7-evm with 0 errors.
> 
> Introduce a new memcmpshow() function that shows the data byte where comparison failed.
> This is useful for debugging. The same function is also used to calculate number of
> bitflip errors over the data block.

Any comments on this series?

cheers,
-roger

> 
> Roger Quadros (2):
>   mtd: mtd_oobtest: Show the verification error location and data
>   mtd: mtd_oobtest: add bitflip_limit parameter
> 
>  drivers/mtd/tests/oobtest.c | 77 +++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 64 insertions(+), 13 deletions(-)
> 

Re: [PATCH 0/2] mtd: oobtest: Add parameter to ignore bitflip errors within specified limit

[Brian Norris]

On Tue, Oct 21, 2014 at 04:53:26PM +0300, Roger Quadros wrote:
> Hi,
> 
> The oobtest case uses raw NAND read/writes to OOB area bypassing the error correction
> mechanism

Hmm, I suppose that's the case. I intended for
ecc.{read,write}_oob_raw() vs. ecc.{read,write}_oob() to provide a
distinction, but most drivers use identical implementations for both,
and both are 'raw.' Now that I think about it again, I'm not sure
there's really a good use for an ECC-protected OOB read/write operation.
If you're intending to use ECC, you should be writing data+OOB (i.e.,
nand_do_{read,write}_ops()).

> and hence is bound to be affected by bitflip errors which are normal
> in NAND memories. (e.g. we can never get DRA7-evm's NAND to fully pass
> mtd_oobtest).
> 
> In these patches we add a module parameter "bitflip_limit" to specify how many
> bitflips per page are tolerable. Not specifiing the parameter defaults to old
> behaviour (i.e. zero bitflips tolerated).
> Specifying bitflip_limit=1 makes us pass on DRA7-evm with 0 errors.
> 
> Introduce a new memcmpshow() function that shows the data byte where comparison failed.
> This is useful for debugging. The same function is also used to calculate number of
> bitflip errors over the data block.

It's possible this sort of function could be useful for the other test
modules too. But we can keep it local for now.

Pushed both to l2-mtd.git.

Brian