[PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[Mike Dunn]

This v2 of this set includes the following changes:
  - [Patch 4/7]: Propagate bitflip_threshold from master to slave partition.
  - [Patch 5/7]: Zero out max_bitflips outside of if-block in fsl_ifc_nand.
  - [Patch 6/7]: Simplify ecc strength sanity check.

This patchset addresses the problem of insufficient information being returned
by mtd_read() with regard to bit error corrections.  Currently -EUCLEAN is
returned if one or more bit errors were corrected during the course of the read
operation.  Higher layers like UBI use this return code as an indication that
the erase block may be degrading and should be considered as a candidate for
being marked as a bad block.  The problem is that high bit error rates are
common on more recent NAND flash devices with strong ecc algorithms.  Frequent
(but entirely normal) bit error corrections on these devices result in blocks
being incorrectly marked as bad.  On some devices, ubi/ubifs can not be reliably
used because of this.

This problem was discussed a while back [1][2][3], and a consensus of sorts was
reached for a solution, which these patches implement.  The recent addition of
the mtd api entry functions now make this solution practical (thanks Artem!).  A
quick description of each patch will provide a synopsis of the set:

(1) Fix how mtd->ecc_strength is calculated from nand->ecc.strength to reflect
    the fact that granularity is now at ecc step level (the two values are now
    the same).
(2) Fix a couple incorrect nand->ecc.strength values.
(3) Expose mtd->ecc_strength through sysfs (read-only).
(4) mtd->bitflip_threshold is added and exposed as read/write through sysfs.
    The drivers can initialize it, otherwise mtd initializes it to the default
    value of ecc_strength.  Users can change it through sysfs.
(5) The nand driver method _read_page() returns max_bitflips to the nand
    infrastructure code.  This is the maximum number of bitflip corrections that
    were made on any one region comprising an ecc step.
(6) Sanity checks added to nand_scan_tail() to ensure that drivers needing to
    set ecc strength do so.  (This includes all drivers using hardware ecc.)
(7) Absent a hard error, all drivers' mtd->_read() methods now return
    max_bitflips to mtd, and mtd_read() makes the decision of whether to return
    -EUCLEAN or 0. If max_bitflips >= bitflip_threshold, -EUCLEAN is returned.
    If bitflip_threshold == 0 (no ecc), make no comparison and return 0.

Of course reviews greatly appreciated.  Thanks!

P.S. It looks like some of these patches may conflict with others that may be
queued up, so let me know if they need to be reworked.

[1] http://lists.infradead.org/pipermail/linux-mtd/2011-December/038755.html
[2] http://lists.infradead.org/pipermail/linux-mtd/2011-December/038688.html
[3] http://lists.infradead.org/pipermail/linux-mtd/2011-December/038828.html

Mike Dunn (7):
  mtd; ecc_strength is at ecc step granularity
  mtd: nand: fix incorrect ecc strength values
  mtd: expose ecc_strength through sysfs
  mtd: bitflip threshold added to mtd_info and sysfs
  mtd: nand: read_page() returns max_bitflips
  mtd: nand: sanity checks of ecc strength in nand_scan_tail()
  mtd: driver _read() returns max_bitflips; mtd_read() returns -EUCLEAN

 Documentation/ABI/testing/sysfs-class-mtd |   48 +++++++++++++++
 drivers/mtd/devices/docg3.c               |    4 +-
 drivers/mtd/mtdcore.c                     |   57 +++++++++++++++++-
 drivers/mtd/mtdpart.c                     |   12 ++--
 drivers/mtd/nand/alauda.c                 |    4 +-
 drivers/mtd/nand/atmel_nand.c             |    9 ++-
 drivers/mtd/nand/bcm_umi_bch.c            |    4 +-
 drivers/mtd/nand/bcm_umi_nand.c           |    7 +--
 drivers/mtd/nand/cafe_nand.c              |    4 +-
 drivers/mtd/nand/denali.c                 |   10 ++-
 drivers/mtd/nand/docg4.c                  |    5 +-
 drivers/mtd/nand/fsl_elbc_nand.c          |   21 ++++--
 drivers/mtd/nand/fsl_ifc_nand.c           |   10 +++-
 drivers/mtd/nand/fsmc_nand.c              |    9 ++-
 drivers/mtd/nand/jz4740_nand.c            |    6 +--
 drivers/mtd/nand/nand_base.c              |   95 ++++++++++++++++++++--------
 drivers/mtd/nand/sh_flctl.c               |    2 +-
 drivers/mtd/onenand/onenand_base.c        |    6 +-
 include/linux/mtd/mtd.h                   |   11 +++-
 include/linux/mtd/nand.h                  |    3 +
 20 files changed, 254 insertions(+), 73 deletions(-)

-- 
1.7.3.4

[PATCH v2 1/7] mtd: ecc_strength is at ecc step granularity

[Mike Dunn]

ecc_strength element of mtd_info will be the strength of one ecc step, not of
the entire writesize, as was previously planned.  This is the appropriate way
because, as was pointed out [1], bit errors in excess of the strength of one
step can cause a hard error if they all occur within the same ecc region.

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040313.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

This v2 is actually unchanged from the original set.

 drivers/mtd/nand/nand_base.c |    2 +-
 include/linux/mtd/mtd.h      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index eb88d8b..c2d2e93 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3484,7 +3484,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 
 	/* propagate ecc info to mtd_info */
 	mtd->ecclayout = chip->ecc.layout;
-	mtd->ecc_strength = chip->ecc.strength * chip->ecc.steps;
+	mtd->ecc_strength = chip->ecc.strength;
 
 	/* Check, if we should skip the bad block table scan */
 	if (chip->options & NAND_SKIP_BBTSCAN)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index cf5ea8c..cd0119d 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -164,7 +164,7 @@ struct mtd_info {
 	/* ECC layout structure pointer - read only! */
 	struct nand_ecclayout *ecclayout;
 
-	/* max number of correctible bit errors per writesize */
+	/* max number of correctible bit errors per ecc step */
 	unsigned int ecc_strength;
 
 	/* Data for variable erase regions. If numeraseregions is zero,
-- 
1.7.3.4

[PATCH 2/7] mtd: nand: fix incorrect ecc strength values

[Mike Dunn]

This fixes a couple of ecc strength values for which I earlier made conservative
guesses, but whose correct values were later determined [1] (thanks Ivan).  Also
sets strength for fsl_ifc_nand, which was merged to mainline after the original
patch that set the strength for all drivers.

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040325.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

This v2 is actually unchanged from the original set.

 drivers/mtd/nand/bcm_umi_nand.c |    7 +------
 drivers/mtd/nand/fsl_ifc_nand.c |    1 +
 drivers/mtd/nand/jz4740_nand.c  |    6 +-----
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/bcm_umi_nand.c b/drivers/mtd/nand/bcm_umi_nand.c
index 6908cdd..7134adf 100644
--- a/drivers/mtd/nand/bcm_umi_nand.c
+++ b/drivers/mtd/nand/bcm_umi_nand.c
@@ -476,12 +476,7 @@ static int __devinit bcm_umi_nand_probe(struct platform_device *pdev)
 		this->badblock_pattern = &largepage_bbt;
 	}
 
-	/*
-	 * FIXME: ecc strength value of 6 bits per 512 bytes of data is a
-	 * conservative guess, given 13 ecc bytes and using bch alg.
-	 * (Assume Galois field order m=15 to allow a margin of error.)
-	 */
-	this->ecc.strength = 6;
+	this->ecc.strength = 8;
 
 #endif
 
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 5387cec..872cc96 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -821,6 +821,7 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv)
 	/* Hardware generates ECC per 512 Bytes */
 	chip->ecc.size = 512;
 	chip->ecc.bytes = 8;
+	chip->ecc.strength = 4;
 
 	switch (csor & CSOR_NAND_PGS_MASK) {
 	case CSOR_NAND_PGS_512:
diff --git a/drivers/mtd/nand/jz4740_nand.c b/drivers/mtd/nand/jz4740_nand.c
index e4147e8..a6fa884 100644
--- a/drivers/mtd/nand/jz4740_nand.c
+++ b/drivers/mtd/nand/jz4740_nand.c
@@ -332,11 +332,7 @@ static int __devinit jz_nand_probe(struct platform_device *pdev)
 	chip->ecc.mode		= NAND_ECC_HW_OOB_FIRST;
 	chip->ecc.size		= 512;
 	chip->ecc.bytes		= 9;
-	chip->ecc.strength	= 2;
-	/*
-	 * FIXME: ecc_strength value of 2 bits per 512 bytes of data is a
-	 * conservative guess, given 9 ecc bytes and reed-solomon alg.
-	 */
+	chip->ecc.strength	= 4;
 
 	if (pdata)
 		chip->ecc.layout = pdata->ecc_layout;
-- 
1.7.3.4

[PATCH v2 3/7] mtd: expose ecc_strength through sysfs

[Mike Dunn]

ecc_strength element of struct mtd_info is exposed as a read-only variable in
sysfs.  

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

This v2 is actually unchanged from the original set.

 Documentation/ABI/testing/sysfs-class-mtd |   12 ++++++++++++
 drivers/mtd/mtdcore.c                     |   10 ++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
index 4d55a18..43d1818 100644
--- a/Documentation/ABI/testing/sysfs-class-mtd
+++ b/Documentation/ABI/testing/sysfs-class-mtd
@@ -123,3 +123,15 @@ Description:
 		half page, or a quarter page).
 
 		In the case of ECC NOR, it is the ECC block size.
+
+What:		/sys/class/mtd/mtdX/ecc_strength
+Date:		April 2012
+KernelVersion:	3.4
+Contact:	linux-mtd@lists.infradead.org
+Description:
+		Maximum number of bit errors that the device is capable of
+		correcting within each region covering an ecc step.  This will
+		always be a non-negative integer.  Note that some devices will
+		have multiple ecc steps within each writesize region.
+
+		In the case of devices lacking any ECC capability, it is 0.
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index c837507..090e849 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -250,6 +250,15 @@ static ssize_t mtd_name_show(struct device *dev,
 }
 static DEVICE_ATTR(name, S_IRUGO, mtd_name_show, NULL);
 
+static ssize_t mtd_ecc_strength_show(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", mtd->ecc_strength);
+}
+static DEVICE_ATTR(ecc_strength, S_IRUGO, mtd_ecc_strength_show, NULL);
+
 static struct attribute *mtd_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_flags.attr,
@@ -260,6 +269,7 @@ static struct attribute *mtd_attrs[] = {
 	&dev_attr_oobsize.attr,
 	&dev_attr_numeraseregions.attr,
 	&dev_attr_name.attr,
+	&dev_attr_ecc_strength.attr,
 	NULL,
 };
 
-- 
1.7.3.4

[PATCH v2 4/7] mtd: bitflip_threshold added to mtd_info and sysfs

[Mike Dunn]

An element 'bitflip_threshold' is added to struct mtd_info, and also exposed as
a read/write variable in sysfs.  This will be used to determine whether or not
mtd_read() returns -EUCLEAN or 0 (absent a hard error).  If the driver leaves it
as zero, mtd will set it to a default value of ecc_strength.  

This v2 adds the line that propagates bitflip_threshold from the master to the
partitions - thanks Ivan [1].

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-April/040900.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 Documentation/ABI/testing/sysfs-class-mtd |   36 +++++++++++++++++++++++++++++
 drivers/mtd/mtdcore.c                     |   33 ++++++++++++++++++++++++++
 drivers/mtd/mtdpart.c                     |    2 +
 include/linux/mtd/mtd.h                   |    9 +++++++
 4 files changed, 80 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-mtd b/Documentation/ABI/testing/sysfs-class-mtd
index 43d1818..7883508 100644
--- a/Documentation/ABI/testing/sysfs-class-mtd
+++ b/Documentation/ABI/testing/sysfs-class-mtd
@@ -135,3 +135,39 @@ Description:
 		have multiple ecc steps within each writesize region.
 
 		In the case of devices lacking any ECC capability, it is 0.
+
+What:		/sys/class/mtd/mtdX/bitflip_threshold
+Date:		April 2012
+KernelVersion:	3.4
+Contact:	linux-mtd@lists.infradead.org
+Description:
+		This allows the user to examine and adjust the criteria by which
+		mtd returns -EUCLEAN from mtd_read().  If the maximum number of
+		bit errors that were corrected on any single region comprising
+		an ecc step (as reported by the driver) equals or exceeds this
+		value, -EUCLEAN is returned.  Otherwise, absent an error, 0 is
+		returned.  Higher layers (e.g., UBI) use this return code as an
+		indication that an erase block may be degrading and should be
+		scrutinized as a candidate for being marked as bad.
+
+		The initial value may be specified by the flash device driver.
+		If not, then the default value is ecc_strength.
+
+		The introduction of this feature brings a subtle change to the
+		meaning of the -EUCLEAN return code.  Previously, it was
+		interpreted to mean simply "one or more bit errors were
+		corrected".  Its new interpretation can be phrased as "a
+		dangerously high number of bit errors were corrected on one or
+		more regions comprising an ecc step".  The precise definition of
+		"dangerously high" can be adjusted by the user with
+		bitflip_threshold.  Users are discouraged from doing this,
+		however, unless they know what they are doing and have intimate
+		knowledge of the properties of their device.  Broadly speaking,
+		bitflip_threshold should be low enough to detect genuine erase
+		block degradation, but high enough to avoid the consequences of
+		a persistent return value of -EUCLEAN on devices where sticky
+		bitflips occur.  Note that if bitflip_threshold exceeds
+		ecc_strength, -EUCLEAN is never returned by the read functions.
+
+		This is generally applicable only to NAND flash devices with ECC
+		capability.  It is ignored on devices lacking ECC capability.
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 090e849..6a7cba1 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -259,6 +259,34 @@ static ssize_t mtd_ecc_strength_show(struct device *dev,
 }
 static DEVICE_ATTR(ecc_strength, S_IRUGO, mtd_ecc_strength_show, NULL);
 
+static ssize_t mtd_bitflip_threshold_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", mtd->bitflip_threshold);
+}
+
+static ssize_t mtd_bitflip_threshold_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count)
+{
+	struct mtd_info *mtd = dev_get_drvdata(dev);
+	unsigned int bitflip_threshold;
+	int retval;
+
+	retval = kstrtouint(buf, 0, &bitflip_threshold);
+	if (retval)
+		return retval;
+
+	mtd->bitflip_threshold = bitflip_threshold;
+	return count;
+}
+static DEVICE_ATTR(bitflip_threshold, S_IRUGO | S_IWUSR,
+		   mtd_bitflip_threshold_show,
+		   mtd_bitflip_threshold_store);
+
 static struct attribute *mtd_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_flags.attr,
