[PATCH 0/3] fixups for OOB ECC problems

[PATCH 0/3] fixups for OOB ECC problems

[Brian Norris]

Hello,

Now that we have added the possibility that mtd->read_oob returns
-EUCLEAN or -EBADMSG for corrected bitflips and for uncorrectable errors,
respectively, there are more pieces of code that need to take this into
account. These patches should address some of these by ignoring bitflips
when bit accuracy is not needed or by ignoring corrected errors when
scrubbing would be unnecessary.

There are still other cases to address, for example, some flash-based BBT
code.

This applies on top of l2-mtd-2.6.git. Patch 3 is an amendment to commit
abf386c475beab813dc9b3366e592e00255e987a, ("mtd: nand: handle ECC errors
in OOB"), so it can be squashed in if you want. Let me know if you want
me to do this and do a resend.

Brian

Brian Norris (3):
  mtd: nand: ignore ECC errors for simple BBM scans
  mtd: tests: ignore corrected bitflips in OOB on mtd_readtest
  mtd: edit NAND-related comment

 drivers/mtd/mtdchar.c            |    5 +++--
 drivers/mtd/nand/nand_bbt.c      |   13 ++++++++++---
 drivers/mtd/tests/mtd_readtest.c |    3 ++-
 3 files changed, 15 insertions(+), 6 deletions(-)

[PATCH 1/3] mtd: nand: ignore ECC errors for simple BBM scans

[Brian Norris]

Now that nand_do_readoob() may return -EUCLEAN or -EBADMSG on ECC errors,
we need to handle the return value specially in some cases.

When scanning for simple bad block markers, reacting to an ECC error is
not very useful, as we assume that the relevant markers are still
non-0xFF for true bad blocks.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_bbt.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index f30807c..a4fcbf1 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -312,14 +312,20 @@ static int scan_read_raw_oob(struct mtd_info *mtd, uint8_t *buf, loff_t offs,
 			ops.oobbuf = buf + len;
 			ops.datbuf = buf;
 			ops.len = len;
-			return mtd->read_oob(mtd, offs, &ops);
+			res = mtd->read_oob(mtd, offs, &ops);
+
+			/* Ignore ECC errors when checking for BBM */
+			if (res != -EUCLEAN && res != -EBADMSG)
+				return res;
+			return 0;
 		} else {
 			ops.oobbuf = buf + mtd->writesize;
 			ops.datbuf = buf;
 			ops.len = mtd->writesize;
 			res = mtd->read_oob(mtd, offs, &ops);
 
-			if (res)
+			/* Ignore ECC errors when checking for BBM */
+			if (res && res != -EUCLEAN && res != -EBADMSG)
 				return res;
 		}
 
@@ -435,7 +441,8 @@ static int scan_block_fast(struct mtd_info *mtd, struct nand_bbt_descr *bd,
 		 * byte reads for 16 bit buswidth.
 		 */
 		ret = mtd->read_oob(mtd, offs, &ops);
-		if (ret)
+		/* Ignore ECC errors when checking for BBM */
+		if (ret && ret != -EUCLEAN && ret != -EBADMSG)
 			return ret;
 
 		if (check_short_pattern(buf, bd))
-- 
1.7.0.4

[PATCH 2/3] mtd: tests: ignore corrected bitflips in OOB on mtd_readtest

[Brian Norris]

read_oob may now return ECC error codes. If the code is -EUCLEAN, then
we can safely ignore the error as a corrected bitflip.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/tests/mtd_readtest.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/tests/mtd_readtest.c b/drivers/mtd/tests/mtd_readtest.c
index afe71aa..de23e8e 100644
--- a/drivers/mtd/tests/mtd_readtest.c
+++ b/drivers/mtd/tests/mtd_readtest.c
@@ -75,7 +75,8 @@ static int read_eraseblock_by_page(int ebnum)
 			ops.datbuf    = NULL;
 			ops.oobbuf    = oobbuf;
 			ret = mtd->read_oob(mtd, addr, &ops);
-			if (ret || ops.oobretlen != mtd->oobsize) {
+			if (ret && ret != -EUCLEAN ||
+					ops.oobretlen != mtd->oobsize) {
 				printk(PRINT_PREF "error: read oob failed at "
 						  "%#llx\n", (long long)addr);
 				if (!err)
-- 
1.7.0.4

[PATCH 3/3] mtd: edit NAND-related comment

[Brian Norris]

This comment was unclear regarding which NAND functions do and do not
support ECC on the spare area. This update should reflect the current
status of the NAND system but can be updated if changes are made in
the standard functions.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/mtdchar.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 0995e8e..978e084 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -482,8 +482,9 @@ static int mtd_do_readoob(struct mtd_info *mtd, uint64_t start,
 	 * to signal the caller that a bitflip has occured and has
 	 * been corrected by the ECC algorithm.
 	 *
-	 * Note: most NAND ECC algorithms do not calculate ECC
-	 * for the OOB area.
+	 * Note: currently the standard NAND function, nand_read_oob_std,
+	 * does not calculate ECC for the OOB area, so do not rely on
+	 * this behavior unless you have replaced it with your own.
 	 */
 	if (ret == -EUCLEAN || ret == -EBADMSG)
 		return 0;
-- 
1.7.0.4

Re: [PATCH 0/3] fixups for OOB ECC problems

[Artem Bityutskiy]

On Tue, 2011-06-28 at 16:28 -0700, Brian Norris wrote:
> Hello,
> 
> Now that we have added the possibility that mtd->read_oob returns
> -EUCLEAN or -EBADMSG for corrected bitflips and for uncorrectable errors,
> respectively, there are more pieces of code that need to take this into
> account. These patches should address some of these by ignoring bitflips
> when bit accuracy is not needed or by ignoring corrected errors when
> scrubbing would be unnecessary.
> 
> There are still other cases to address, for example, some flash-based BBT
> code.
> 
> This applies on top of l2-mtd-2.6.git. Patch 3 is an amendment to commit
> abf386c475beab813dc9b3366e592e00255e987a, ("mtd: nand: handle ECC errors
> in OOB"), so it can be squashed in if you want. Let me know if you want
> me to do this and do a resend.

Do you test these things somehow? I'd just injected some code to a
driver or nandsim which returns -EUCLEAN for OOB and check what fails.

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 1/3] mtd: nand: ignore ECC errors for simple BBM scans

[Artem Bityutskiy]

On Tue, 2011-06-28 at 16:28 -0700, Brian Norris wrote:
> Now that nand_do_readoob() may return -EUCLEAN or -EBADMSG on ECC errors,
> we need to handle the return value specially in some cases.
> 
> When scanning for simple bad block markers, reacting to an ECC error is
> not very useful, as we assume that the relevant markers are still
> non-0xFF for true bad blocks.
> 
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/nand_bbt.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)

Pushed to l2-mtd-2.6.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH 0/3] fixups for OOB ECC problems

[Brian Norris]

On Tue, Jun 28, 2011 at 11:59 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Tue, 2011-06-28 at 16:28 -0700, Brian Norris wrote:
>> Now that we have added the possibility that mtd->read_oob returns
>> -EUCLEAN or -EBADMSG for corrected bitflips and for uncorrectable errors,
>> respectively, there are more pieces of code that need to take this into
>> account. These patches should address some of these by ignoring bitflips
>> when bit accuracy is not needed or by ignoring corrected errors when
>> scrubbing would be unnecessary.
>
> Do you test these things somehow? I'd just injected some code to a
> driver or nandsim which returns -EUCLEAN for OOB and check what fails.

Yes, I've done some testing.

Particularly, I was seeing a problem on boot-time BBM scan (scanning
each block's spare area for a bad block marker), since every ECC error
(correctable or uncorrectable) caused the scan to panic and quit. So
if there were any ECC problems on the whole device, our BBM scan would
fail and the device wouldn't be initialized. I tested this fix on this
particular BBM scan issue, and I've tested some other general OOB
injections and it looks good for now.

Brian