[U-Boot-Users] [PATCH 1/2] NAND: Add support for transparent hardware ECC.

[U-Boot-Users] [PATCH 1/2] NAND: Add support for transparent hardware ECC.

[Scott Wood]

Some NAND controllers (such as on the MPC831x chips) have hardware ECC,
but can only do it during a transfer (i.e. we can't implement
calculate_ecc()).  When NAND_ECC_TRANSPARENT is used, then ECC errors on
reads are reported through correct_data() (with no arguments other than
the mtd device), and on writes through waitfunc()'s return value.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 drivers/nand/nand_base.c |   84 ++++++++++++++++++++++++++++++++--------------
 include/linux/mtd/nand.h |   10 +++++
 2 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/drivers/nand/nand_base.c b/drivers/nand/nand_base.c
index 8495829..52c612c 100644
--- a/drivers/nand/nand_base.c
+++ b/drivers/nand/nand_base.c
@@ -911,6 +911,11 @@ static int nand_write_page (struct mtd_info *mtd, struct nand_chip *this, int pa
 		this->write_buf(mtd, this->data_poi, mtd->oobblock);
 		break;
 
+	case NAND_ECC_TRANSPARENT:
+		this->enable_hwecc(mtd, NAND_ECC_WRITE);
+		this->write_buf(mtd, this->data_poi, mtd->oobblock);
+		break;
+
 	/* Software ecc 3/256, write all */
 	case NAND_ECC_SOFT:
 		for (; eccsteps; eccsteps--) {
@@ -992,10 +997,14 @@ static int nand_verify_pages (struct mtd_info *mtd, struct nand_chip *this, int
 	int	eccsteps = this->eccsteps;
 	int	hweccbytes;
 	u_char 	oobdata[64];
+	int eccmode = oobsel->useecc ? this->eccmode : NAND_ECC_NONE;
 
 	hweccbytes = (this->options & NAND_HWECC_SYNDROME) ? (oobsel->eccbytes / eccsteps) : 0;
 
 	/* Send command to read back the first page */
+	if (eccmode == NAND_ECC_TRANSPARENT)
+		this->enable_hwecc(mtd, NAND_ECC_READ);
+
 	this->cmdfunc (mtd, NAND_CMD_READ0, 0, page);
 
 	for(;;) {
@@ -1019,7 +1028,30 @@ static int nand_verify_pages (struct mtd_info *mtd, struct nand_chip *this, int
 		/* check, if we must compare all data or if we just have to
 		 * compare the ecc bytes
 		 */
-		if (oobmode) {
+		if (eccmode == NAND_ECC_TRANSPARENT) {
+			if (oobmode) {
+				/* Compare everything *but* the ECC */
+				this->read_buf(mtd, oobdata, mtd->oobsize -
+				               hweccbytes * eccsteps);
+
+				for (i = 0, j = 0; i < mtd->oobsize; i++) {
+					if (j < oobsel->eccbytes &&
+					    i == oobsel->eccpos[j]) {
+						j++;
+						continue;
+					}
+
+					if (oobdata[i] != oob_buf[oobofs + i]) {
+						DEBUG (MTD_DEBUG_LEVEL0,
+					          "%s: Failed ECC write "
+						"verify, page 0x%08x, "
+						"%6i bytes were succesful\n",
+						__FUNCTION__, page, i);
+						goto out;
+					}
+				}
+			}
+		} else if (oobmode) {
 			if (this->verify_buf(mtd, &oob_buf[oobofs], mtd->oobsize - hweccbytes * eccsteps)) {
 				DEBUG (MTD_DEBUG_LEVEL0, "%s: " "Failed write verify, page 0x%08x ", __FUNCTION__, page);
 				goto out;
@@ -1062,10 +1094,13 @@ static int nand_verify_pages (struct mtd_info *mtd, struct nand_chip *this, int
 		if (!numpages)
 			return 0;
 
-
 		/* Check, if the chip supports auto page increment */
-		if (!NAND_CANAUTOINCR(this))
+		if (!NAND_CANAUTOINCR(this)) {
+			if (eccmode == NAND_ECC_TRANSPARENT)
+				this->enable_hwecc(mtd, NAND_ECC_READ);
+
 			this->cmdfunc (mtd, NAND_CMD_READ0, 0x00, page);
+		}
 	}
 	/*
 	 * Terminate the read command. We come here in case of an error
@@ -1161,7 +1196,8 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
 	ecc = this->eccsize;
 	eccbytes = this->eccbytes;
 
-	if ((eccmode == NAND_ECC_NONE) || (this->options & NAND_HWECC_SYNDROME))
+	if (eccmode == NAND_ECC_NONE || eccmode == NAND_ECC_TRANSPARENT ||
+	    (this->options & NAND_HWECC_SYNDROME))
 		compareecc = 0;
 
 	oobreadlen = mtd->oobsize;
@@ -1195,6 +1231,9 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
 
 		/* Check, if we must send the read command */
 		if (sndcmd) {
+			if (eccmode == NAND_ECC_TRANSPARENT)
+				this->enable_hwecc(mtd, NAND_ECC_READ);
+
 			this->cmdfunc (mtd, NAND_CMD_READ0, 0x00, page);
 			sndcmd = 0;
 		}
@@ -1222,6 +1261,18 @@ static int nand_read_ecc (struct mtd_info *mtd, loff_t from, size_t len,
 			break;
 		}
 
+		case NAND_ECC_TRANSPARENT:
+			this->read_buf(mtd, data_poi, end);
+
+			if (this->correct_data(mtd, NULL, NULL, NULL) == -1) {
+				DEBUG(MTD_DEBUG_LEVEL0, "nand_read_ecc: "
+				      "Failed ECC read, page 0x%08x on chip %d\n",
+				      page, chipnr);
+				ecc_failed++;
+			}
+
+			break;
+
 		case NAND_ECC_SOFT:	/* Software ECC 3/256: Read in a page + oob data */
 			this->read_buf(mtd, data_poi, end);
 			for (i = 0, datidx = 0; eccsteps; eccsteps--, i+=3, datidx += ecc)
@@ -2530,7 +2581,9 @@ int nand_scan (struct mtd_info *mtd, int maxchips)
 
 	case NAND_ECC_NONE:
 		printk (KERN_WARNING "NAND_ECC_NONE selected by board driver. This is not recommended !!\n");
-		this->eccmode = NAND_ECC_NONE;
+		/* fall-through */
+	case NAND_ECC_TRANSPARENT:
+		this->eccsize = mtd->oobblock;
 		break;
 
 	case NAND_ECC_SOFT:
@@ -2562,26 +2615,7 @@ int nand_scan (struct mtd_info *mtd, int maxchips)
 	}
 
 	mtd->eccsize = this->eccsize;
-
-	/* Set the number of read / write steps for one page to ensure ECC generation */
-	switch (this->eccmode) {
-	case NAND_ECC_HW12_2048:
-		this->eccsteps = mtd->oobblock / 2048;
-		break;
-	case NAND_ECC_HW3_512:
-	case NAND_ECC_HW6_512:
-	case NAND_ECC_HW8_512:
-		this->eccsteps = mtd->oobblock / 512;
-		break;
-	case NAND_ECC_HW3_256:
-	case NAND_ECC_SOFT:
-		this->eccsteps = mtd->oobblock / 256;
-		break;
-
-	case NAND_ECC_NONE:
-		this->eccsteps = 1;
-		break;
-	}
+	this->eccsteps = mtd->oobblock / this->eccsize;
 
 /* XXX U-BOOT XXX */
 #if 0
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 4b48564..1af0666 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -136,6 +136,16 @@ extern int nand_read_raw (struct mtd_info *mtd, uint8_t *buf, loff_t from, size_
 /* Hardware ECC 12 byte ECC per 2048 Byte data */
 #define NAND_ECC_HW12_2048	7
 
+/* The hardware does ECC transparently on transfers.  This is treated
+ * like NAND_ECC_NONE, except without lecturing the user that they really
+ * should use ECC.  Also, it bypasses verification of OOB data, as the
+ * ECC written by the hardware will differ from what the generic code
+ * expects.  The calculate_ecc method should not be called.  The
+ * correct_data method is used to return errors on read; only the mtd
+ * argument is used.
+ */
+#define NAND_ECC_TRANSPARENT 0x1000
+
 /*
  * Constants for Hardware ECC
 */
-- 
1.5.0.3

[U-Boot-Users] [PATCH 1/2] NAND: Add support for transparent hardware ECC.

[Stefan Roese]

Hi Scott,

sorry for the late response.

On Wednesday 16 May 2007, Scott Wood wrote:
> Some NAND controllers (such as on the MPC831x chips) have hardware ECC,
> but can only do it during a transfer (i.e. we can't implement
> calculate_ecc()).  When NAND_ECC_TRANSPARENT is used, then ECC errors on
> reads are reported through correct_data() (with no arguments other than
> the mtd device), and on writes through waitfunc()'s return value.

I would like to know the status of this "tranparent" hardware ECC mechanism
in the Linux source. I assume that you want to integrate this support there
too, right? I couldn't find any reference to this in the current mtd/nand
implementations and/or any reference to the MPC831x.

I'm asking, since the U-Boot nand code is based on the Linux mtd codebase,
and we should try to keep features common between both source codes as
often as possible.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [PATCH 1/2] NAND: Add support for transparent hardware ECC.

[Scott Wood]

Stefan Roese wrote:
> I would like to know the status of this "tranparent" hardware ECC mechanism
> in the Linux source. I assume that you want to integrate this support there
> too, right? I couldn't find any reference to this in the current mtd/nand
> implementations and/or any reference to the MPC831x.

I have a Linux patch from Nick Spence for nand on the 831x, but it needs 
a lot of cleanup before I can submit it, which I haven't had a chance to 
do yet.  It currently uses a rather ugly hack to get around the lack of 
transparent ECC support (as did the u-boot patch before I reworked it).

> I'm asking, since the U-Boot nand code is based on the Linux mtd codebase,
> and we should try to keep features common between both source codes as
> often as possible.

Agreed, though when writing the u-boot patch I looked at the kernel 
source, and it appears to have already diverged quite a bit.  When I get 
a chance to look at the 831x NAND Linux patch, I'll most likely 
implement similar transparent ECC support there.

-Scott