@@ -270,6 +298,7 @@ static struct attribute *mtd_attrs[] = {
 	&dev_attr_numeraseregions.attr,
 	&dev_attr_name.attr,
 	&dev_attr_ecc_strength.attr,
+	&dev_attr_bitflip_threshold.attr,
 	NULL,
 };
 
@@ -332,6 +361,10 @@ int add_mtd_device(struct mtd_info *mtd)
 	mtd->index = i;
 	mtd->usecount = 0;
 
+	/* default value if not set by driver */
+	if (mtd->bitflip_threshold == 0)
+		mtd->bitflip_threshold = mtd->ecc_strength;
+
 	if (is_power_of_2(mtd->erasesize))
 		mtd->erasesize_shift = ffs(mtd->erasesize) - 1;
 	else
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 9651c06..ec75d44 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -517,6 +517,8 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
 
 	slave->mtd.ecclayout = master->ecclayout;
 	slave->mtd.ecc_strength = master->ecc_strength;
+	slave->mtd.bitflip_threshold = master->bitflip_threshold;
+
 	if (master->_block_isbad) {
 		uint64_t offs = 0;
 
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index cd0119d..63dadc0 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -157,6 +157,15 @@ struct mtd_info {
 	unsigned int erasesize_mask;
 	unsigned int writesize_mask;
 
+	/*
+	 * read ops return -EUCLEAN if max number of bitflips corrected on any
+	 * one region comprising an ecc step equals or exceeds this value.
+	 * Settable by driver, else defaults to ecc_strength.  User can override
+	 * in sysfs.  N.B. The meaning of the -EUCLEAN return code has changed;
+	 * see Documentation/ABI/testing/sysfs-class-mtd for more detail.
+	 */
+	unsigned int bitflip_threshold;
+
 	// Kernel-only stuff starts here.
 	const char *name;
 	int index;
-- 
1.7.3.4

[PATCH v2 5/7] mtd: nand: read_page() returns max_bitflips

[Mike Dunn]

The ecc.read_page() method for nand drivers is changed to return the maximum
number of bitflips that were corrected on any one region covering an ecc step,
This patch doesn't change what the nand code returns to mtd.

This v2 includes the change to the fsl_ifc_nand driver requested by Scott [1].

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-April/040883.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

Most of the changes to the drivers are straightforward, but a few really need
review.  The changes to the freescale controllers (fsl_elbc_nand and
fsl_ifc_nand) are a little more involved.  I am least confident about the denali
driver... I confess I am totally confused by the "multi connected nand support"
patch (commit 08b9ab9996c7e582f86da319f43d2dcb8ff55993).

 drivers/mtd/nand/atmel_nand.c    |    9 +++++--
 drivers/mtd/nand/bcm_umi_bch.c   |    4 ++-
 drivers/mtd/nand/cafe_nand.c     |    4 ++-
 drivers/mtd/nand/denali.c        |   10 +++++--
 drivers/mtd/nand/docg4.c         |    5 ++-
 drivers/mtd/nand/fsl_elbc_nand.c |   21 +++++++++++-----
 drivers/mtd/nand/fsl_ifc_nand.c  |    9 ++++++-
 drivers/mtd/nand/fsmc_nand.c     |    9 +++++--
 drivers/mtd/nand/nand_base.c     |   47 +++++++++++++++++++++++++-------------
 drivers/mtd/nand/sh_flctl.c      |    2 +-
 10 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 2165576..9a7876e 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -335,6 +335,7 @@ static int atmel_nand_read_page(struct mtd_info *mtd,
 	uint8_t *oob = chip->oob_poi;
 	uint8_t *ecc_pos;
 	int stat;
+	unsigned int max_bitflips = 0;
 
 	/*
 	 * Errata: ALE is incorrectly wired up to the ECC controller
@@ -371,10 +372,12 @@ static int atmel_nand_read_page(struct mtd_info *mtd,
 	/* check if there's an error */
 	stat = chip->ecc.correct(mtd, p, oob, NULL);
 
-	if (stat < 0)
+	if (stat < 0) {
 		mtd->ecc_stats.failed++;
-	else
+	} else {
 		mtd->ecc_stats.corrected += stat;
+		max_bitflips = max_t(unsigned int, max_bitflips, stat);
+	}
 
 	/* get back to oob start (end of page) */
 	chip->cmdfunc(mtd, NAND_CMD_RNDOUT, mtd->writesize, -1);
@@ -382,7 +385,7 @@ static int atmel_nand_read_page(struct mtd_info *mtd,
 	/* read the oob */
 	chip->read_buf(mtd, oob, mtd->oobsize);
 
-	return 0;
+	return max_bitflips;
 }
 
 /*
diff --git a/drivers/mtd/nand/bcm_umi_bch.c b/drivers/mtd/nand/bcm_umi_bch.c
index a930666..f8472b4 100644
--- a/drivers/mtd/nand/bcm_umi_bch.c
+++ b/drivers/mtd/nand/bcm_umi_bch.c
@@ -116,6 +116,7 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 	uint8_t eccCalc[NAND_ECC_NUM_BYTES];
 	int sectorOobSize = mtd->oobsize / eccsteps;
 	int stat;
+	unsigned int max_bitflips = 0;
 
 	for (sectorIdx = 0; sectorIdx < eccsteps;
 			sectorIdx++, datap += eccsize) {
@@ -177,9 +178,10 @@ static int bcm_umi_bch_read_page_hwecc(struct mtd_info *mtd,
 			}
 #endif
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
 		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 /****************************************************************************
diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index 2973d97..886ff3a 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -383,6 +383,7 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			       uint8_t *buf, int page)
 {
 	struct cafe_priv *cafe = mtd->priv;
+	unsigned int max_bitflips = 0;
 
 	cafe_dev_dbg(&cafe->pdev->dev, "ECC result %08x SYN1,2 %08x\n",
 		     cafe_readl(cafe, NAND_ECC_RESULT),
@@ -449,10 +450,11 @@ static int cafe_nand_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 		} else {
 			dev_dbg(&cafe->pdev->dev, "Corrected %d symbol errors\n", n);
 			mtd->ecc_stats.corrected += n;
+			max_bitflips = max_t(unsigned int, max_bitflips, n);
 		}
 	}
 
-	return 0;
+	return max_bitflips;
 }
 
 static struct nand_ecclayout cafe_oobinfo_2048 = {
diff --git a/drivers/mtd/nand/denali.c b/drivers/mtd/nand/denali.c
index a1048c7..1b34647 100644
--- a/drivers/mtd/nand/denali.c
+++ b/drivers/mtd/nand/denali.c
@@ -924,9 +924,10 @@ bool is_erased(uint8_t *buf, int len)
 #define ECC_LAST_ERR(x)		((x) & ERR_CORRECTION_INFO__LAST_ERR_INFO)
 
 static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
-					uint32_t irq_status)
+		       uint32_t irq_status, unsigned int *max_bitflips)
 {
 	bool check_erased_page = false;
+	unsigned int bitflips = 0;
 
 	if (irq_status & INTR_STATUS__ECC_ERR) {
 		/* read the ECC errors. we'll ignore them for now */
@@ -965,6 +966,7 @@ static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
 					/* correct the ECC error */
 					buf[offset] ^= err_correction_value;
 					denali->mtd.ecc_stats.corrected++;
+					bitflips++;
 				}
 			} else {
 				/* if the error is not correctable, need to
@@ -984,6 +986,7 @@ static bool handle_ecc(struct denali_nand_info *denali, uint8_t *buf,
 		clear_interrupts(denali);
 		denali_set_intr_modes(denali, true);
 	}
+	*max_bitflips = bitflips;
 	return check_erased_page;
 }
 
@@ -1121,6 +1124,7 @@ static int denali_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
 static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 			    uint8_t *buf, int page)
 {
+	unsigned int max_bitflips;
 	struct denali_nand_info *denali = mtd_to_denali(mtd);
 
 	dma_addr_t addr = denali->buf.dma_buf;
@@ -1153,7 +1157,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	memcpy(buf, denali->buf.buf, mtd->writesize);
 
-	check_erased_page = handle_ecc(denali, buf, irq_status);
+	check_erased_page = handle_ecc(denali, buf, irq_status, &max_bitflips);
 	denali_enable_dma(denali, false);
 
 	if (check_erased_page) {
@@ -1167,7 +1171,7 @@ static int denali_read_page(struct mtd_info *mtd, struct nand_chip *chip,
 				denali->mtd.ecc_stats.failed++;
 		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 static int denali_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
diff --git a/drivers/mtd/nand/docg4.c b/drivers/mtd/nand/docg4.c
index b082026..8e7da01 100644
--- a/drivers/mtd/nand/docg4.c
+++ b/drivers/mtd/nand/docg4.c
@@ -720,6 +720,7 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 	struct docg4_priv *doc = nand->priv;
 	void __iomem *docptr = doc->virtadr;
 	uint16_t status, edc_err, *buf16;
+	int bits_corrected = 0;
 
 	dev_dbg(doc->dev, "%s: page %08x\n", __func__, page);
 
@@ -772,7 +773,7 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 
 		/* If bitflips are reported, attempt to correct with ecc */
 		if (edc_err & DOC_ECCCONF1_BCH_SYNDROM_ERR) {
-			int bits_corrected = correct_data(mtd, buf, page);
+			bits_corrected = correct_data(mtd, buf, page);
 			if (bits_corrected == -EBADMSG)
 				mtd->ecc_stats.failed++;
 			else
@@ -781,7 +782,7 @@ static int read_page(struct mtd_info *mtd, struct nand_chip *nand,
 	}
 
 	writew(0, docptr + DOC_DATAEND);
-	return 0;
+	return bits_corrected;
 }
 
 
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 80b5264..8638b5e 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -75,6 +75,7 @@ struct fsl_elbc_fcm_ctrl {
 	unsigned int use_mdr;    /* Non zero if the MDR is to be set      */
 	unsigned int oob;        /* Non zero if operating on OOB data     */
 	unsigned int counter;	 /* counter for the initializations	  */
+	unsigned int max_bitflips;  /* Saved during READ0 cmd		  */
 };
 
 /* These map to the positions used by the FCM hardware ECC generator */
@@ -253,6 +254,8 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 	if (chip->ecc.mode != NAND_ECC_HW)
 		return 0;
 
+	elbc_fcm_ctrl->max_bitflips = 0;
+
 	if (elbc_fcm_ctrl->read_bytes == mtd->writesize + mtd->oobsize) {
 		uint32_t lteccr = in_be32(&lbc->lteccr);
 		/*
@@ -262,11 +265,16 @@ static int fsl_elbc_run_command(struct mtd_info *mtd)
 		 * bits 28-31 are uncorrectable errors, marked elsewhere.
 		 * for small page nand only 1 bit is used.
 		 * if the ELBC doesn't have the lteccr register it reads 0
+		 * FIXME: 4 bits can be corrected on NANDs with 2k pages, so
+		 * count the number of sub-pages with bitflips and update
+		 * ecc_stats.corrected accordingly.
 		 */
 		if (lteccr & 0x000F000F)
 			out_be32(&lbc->lteccr, 0x000F000F); /* clear lteccr */
-		if (lteccr & 0x000F0000)
+		if (lteccr & 0x000F0000) {
 			mtd->ecc_stats.corrected++;
+			elbc_fcm_ctrl->max_bitflips = 1;
+		}
 	}
 
 	return 0;
@@ -743,13 +751,17 @@ static int fsl_elbc_read_page(struct mtd_info *mtd,
 			      uint8_t *buf,
 			      int page)
 {
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = ctrl->nand;
+
 	fsl_elbc_read_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
 
 	if (fsl_elbc_wait(mtd, chip) & NAND_STATUS_FAIL)
 		mtd->ecc_stats.failed++;
 
-	return 0;
+	return elbc_fcm_ctrl->max_bitflips;
 }
 
 /* ECC will be calculated automatically, and errors will be detected in
@@ -814,11 +826,6 @@ static int fsl_elbc_chip_init(struct fsl_elbc_mtd *priv)
 		chip->ecc.size = 512;
 		chip->ecc.bytes = 3;
 		chip->ecc.strength = 1;
-		/*
-		 * FIXME: can hardware ecc correct 4 bitflips if page size is
-		 * 2k?  Then does hardware report number of corrections for this
-		 * case?  If so, ecc_stats reporting needs to be fixed as well.
-		 */
 	} else {
 		/* otherwise fall back to default software ECC */
 		chip->ecc.mode = NAND_ECC_SOFT;
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index 872cc96..0adde96 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -63,6 +63,7 @@ struct fsl_ifc_nand_ctrl {
 	unsigned int oob;	/* Non zero if operating on OOB data	*/
 	unsigned int eccread;	/* Non zero for a full-page ECC read	*/
 	unsigned int counter;	/* counter for the initializations	*/
+	unsigned int max_bitflips;  /* Saved during READ0 cmd		*/
 };
 
 static struct fsl_ifc_nand_ctrl *ifc_nand_ctrl;
@@ -262,6 +263,8 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
 	if (ctrl->nand_stat & IFC_NAND_EVTER_STAT_WPER)
 		dev_err(priv->dev, "NAND Flash Write Protect Error\n");
 
+	nctrl->max_bitflips = 0;
+
 	if (nctrl->eccread) {
 		int errors;
 		int bufnum = nctrl->page & priv->bufnum_mask;
@@ -290,6 +293,9 @@ static void fsl_ifc_run_command(struct mtd_info *mtd)
 			}
 
 			mtd->ecc_stats.corrected += errors;
+			nctrl->max_bitflips = max_t(unsigned int,
+						    nctrl->max_bitflips,
+						    errors);
 		}
 
 		nctrl->eccread = 0;
@@ -698,6 +704,7 @@ static int fsl_ifc_read_page(struct mtd_info *mtd,
 {
 	struct fsl_ifc_mtd *priv = chip->priv;
 	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
+	struct fsl_ifc_nand_ctrl *nctrl = ifc_nand_ctrl;
 
 	fsl_ifc_read_buf(mtd, buf, mtd->writesize);
 	fsl_ifc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -708,7 +715,7 @@ static int fsl_ifc_read_page(struct mtd_info *mtd,
 	if (ctrl->nand_stat != IFC_NAND_EVTER_STAT_OPC)
 		mtd->ecc_stats.failed++;
 
-	return 0;
+	return nctrl->max_bitflips;
 }
 
 /* ECC will be calculated automatically, and errors will be detected in
diff --git a/drivers/mtd/nand/fsmc_nand.c b/drivers/mtd/nand/fsmc_nand.c
index 1b8330e..1bf7f80 100644
--- a/drivers/mtd/nand/fsmc_nand.c
+++ b/drivers/mtd/nand/fsmc_nand.c
@@ -720,6 +720,7 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 	 */
 	uint16_t ecc_oob[7];
 	uint8_t *oob = (uint8_t *)&ecc_oob[0];
+	unsigned int max_bitflips = 0;
 
 	for (i = 0, s = 0; s < eccsteps; s++, i += eccbytes, p += eccsize) {
 		chip->cmdfunc(mtd, NAND_CMD_READ0, s * eccsize, page);
@@ -748,13 +749,15 @@ static int fsmc_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
 
 		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 	}
 
-	return 0;
+	return max_bitflips;
 }
 
 /*
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c2d2e93..3137abd 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1138,6 +1138,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	unsigned int max_bitflips = 0;
 
 	chip->ecc.read_page_raw(mtd, chip, buf, page);
 
@@ -1154,12 +1155,14 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 		int stat;
 
 		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 /**
@@ -1180,6 +1183,7 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 	int datafrag_len, eccfrag_len, aligned_len, aligned_pos;
 	int busw = (chip->options & NAND_BUSWIDTH_16) ? 2 : 1;
 	int index = 0;
+	unsigned int max_bitflips = 0;
 
 	/* Column address within the page aligned to ECC size (256bytes) */
 	start_step = data_offs / chip->ecc.size;
@@ -1244,12 +1248,14 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
 
 		stat = chip->ecc.correct(mtd, p,
 			&chip->buffers->ecccode[i], &chip->buffers->ecccalc[i]);
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 /**
@@ -1271,6 +1277,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
+	unsigned int max_bitflips = 0;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		chip->ecc.hwctl(mtd, NAND_ECC_READ);
@@ -1289,12 +1296,14 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 		int stat;
 
 		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 /**
@@ -1320,6 +1329,7 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 	uint8_t *ecc_calc = chip->buffers->ecccalc;
+	unsigned int max_bitflips = 0;
 
 	/* Read the OOB area first */
 	chip->cmdfunc(mtd, NAND_CMD_READOOB, 0, page);
@@ -1337,12 +1347,14 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
 
 		stat = chip->ecc.correct(mtd, p, &ecc_code[i], NULL);
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 	}
-	return 0;
+	return max_bitflips;
 }
 
 /**
@@ -1363,6 +1375,7 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 	int eccsteps = chip->ecc.steps;
 	uint8_t *p = buf;
 	uint8_t *oob = chip->oob_poi;
+	unsigned int max_bitflips = 0;
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
 		int stat;
@@ -1379,10 +1392,12 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 		chip->read_buf(mtd, oob, eccbytes);
 		stat = chip->ecc.correct(mtd, p, oob, NULL);
 
-		if (stat < 0)
+		if (stat < 0) {
 			mtd->ecc_stats.failed++;
-		else
+		} else {
 			mtd->ecc_stats.corrected += stat;
+			max_bitflips = max_t(unsigned int, max_bitflips, stat);
+		}
 
 		oob += eccbytes;
 
@@ -1397,7 +1412,7 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
 	if (i)
 		chip->read_buf(mtd, oob, i);
 
-	return 0;
+	return max_bitflips;
 }
 
 /**
@@ -1588,7 +1603,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	if (oob)
 		ops->oobretlen = ops->ooblen - oobreadlen;
 
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	if (mtd->ecc_stats.failed - stats.failed)
diff --git a/drivers/mtd/nand/sh_flctl.c b/drivers/mtd/nand/sh_flctl.c
index e9b2b26..4ea3e20 100644
--- a/drivers/mtd/nand/sh_flctl.c
+++ b/drivers/mtd/nand/sh_flctl.c
@@ -359,7 +359,7 @@ static int flctl_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
 		if (flctl->hwecc_cant_correct[i])
 			mtd->ecc_stats.failed++;
 		else
-			mtd->ecc_stats.corrected += 0;
+			mtd->ecc_stats.corrected += 0; /* FIXME */
 	}
 
 	return 0;
-- 
1.7.3.4

[PATCH v2 6/7] mtd: nand: add sanity check of ecc strength to nand_scan_tail()

[Mike Dunn]

This patch adds sanity checks that ensure that drivers for controllers with
hardware ECC set the 'strength' element in struct nand_ecc_ctrl.  Also stylistic
changes to the line that calculates strength for software ECC.

This v2 simplifies the check.  Thanks Brian! [1]

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-April/040890.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---
 drivers/mtd/nand/nand_base.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 3137abd..dac0afa 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3345,8 +3345,13 @@ int nand_scan_tail(struct mtd_info *mtd)
 		if (!chip->ecc.write_oob)
 			chip->ecc.write_oob = nand_write_oob_syndrome;
 
-		if (mtd->writesize >= chip->ecc.size)
+		if (mtd->writesize >= chip->ecc.size) {
+			if (!chip->ecc.strength) {
+				pr_warn("Driver must set ecc.strength when using hardware ECC\n");
+				BUG();
+			}
 			break;
+		}
 		pr_warn("%d byte HW ECC not possible on "
 			   "%d byte page size, fallback to SW ECC\n",
 			   chip->ecc.size, mtd->writesize);
@@ -3401,7 +3406,7 @@ int nand_scan_tail(struct mtd_info *mtd)
 			BUG();
 		}
 		chip->ecc.strength =
-			chip->ecc.bytes*8 / fls(8*chip->ecc.size);
+			chip->ecc.bytes * 8 / fls(8 * chip->ecc.size);
 		break;
 
 	case NAND_ECC_NONE:
-- 
1.7.3.4

[PATCH v2 7/7] mtd: driver _read() returns max_bitflips; mtd_read() returns -EUCLEAN

[Mike Dunn]

The drivers _read() method, absent an error, return a non-negative integer
indicating the maximum number of bit errors that were corrected in any one
region comprising an ecc step.  MTD returns -EUCLEAN if this is >=
bitflip_threshold, 0 otherwise.  If bitflip_threshold is zero, the comparison is
not made since these devices lack ECC and always return zero in the non-error
case (thanks Brian) [1].  Note that this is a subtle change to the driver
interface.

This and the preceeding patches in this set were tested with ubi on top of the
nandsim and docg4 devices, running the ubi test io_basic from mtd-utils.

[1] http://lists.infradead.org/pipermail/linux-mtd/2012-March/040468.html

Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
---

This v2 is actually unchanged from the original set.

 drivers/mtd/devices/docg3.c        |    4 +++-
 drivers/mtd/mtdcore.c              |   14 +++++++++++++-
 drivers/mtd/mtdpart.c              |   12 ++++++------
 drivers/mtd/nand/alauda.c          |    4 ++--
 drivers/mtd/nand/nand_base.c       |   18 ++++++++++++++----
 drivers/mtd/onenand/onenand_base.c |    6 ++++--
 include/linux/mtd/nand.h           |    3 +++
 7 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c
index 3414031..7644d59 100644
--- a/drivers/mtd/devices/docg3.c
+++ b/drivers/mtd/devices/docg3.c
@@ -862,6 +862,7 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 	u8 *buf = ops->datbuf;
 	size_t len, ooblen, nbdata, nboob;
 	u8 hwecc[DOC_ECC_BCH_SIZE], eccconf1;
+	int max_bitflips = 0;
 
 	if (buf)
 		len = ops->len;
@@ -948,7 +949,8 @@ static int doc_read_oob(struct mtd_info *mtd, loff_t from,
 			}
 			if (ret > 0) {
 				mtd->ecc_stats.corrected += ret;
-				ret = -EUCLEAN;
+				max_bitflips = max(max_bitflips, ret);
+				ret = max_bitflips;
 			}
 		}
 
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 6a7cba1..b5bb628 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -800,12 +800,24 @@ EXPORT_SYMBOL_GPL(mtd_get_unmapped_area);
 int mtd_read(struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen,
 	     u_char *buf)
 {
+	int ret_code;
 	*retlen = 0;
 	if (from < 0 || from > mtd->size || len > mtd->size - from)
 		return -EINVAL;
 	if (!len)
 		return 0;
-	return mtd->_read(mtd, from, len, retlen, buf);
+
+	/*
+	 * In the absence of an error, drivers return a non-negative integer
+	 * representing the maximum number of bitflips that were corrected on
+	 * any one ecc region (if applicable; zero otherwise).
+	 */
+	ret_code = mtd->_read(mtd, from, len, retlen, buf);
+	if (unlikely(ret_code < 0))
+		return ret_code;
+	if (mtd->bitflip_threshold == 0)
+		return 0;	/* device lacks ecc */
+	return ret_code >= mtd->bitflip_threshold ? -EUCLEAN : 0;
 }
 EXPORT_SYMBOL_GPL(mtd_read);
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 9651c06..d6321f6 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -67,12 +67,12 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
 	stats = part->master->ecc_stats;
 	res = part->master->_read(part->master, from + part->offset, len,
 				  retlen, buf);
-	if (unlikely(res)) {
-		if (mtd_is_bitflip(res))
-			mtd->ecc_stats.corrected += part->master->ecc_stats.corrected - stats.corrected;
-		if (mtd_is_eccerr(res))
-			mtd->ecc_stats.failed += part->master->ecc_stats.failed - stats.failed;
-	}
+	if (unlikely(mtd_is_eccerr(res)))
+		mtd->ecc_stats.failed +=
+			part->master->ecc_stats.failed - stats.failed;
+	else
+		mtd->ecc_stats.corrected +=
+			part->master->ecc_stats.corrected - stats.corrected;
 	return res;
 }
 
diff --git a/drivers/mtd/nand/alauda.c b/drivers/mtd/nand/alauda.c
index 4f20e1d..60a0dfd 100644
--- a/drivers/mtd/nand/alauda.c
+++ b/drivers/mtd/nand/alauda.c
@@ -414,7 +414,7 @@ static int alauda_bounce_read(struct mtd_info *mtd, loff_t from, size_t len,
 	}
 	err = 0;
 	if (corrected)
-		err = -EUCLEAN;
+		err = 1;	/* return max_bitflips per ecc step */
 	if (uncorrected)
 		err = -EBADMSG;
 out:
@@ -446,7 +446,7 @@ static int alauda_read(struct mtd_info *mtd, loff_t from, size_t len,
 	}
 	err = 0;
 	if (corrected)
-		err = -EUCLEAN;
+		err = 1;	/* return max_bitflips per ecc step */
 	if (uncorrected)
 		err = -EBADMSG;
 	return err;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 275c43f..9f1b202 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1486,6 +1486,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 		mtd->oobavail : mtd->oobsize;
 
 	uint8_t *bufpoi, *oob, *buf;
+	unsigned int max_bitflips = 0;
 
 	stats = mtd->ecc_stats;
 
@@ -1513,7 +1514,10 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 				sndcmd = 0;
 			}
 
-			/* Now read the page into the buffer */
+			/*
+			 * Now read the page into the buffer.  Absent an error,
+			 * the read methods return max bitflips per ecc step.
+			 */
 			if (unlikely(ops->mode == MTD_OPS_RAW))
 				ret = chip->ecc.read_page_raw(mtd, chip,
 							      bufpoi, page);
@@ -1530,15 +1534,19 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 				break;
 			}
 
+			max_bitflips = max_t(unsigned int, max_bitflips, ret);
+
 			/* Transfer not aligned data */
 			if (!aligned) {
 				if (!NAND_SUBPAGE_READ(chip) && !oob &&
 				    !(mtd->ecc_stats.failed - stats.failed) &&
-				    (ops->mode != MTD_OPS_RAW))
+				    (ops->mode != MTD_OPS_RAW)) {
 					chip->pagebuf = realpage;
-				else
+					chip->pagebuf_bitflips = ret;
+				} else {
 					/* Invalidate page cache */
 					chip->pagebuf = -1;
+				}
 				memcpy(buf, chip->buffers->databuf + col, bytes);
 			}
 
@@ -1571,6 +1579,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 		} else {
 			memcpy(buf, chip->buffers->databuf + col, bytes);
 			buf += bytes;
+			max_bitflips = max_t(unsigned int, max_bitflips,
+					     chip->pagebuf_bitflips);
 		}
 
 		readlen -= bytes;
@@ -1609,7 +1619,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	if (mtd->ecc_stats.failed - stats.failed)
 		return -EBADMSG;
 
-	return  mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
+	return max_bitflips;
 }
 
 /**
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index b3ce12e..7153e0d 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1201,7 +1201,8 @@ static int onenand_mlc_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 	if (mtd->ecc_stats.failed - stats.failed)
 		return -EBADMSG;
 
-	return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
+	/* return max bitflips per ecc step; ONENANDs correct 1 bit only */
+	return mtd->ecc_stats.corrected != stats.corrected ? 1 : 0;
 }
 
 /**
@@ -1333,7 +1334,8 @@ static int onenand_read_ops_nolock(struct mtd_info *mtd, loff_t from,
 	if (mtd->ecc_stats.failed - stats.failed)
 		return -EBADMSG;
 
-	return mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
+	/* return max bitflips per ecc step; ONENANDs correct 1 bit only */
+	return mtd->ecc_stats.corrected != stats.corrected ? 1 : 0;
 }
 
 /**
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 1482340..2829e8b 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -459,6 +459,8 @@ struct nand_buffers {
  * @pagemask:		[INTERN] page number mask = number of (pages / chip) - 1
  * @pagebuf:		[INTERN] holds the pagenumber which is currently in
  *			data_buf.
+ * @pagebuf_bitflips:	[INTERN] holds the bitflip count for the page which is
+ *			currently in data_buf.
  * @subpagesize:	[INTERN] holds the subpagesize
  * @onfi_version:	[INTERN] holds the chip ONFI version (BCD encoded),
  *			non 0 if ONFI supported.
@@ -519,6 +521,7 @@ struct nand_chip {
 	uint64_t chipsize;
 	int pagemask;
 	int pagebuf;
+	unsigned int pagebuf_bitflips;
 	int subpagesize;
 	uint8_t cellinfo;
 	int badblockpos;
-- 
1.7.3.4

Re: [PATCH v2 6/7] mtd: nand: add sanity check of ecc strength to nand_scan_tail()

[Brian Norris]

On Wed, Apr 25, 2012 at 12:06 PM, Mike Dunn <mikedunn@newsguy.com> wrote:
> This patch adds sanity checks that ensure that drivers for controllers with
> hardware ECC set the 'strength' element in struct nand_ecc_ctrl.  Also stylistic
> changes to the line that calculates strength for software ECC.
>
> This v2 simplifies the check.  Thanks Brian! [1]

Thank you for going through the review process! This series provides a
good general fix for something that's been kind of masked in hardware
like mine (which has its own threshold for suppressing bitflip
messages).

> [1] http://lists.infradead.org/pipermail/linux-mtd/2012-April/040890.html
>
> Signed-off-by: Mike Dunn <mikedunn@newsguy.com>

If we're looking for Ack's to rebase with:

Acked-by: Brian Norris <computersforpeace@gmail.com>

Re: [PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Wed, 2012-04-25 at 12:06 -0700, Mike Dunn wrote:
> This v2 of this set includes the following changes:
>   - [Patch 4/7]: Propagate bitflip_threshold from master to slave partition.
>   - [Patch 5/7]: Zero out max_bitflips outside of if-block in fsl_ifc_nand.
>   - [Patch 6/7]: Simplify ecc strength sanity check.

Would you please do one more thing - create a piece of documentation
about this feature at the mtd web site.

http://git.infradead.org/mtd-www.git

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[Mike Dunn]

On 05/05/2012 02:43 AM, Artem Bityutskiy wrote:
> On Wed, 2012-04-25 at 12:06 -0700, Mike Dunn wrote:
>> This v2 of this set includes the following changes:
>>   - [Patch 4/7]: Propagate bitflip_threshold from master to slave partition.
>>   - [Patch 5/7]: Zero out max_bitflips outside of if-block in fsl_ifc_nand.
>>   - [Patch 6/7]: Simplify ecc strength sanity check.
> 
> Would you please do one more thing - create a piece of documentation
> about this feature at the mtd web site.


OK.  Please give me a few days to get to this.

Thanks,
Mike

Re: [PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[Mike Dunn]

On 05/05/2012 02:43 AM, Artem Bityutskiy wrote:
> On Wed, 2012-04-25 at 12:06 -0700, Mike Dunn wrote:
>> This v2 of this set includes the following changes:
>>   - [Patch 4/7]: Propagate bitflip_threshold from master to slave partition.
>>   - [Patch 5/7]: Zero out max_bitflips outside of if-block in fsl_ifc_nand.
>>   - [Patch 6/7]: Simplify ecc strength sanity check.
> 
> Would you please do one more thing - create a piece of documentation
> about this feature at the mtd web site.


Do you have any suggestions regarding where on the site this should be added?

Thanks,
Mike

Re: [PATCH v2 0/7] mtd: Change meaning of -EUCLEAN return code on reads

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Wed, 2012-05-09 at 09:49 -0700, Mike Dunn wrote:
> On 05/05/2012 02:43 AM, Artem Bityutskiy wrote:
> > On Wed, 2012-04-25 at 12:06 -0700, Mike Dunn wrote:
> >> This v2 of this set includes the following changes:
> >>   - [Patch 4/7]: Propagate bitflip_threshold from master to slave partition.
> >>   - [Patch 5/7]: Zero out max_bitflips outside of if-block in fsl_ifc_nand.
> >>   - [Patch 6/7]: Simplify ecc strength sanity check.
> > 
> > Would you please do one more thing - create a piece of documentation
> > about this feature at the mtd web site.
> 
> 
> Do you have any suggestions regarding where on the site this should be added?

Not sure, the site is poorly organized, may be here:

http://www.linux-mtd.infradead.org/doc/general.html

?

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]