[Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[Brian Foster]

 Our current reference Linux kernel for the MAX32590 (JIBE)
 is based on v2.6.36.  (Unfortunately, upgrading to a more
 recent version is not within the timeframe for solving the
 current problem.)  Our recent reference boards use one of
 those Micron NAND chips with an on-die 4-bit ECC, which we
 have basically ignored:  To-date, we have simply used the
 usual 1-bit ECC (i.e., living dangerously!).

 This must change, and indeed we now have a case on my desk
 where, had we been using the on-die ECC, it would have saved
 us a ton of grief.  The problem is our kernel version is far
 too old to take advantage of any of the recent-ish work for
 on-die ECC.

 Hence, I am looking into the possibility of adding on-die ECC
 support to our JIBE controller driver specifically for such
 NAND chips (or at least the specific Micron NAND chip on the
 reference boards).  Broadly, pretending JIBE's H/W directly
 supports on-die ECC, but actually doing the work in the driver.
 A similar trick we played in the past (bitwise-inverted ECC
 (now obsoleted and long-removed from the driver)) suggests
 this is not too difficult.

 I am looking for hints (suggestions), gotchas (warnings),
 and/or any examples of similar (or other plausible) approaches.
 Or for something I am overlooking in (or available for) kernels
 of approximately the vintage we are using.

Thanks & cheers!
	-blf-

p.s.  At the present time, I am not too interested in the
     problem of converting existing boards.  This MAY change
     as the scope and details of the solution become more
     apparent.

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated               |  http://www.maximintegrated.com/

Re: [Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[David Mosberger-Tang]

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

[Resend as plain text...]

Attached is a patch relative to 2.6.27.y.   We use it with a 16-bit
wide Micron part needing 4-bit ECC.  It works for us, YMMV.  I'm
pretty sure the raw access is broken badly but we are not using that
so it's not a problem from us.  The patch assumes that on-die ECC is
enabled in the bootstrap loader.

On Wed, Jun 19, 2013 at 8:14 AM, Brian Foster
<brian.foster@maximintegrated.com> wrote:
>
>  Our current reference Linux kernel for the MAX32590 (JIBE)
>  is based on v2.6.36.  (Unfortunately, upgrading to a more
>  recent version is not within the timeframe for solving the
>  current problem.)  Our recent reference boards use one of
>  those Micron NAND chips with an on-die 4-bit ECC, which we
>  have basically ignored:  To-date, we have simply used the
>  usual 1-bit ECC (i.e., living dangerously!).
>
>  This must change, and indeed we now have a case on my desk
>  where, had we been using the on-die ECC, it would have saved
>  us a ton of grief.  The problem is our kernel version is far
>  too old to take advantage of any of the recent-ish work for
>  on-die ECC.
>
>  Hence, I am looking into the possibility of adding on-die ECC
>  support to our JIBE controller driver specifically for such
>  NAND chips (or at least the specific Micron NAND chip on the
>  reference boards).  Broadly, pretending JIBE's H/W directly
>  supports on-die ECC, but actually doing the work in the driver.
>  A similar trick we played in the past (bitwise-inverted ECC
>  (now obsoleted and long-removed from the driver)) suggests
>  this is not too difficult.
>
>  I am looking for hints (suggestions), gotchas (warnings),
>  and/or any examples of similar (or other plausible) approaches.
>  Or for something I am overlooking in (or available for) kernels
>  of approximately the vintage we are using.
>
> Thanks & cheers!
>         -blf-
>
> p.s.  At the present time, I am not too interested in the
>      problem of converting existing boards.  This MAY change
>      as the scope and details of the solution become more
>      apparent.
>
> --
> Brian Foster
> Principal MTS, Software        |  La Ciotat, France
> Maxim Integrated               |  http://www.maximintegrated.com/
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

[-- Attachment #2: nand-on-die-ecc-2.6.27.diff.txt --]
[-- Type: text/plain, Size: 21120 bytes --]

diff --git a/drivers/mtd/nand/atmel_nand.c b/drivers/mtd/nand/atmel_nand.c
index 3387e0d..9d352f3 100644
--- a/drivers/mtd/nand/atmel_nand.c
+++ b/drivers/mtd/nand/atmel_nand.c
@@ -255,7 +255,7 @@ static int atmel_nand_calculate(struct mtd_info *mtd,
  * buf:        buffer to store read data
  */
 static int atmel_nand_read_page(struct mtd_info *mtd,
-		struct nand_chip *chip, uint8_t *buf)
+				struct nand_chip *chip, uint8_t *buf, int page)
 {
 	int eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d1129ba..b7abe82 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -82,6 +82,20 @@ static struct nand_ecclayout nand_oob_64 = {
 		 .length = 38}}
 };
 
+static struct nand_ecclayout nand_oob_64_on_die = {
+	.eccbytes = 32,
+	.eccpos = {
+		    8,  9, 10, 11, 12, 13, 14, 15,
+		   24, 25, 26, 27, 28, 29, 30, 31,
+		   40, 41, 42, 43, 44, 45, 46, 47,
+		   56, 57, 58, 59, 60, 61, 62, 63},
+	.oobfree = {
+		{.offset =  4,	 .length = 4},
+		{.offset = 20,	 .length = 4},
+		{.offset = 36,	 .length = 4},
+		{.offset = 52,	 .length = 4}}
+};
+
 static int nand_get_device(struct nand_chip *chip, struct mtd_info *mtd,
 			   int new_state);
 
@@ -434,6 +448,32 @@ void nand_wait_ready(struct mtd_info *mtd)
 }
 EXPORT_SYMBOL_GPL(nand_wait_ready);
 
+static int
+set_column (struct mtd_info *mtd, struct nand_chip *chip,
+	    unsigned int command, int column,
+	    unsigned int column_width, int ctrl)
+{
+	switch (command) {
+	case NAND_CMD_READID:
+	case NAND_CMD_GET_FEATURES:
+	case NAND_CMD_SET_FEATURES:
+		chip->cmd_ctrl(mtd, column, ctrl);
+		ctrl &= ~NAND_CTRL_CHANGE;
+		break;
+
+	default:
+		/* Adjust columns for 16 bit buswidth */
+		if (chip->options & NAND_BUSWIDTH_16)
+			column >>= 1;
+		chip->cmd_ctrl(mtd, column, ctrl);
+		ctrl &= ~NAND_CTRL_CHANGE;
+		if (column_width > 8)
+			chip->cmd_ctrl(mtd, column >> 8, ctrl);
+		break;
+	}
+	return ctrl;
+}
+
 /**
  * nand_command - [DEFAULT] Send command to NAND device
  * @mtd:	MTD device structure
@@ -477,13 +517,8 @@ static void nand_command(struct mtd_info *mtd, unsigned int command,
 	 */
 	ctrl = NAND_CTRL_ALE | NAND_CTRL_CHANGE;
 	/* Serially input address */
-	if (column != -1) {
-		/* Adjust columns for 16 bit buswidth */
-		if (chip->options & NAND_BUSWIDTH_16)
-			column >>= 1;
-		chip->cmd_ctrl(mtd, column, ctrl);
-		ctrl &= ~NAND_CTRL_CHANGE;
-	}
+	if (column != -1)
+		ctrl = set_column(mtd, chip, command, column, 8, ctrl);
 	if (page_addr != -1) {
 		chip->cmd_ctrl(mtd, page_addr, ctrl);
 		ctrl &= ~NAND_CTRL_CHANGE;
@@ -566,14 +601,9 @@ static void nand_command_lp(struct mtd_info *mtd, unsigned int command,
 		int ctrl = NAND_CTRL_CHANGE | NAND_NCE | NAND_ALE;
 
 		/* Serially input address */
-		if (column != -1) {
-			/* Adjust columns for 16 bit buswidth */
-			if (chip->options & NAND_BUSWIDTH_16)
-				column >>= 1;
-			chip->cmd_ctrl(mtd, column, ctrl);
-			ctrl &= ~NAND_CTRL_CHANGE;
-			chip->cmd_ctrl(mtd, column >> 8, ctrl);
-		}
+		if (column != -1)
+			ctrl = set_column (mtd, chip, command, column, 16,
+					   ctrl);
 		if (page_addr != -1) {
 			chip->cmd_ctrl(mtd, page_addr, ctrl);
 			chip->cmd_ctrl(mtd, page_addr >> 8,
@@ -750,7 +780,7 @@ static int nand_wait(struct mtd_info *mtd, struct nand_chip *chip)
  * @buf:	buffer to store read data
  */
 static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
-			      uint8_t *buf)
+			      uint8_t *buf, int page)
 {
 	chip->read_buf(mtd, buf, mtd->writesize);
 	chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);
@@ -764,7 +794,7 @@ static int nand_read_page_raw(struct mtd_info *mtd, struct nand_chip *chip,
  * @buf:	buffer to store read data
  */
 static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf)
+				uint8_t *buf, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -774,7 +804,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
 	uint8_t *ecc_code = chip->buffers->ecccode;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
 
-	chip->ecc.read_page_raw(mtd, chip, buf);
+	chip->ecc.read_page_raw(mtd, chip, buf, page);
 
 	for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize)
 		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
@@ -805,7 +835,7 @@ static int nand_read_page_swecc(struct mtd_info *mtd, struct nand_chip *chip,
  * @readlen	data length
  * @buf:	buffer to store read data
  */
-static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, uint32_t data_offs, uint32_t readlen, uint8_t *bufpoi)
+static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, uint32_t data_offs, uint32_t readlen, uint8_t *bufpoi, int page)
 {
 	int start_step, end_step, num_steps;
 	uint32_t *eccpos = chip->ecc.layout->eccpos;
@@ -887,7 +917,7 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip, uint3
  * Not for syndrome calculating ecc controllers which need a special oob layout
  */
 static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
-				uint8_t *buf)
+				uint8_t *buf, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -932,7 +962,7 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
  * we need a special oob layout and handling.
  */
 static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
-				   uint8_t *buf)
+				   uint8_t *buf, int page)
 {
 	int i, eccsize = chip->ecc.size;
 	int eccbytes = chip->ecc.bytes;
@@ -1025,6 +1055,80 @@ static uint8_t *nand_transfer_oob(struct nand_chip *chip, uint8_t *oob,
 }
 
 /**
+ * nand_onfi_set_features- [REPLACEABLE] set features for ONFI nand
+ * @mtd: MTD device structure
+ * @chip: nand chip info structure
+ * @addr: feature address.
+ * @subfeature_param: the subfeature parameters, a four bytes array.
+ */
+static int nand_onfi_set_features(struct mtd_info *mtd, struct nand_chip *chip,
+			int addr, uint8_t *subfeature_param)
+{
+	uint16_t buf[ONFI_SUBFEATURE_PARAM_LEN];
+	size_t len = ONFI_SUBFEATURE_PARAM_LEN;
+	int status, i;
+
+	chip->cmdfunc(mtd, NAND_CMD_SET_FEATURES, addr, -1);
+	if (chip->options & NAND_BUSWIDTH_16) {
+		/*
+		 * ONFI says parameters are always transferred on the
+		 * lower 8-bits of the databus.  Since there is no
+		 * chip->write_byte callback, we have to convert
+		 * subfeature_param to 16-bit data.
+		 */
+		for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
+			buf[i] = subfeature_param[i];
+		subfeature_param = (uint8_t *) buf;
+		len = sizeof (buf);
+	}
+	chip->write_buf(mtd, subfeature_param, len);
+	status = chip->waitfunc(mtd, chip);
+	if (status & NAND_STATUS_FAIL)
+		return -EIO;
+	return 0;
+}
+
+/**
+ * nand_onfi_get_features- [REPLACEABLE] get features for ONFI nand
+ * @mtd: MTD device structure
+ * @chip: nand chip info structure
+ * @addr: feature address.
+ * @subfeature_param: the subfeature parameters, a four bytes array.
+ */
+static int nand_onfi_get_features(struct mtd_info *mtd, struct nand_chip *chip,
+			int addr, uint8_t *subfeature_param)
+{
+	int i;
+
+	/* clear the sub feature parameters */
+	memset(subfeature_param, 0, ONFI_SUBFEATURE_PARAM_LEN);
+
+	chip->cmdfunc(mtd, NAND_CMD_GET_FEATURES, addr, -1);
+	/*
+	 * ONFI says parameters are always transferred on the
+	 * lower 8-bits of the databus.  Use read_byte() since
+	 * that works even on 16-bit devices.
+	 */
+	for (i = 0; i < ONFI_SUBFEATURE_PARAM_LEN; ++i)
+	  subfeature_param[i] = chip->read_byte(mtd);
+	return 0;
+}
+
+static int
+set_on_die_ecc (struct mtd_info *mtd, struct nand_chip *chip, int on)
+{
+	u8 data[ONFI_SUBFEATURE_PARAM_LEN] = { 0, };
+
+	if (chip->ecc.mode != NAND_ECC_HW_ON_DIE)
+		return 0;
+
+	if (on)
+		data[0] = 0x8;
+
+	return nand_onfi_set_features(mtd, chip, 0x90, data);
+}
+
+/**
  * nand_do_read_ops - [Internal] Read data with ECC
  *
  * @mtd:	MTD device structure
@@ -1068,17 +1172,20 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			bufpoi = aligned ? buf : chip->buffers->databuf;
 
 			if (likely(sndcmd)) {
+				if (unlikely(ops->mode == MTD_OOB_RAW))
+					set_on_die_ecc(mtd, chip, 0);
+
 				chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
 				sndcmd = 0;
 			}
 
 			/* Now read the page into the buffer */
 			if (unlikely(ops->mode == MTD_OOB_RAW))
-				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi);
+				ret = chip->ecc.read_page_raw(mtd, chip, bufpoi, page);
 			else if (!aligned && NAND_SUBPAGE_READ(chip) && !oob)
-				ret = chip->ecc.read_subpage(mtd, chip, col, bytes, bufpoi);
+				ret = chip->ecc.read_subpage(mtd, chip, col, bytes, bufpoi, page);
 			else
-				ret = chip->ecc.read_page(mtd, chip, bufpoi);
+				ret = chip->ecc.read_page(mtd, chip, bufpoi, page);
 			if (ret < 0)
 				break;
 
@@ -1149,6 +1256,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 			sndcmd = 1;
 	}
 
+	set_on_die_ecc(mtd, chip, 1);
+
 	ops->retlen = ops->len - (size_t) readlen;
 	if (oob)
 		ops->oobretlen = ops->ooblen - oobreadlen;
@@ -1162,6 +1271,173 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	return  mtd->ecc_stats.corrected - stats.corrected ? -EUCLEAN : 0;
 }
 
+/*
+ * Return the number of bits that differ between buffers SRC1 and
+ * SRC2, both of which are LEN bytes long.
+ *
+ * This code could be optimized for, but it only gets called on pages
+ * with bitflips and compared to the cost of migrating an eraseblock,
+ * the execution time here is trivial...
+ */
+static int
+bitdiff (const void *s1, const void *s2, size_t len)
+{
+	const uint8_t *src1 = s1, *src2 = s2;
+	int count = 0, i;
+
+	for (i = 0; i < len; ++i)
+		count += hweight8 (*src1++ ^ *src2++);
+	return count;
+}
+
+static int
+check_for_bitflips (struct mtd_info *mtd, struct nand_chip *chip, int page)
+{
+	int flips = 0, max_bitflips = 0, i, j, read_size;
+	uint8_t *chkbuf, *rawbuf, *chkoob, *rawoob;
+	uint32_t *eccpos;
+
+	chkbuf = chip->buffers->chkbuf;
+	rawbuf = chip->buffers->rawbuf;
+	read_size = mtd->writesize + mtd->oobsize;
+
+	/* Read entire page w/OOB area with on-die ECC on: */
+	chip->read_buf(mtd, chkbuf, read_size);
+
+	/* Re-read page with on-die ECC off: */
+	set_on_die_ecc(mtd, chip, 0);
+	{
+		chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+		chip->read_buf(mtd, rawbuf, read_size);
+	}
+	set_on_die_ecc(mtd, chip, 1);
+
+	chkoob = chkbuf + mtd->writesize;
+	rawoob = rawbuf + mtd->writesize;
+	eccpos = chip->ecc.layout->eccpos;
+	for (i = 0; i < chip->ecc.steps; ++i) {
+		/* Count bit flips in the actual data area: */
+		flips = bitdiff (chkbuf, rawbuf, chip->ecc.size);
+		/* Count bit flips in the ECC bytes: */
+		for (j = 0; j < chip->ecc.bytes; ++j) {
+			flips += hweight8(chkoob[*eccpos] ^ rawoob[*eccpos]);
+			++eccpos;
+		}
+		max_bitflips = max_t(int, max_bitflips, flips);
+		if (max_bitflips >= 3)
+			mtd->ecc_stats.corrected++;
+		chkbuf += chip->ecc.size;
+		rawbuf += chip->ecc.size;
+	}
+
+	/* Re-issue the READ command for the actual data read that follows.  */
+	chip->cmdfunc(mtd, NAND_CMD_READ0, 0x00, page);
+
+	return max_bitflips;
+}
+
+static int check_read_status_on_die (struct mtd_info *mtd, struct nand_chip *chip, int page)
+{
+	uint8_t status;
+
+	/* Check ECC status of page just transferred into NAND's page buffer: */
+	chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+	status = chip->read_byte(mtd);
+
+	/* Switch back to data reading: */
+	chip->cmd_ctrl(mtd, NAND_CMD_READ0,
+		       NAND_NCE | NAND_CLE | NAND_CTRL_CHANGE);
+	chip->cmd_ctrl(mtd, NAND_CMD_NONE,
+		       NAND_NCE | NAND_CTRL_CHANGE);
+
+	if (status & NAND_STATUS_FAIL) {
+		/* Page has invalid ECC. */
+		mtd->ecc_stats.failed++;
+	} else if (status & NAND_STATUS_REWRITE) {
+		/*
+		 * The Micron chips turn on the REWRITE status bit for
+		 * ANY bit flips.  Some pages have stuck bits, so we
+		 * don't want to migrate a block just because of
+		 * single bit errors because otherwise, that block
+		 * would effectively become unusable.  So, work out in
+		 * software what the max number of flipped bits is for
+		 * all subpages in a page:
+		 */
+		check_for_bitflips (mtd, chip, page);
+	}
+	return 0;
+}
+
+/**
+ * nand_read_subpage_on_die - [REPLACEABLE] raw sub-page read function
+ * @mtd: mtd info structure
+ * @chip: nand chip info structure
+ * @data_offs: offset of requested data within the page
+ * @readlen: data length
+ * @bufpoi: buffer to store read data
+ */
+static int nand_read_subpage_on_die(struct mtd_info *mtd, struct nand_chip *chip,
+			uint32_t data_offs, uint32_t readlen, uint8_t *bufpoi, int page)
+{
+	int start_step, end_step, num_steps, ret;
+	int data_col_addr;
+	int datafrag_len;
+	uint32_t failed;
+	uint8_t *p;
+
+	/* Column address within the page aligned to ECC size */
+	start_step = data_offs / chip->ecc.size;
+	end_step = (data_offs + readlen - 1) / chip->ecc.size;
+	num_steps = end_step - start_step + 1;
+
+	/* Data size aligned to ECC ecc.size */
+	datafrag_len = num_steps * chip->ecc.size;
+	data_col_addr = start_step * chip->ecc.size;
+	p = bufpoi + data_col_addr;
+
+	failed = mtd->ecc_stats.failed;
+
+	ret = check_read_status_on_die (mtd, chip, page);
+	if (ret < 0 || mtd->ecc_stats.failed != failed) {
+		memset (p, 0, datafrag_len);
+		return ret;
+	}
+
+	/* If we read not a page aligned data */
+	if (data_col_addr != 0)
+		chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_col_addr, -1);
+
+	chip->read_buf(mtd, p, datafrag_len);
+
+	return ret;
+}
+
+/**
+ * nand_read_page_on_die - [INTERN] read raw page data without ecc
+ * @mtd: mtd info structure
+ * @chip: nand chip info structure
+ * @buf: buffer to store read data
+ * @oob_required: caller requires OOB data read to chip->oob_poi
+ * @page: page number to read
+ */
+static int nand_read_page_on_die(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf,
+				 int page)
+{
+	uint32_t failed;
+	int ret;
+
+	failed = mtd->ecc_stats.failed;
+
+	ret = check_read_status_on_die (mtd, chip, page);
+	if (ret < 0 || mtd->ecc_stats.failed != failed) {
+		memset (buf, 0, mtd->writesize);
+		return ret;
+	}
+
+	chip->read_buf(mtd, buf, mtd->writesize);
+	return ret;
+}
+
 /**
  * nand_read - [MTD Interface] MTD compability function for nand_do_read_ecc
  * @mtd:	MTD device structure
@@ -1602,6 +1878,9 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 {
 	int status;
 
+	if (unlikely(raw))
+		set_on_die_ecc(mtd, chip, 0);
+
 	chip->cmdfunc(mtd, NAND_CMD_SEQIN, 0x00, page);
 
 	if (unlikely(raw))
@@ -1627,11 +1906,16 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 			status = chip->errstat(mtd, chip, FL_WRITING, status,
 					       page);
 
+		if (unlikely(raw))
+			set_on_die_ecc(mtd, chip, 1);
+
 		if (status & NAND_STATUS_FAIL)
 			return -EIO;
 	} else {
 		chip->cmdfunc(mtd, NAND_CMD_CACHEDPROG, -1, -1);
 		status = chip->waitfunc(mtd, chip);
+		if (unlikely(raw))
+			set_on_die_ecc(mtd, chip, 1);
 	}
 
 #ifdef CONFIG_MTD_NAND_VERIFY_WRITE
@@ -2518,6 +2802,7 @@ int nand_scan_ident(struct mtd_info *mtd, int maxchips)
 int nand_scan_tail(struct mtd_info *mtd)
 {
 	int i;
+	u8 features[ONFI_SUBFEATURE_PARAM_LEN];
 	struct nand_chip *chip = mtd->priv;
 
 	if (!(chip->options & NAND_OWN_BUFFERS))
@@ -2552,6 +2837,16 @@ int nand_scan_tail(struct mtd_info *mtd)
 	if (!chip->write_page)
 		chip->write_page = nand_write_page;
 
+	if (nand_onfi_get_features(mtd, chip, 0x90, features) >= 0) {
+		if (features[0] & 0x08) {
+			/*
+			 * If the chip has on-die ECC enabled, we kind of have to use it.
+			 */
+			chip->ecc.mode = NAND_ECC_HW_ON_DIE;
+			pr_info("NAND device: Using on-die ECC\n");
+		}
+	}
+
 	/*
 	 * check ECC mode, default to software if 3byte/512byte hardware ECC is
 	 * selected and we have 256 byte pagesize fallback to software ECC
@@ -2613,6 +2908,19 @@ int nand_scan_tail(struct mtd_info *mtd)
 		chip->ecc.bytes = 3;
 		break;
 
+	case NAND_ECC_HW_ON_DIE:
+		chip->ecc.layout = &nand_oob_64_on_die;
+		chip->ecc.read_page = nand_read_page_on_die;
+		chip->ecc.read_subpage = nand_read_subpage_on_die;
+		chip->ecc.write_page = nand_write_page_raw;
+		chip->ecc.read_oob = nand_read_oob_std;
+		chip->ecc.read_page_raw = nand_read_page_raw;
+		chip->ecc.write_page_raw = nand_write_page_raw;
+		chip->ecc.write_oob = nand_write_oob_std;
+		chip->ecc.size = 512;
+		chip->ecc.bytes = 8;
+		break;
+
 	case NAND_ECC_NONE:
 		printk(KERN_WARNING "NAND_ECC_NONE selected by board driver. "
 		       "This is not recommended !!\n");
diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c
index 0b1c485..f09e80c 100644
--- a/drivers/mtd/nand/nand_bbt.c
+++ b/drivers/mtd/nand/nand_bbt.c
@@ -1117,7 +1117,9 @@ static uint8_t mirror_pattern[] = {'1', 't', 'b', 'B' };
 static struct nand_bbt_descr bbt_main_descr = {
 	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
 		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs =	8,
+	/* Micron MT29F4G16ABADAWP places ECC at offset 8, so we use 4
+	   instead (User metadata I).  */
+	.offs =	4,
 	.len = 4,
 	.veroffs = 12,
 	.maxblocks = 4,
@@ -1127,7 +1129,9 @@ static struct nand_bbt_descr bbt_main_descr = {
 static struct nand_bbt_descr bbt_mirror_descr = {
 	.options = NAND_BBT_LASTBLOCK | NAND_BBT_CREATE | NAND_BBT_WRITE
 		| NAND_BBT_2BIT | NAND_BBT_VERSION | NAND_BBT_PERCHIP,
-	.offs =	8,
+	/* Micron MT29F4G16ABADAWP places ECC at offset 8, so we use 4
+	   instead (User metadata I).  */
+	.offs =	4,
 	.len = 4,
 	.veroffs = 12,
 	.maxblocks = 4,
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 81774e5..cbdb01e 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -78,6 +78,8 @@ extern void nand_wait_ready(struct mtd_info *mtd);
 #define NAND_CMD_RNDIN		0x85
 #define NAND_CMD_READID		0x90
 #define NAND_CMD_ERASE2		0xd0
+#define NAND_CMD_GET_FEATURES	0xee
+#define NAND_CMD_SET_FEATURES	0xef
 #define NAND_CMD_RESET		0xff
 
 /* Extended commands for large page devices */
@@ -109,6 +111,7 @@ extern void nand_wait_ready(struct mtd_info *mtd);
 /* Status bits */
 #define NAND_STATUS_FAIL	0x01
 #define NAND_STATUS_FAIL_N1	0x02
+#define NAND_STATUS_REWRITE	0x08
 #define NAND_STATUS_TRUE_READY	0x20
 #define NAND_STATUS_READY	0x40
 #define NAND_STATUS_WP		0x80
@@ -121,6 +124,7 @@ typedef enum {
 	NAND_ECC_SOFT,
 	NAND_ECC_HW,
 	NAND_ECC_HW_SYNDROME,
+	NAND_ECC_HW_ON_DIE,
 } nand_ecc_modes_t;
 
 /*
@@ -178,8 +182,9 @@ typedef enum {
 #define NAND_HAS_CACHEPROG(chip) ((chip->options & NAND_CACHEPRG))
 #define NAND_HAS_COPYBACK(chip) ((chip->options & NAND_COPYBACK))
 /* Large page NAND with SOFT_ECC should support subpage reads */
-#define NAND_SUBPAGE_READ(chip) ((chip->ecc.mode == NAND_ECC_SOFT) \
-					&& (chip->page_shift > 9))
+#define NAND_SUBPAGE_READ(chip)	(((chip->ecc.mode == NAND_ECC_SOFT) ||	\
+				  (chip->ecc.mode == NAND_ECC_HW_ON_DIE)) \
+				 && (chip->page_shift > 9))
 
 /* Mask to zero out the chip options, which come from the id table */
 #define NAND_CHIPOPTIONS_MSK	(0x0000ffff & ~NAND_NO_AUTOINCR)
@@ -270,17 +275,17 @@ struct nand_ecc_ctrl {
 					   uint8_t *calc_ecc);
 	int			(*read_page_raw)(struct mtd_info *mtd,
 						 struct nand_chip *chip,
-						 uint8_t *buf);
+						 uint8_t *buf, int page);
 	void			(*write_page_raw)(struct mtd_info *mtd,
 						  struct nand_chip *chip,
 						  const uint8_t *buf);
 	int			(*read_page)(struct mtd_info *mtd,
 					     struct nand_chip *chip,
-					     uint8_t *buf);
+					     uint8_t *buf, int page);
 	int			(*read_subpage)(struct mtd_info *mtd,
 					     struct nand_chip *chip,
 					     uint32_t offs, uint32_t len,
-					     uint8_t *buf);
+						uint8_t *buf, int page);
 	void			(*write_page)(struct mtd_info *mtd,
 					      struct nand_chip *chip,
 					      const uint8_t *buf);
@@ -306,6 +311,8 @@ struct nand_buffers {
 	uint8_t	ecccalc[NAND_MAX_OOBSIZE];
 	uint8_t	ecccode[NAND_MAX_OOBSIZE];
 	uint8_t databuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+	uint8_t chkbuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
+	uint8_t rawbuf[NAND_MAX_PAGESIZE + NAND_MAX_OOBSIZE];
 };
 
 /**
@@ -611,6 +618,9 @@ struct platform_nand_data {
 	struct platform_nand_ctrl	ctrl;
 };
 
+/* ONFI subfeature parameters length */
+#define ONFI_SUBFEATURE_PARAM_LEN	4
+
 /* Some helpers to access the data structures */
 static inline
 struct platform_nand_chip *get_platform_nandchip(struct mtd_info *mtd)

Re: [Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[Brian Foster]

On Wednesday 19-June-2013 14:40:45 David Mosberger-Tang wrote:
> Attached is a patch relative to 2.6.27.y.   We use it with a 16-bit
> wide Micron part needing 4-bit ECC.  It works for us, YMMV.  I'm
> pretty sure the raw access is broken badly but we are not using that
> so it's not a problem from us.  The patch assumes that on-die ECC is
> enabled in the bootstrap loader.

David,

 Thanks!  I've been working on getting our kernel and Das U-Boot
 up to a common baseline w.r.t. the NAND-handling (including a
 on-die compatible BBT layout in the OOB and other details), so
 I had not looked too closely at your Patch until now-ish....

 In examining it, I may have spotted an oddity:  When counting
 bit-flips, you only count them in the main-data area and in
 the ECC-area (in the OOB).  What you do not count are any in
 the on-die ECC-protected bits in the OOB (which, in fact,
 happens to be all of .oobfree).

 Whilst not necessarily wrong, it does mean you may under-count
 the true number of ECC-detected bit-flips, under-reporting the
 number of corrections, and hence (inadvertently?) fooling the
 system into thinking the block is in better health than it
 really is.

 Or I may mis-understanding something here....

cheers!
	-blf-
-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated               |  http://www.maximintegrated.com/

[PATCH] Init ONFI get/set features in a more logical place

[Brian Foster]

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


 Attached (SORRY! see below) is a Patch based on some
 work being done on a v2.6.36-based kernel (with parts
 of more recent MTD/NAND support backported), to add
 support for Micron on-die ECC to Maxim Integrated's
 MAX32590 (JIBE) reference boards.

 This Patch makes a minor change to initialization order
 so code between nand_scan_ident() and nand_scan_tail()
 can easily get (or set) ONFI features.

 Since the original Patch is based on that older kernel,
 I can neither confirm nor deny it builds and works with
 the latest kernel/MTD/NAND source.  However, I also have
 no reason to believe it won't build or work.

 Due to limitations of the e-mail server, the Patch is
 attached rather then being included inline.  Apologies.
 Even so, the bloody server has _probably_ added CRs (\r),
 so you will _probably_ have to run the attached Patch
 through dos2unix(1) before it will apply.  ;-\ 

 CC-ed to the author of the original work.  Since I do
 not regularly read this list (linux-mtd), any e-mail
 not CC-ed to me _may_ not be seen.  Apologies!

cheers!
	-blf-

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Office:  +33 (0)4 42 98 15 35  |  Fax:  +33 (0)4 42 08 33 19
Maxim Integrated               |  http://www.maximintegrated.com/

[-- Attachment #2: 0001-Init-ONFI-get-set-features-in-a-more-logical-place.patch --]
[-- Type: text/x-patch, Size: 1891 bytes --]

>From 400afdd7734c74881e208ae4b7fdb2a3dc2220bb Mon Sep 17 00:00:00 2001
From: Brian Foster <brian.foster@maximintegrated.com>
Date: Thu, 4 Jul 2013 13:35:31 +0200
Subject: [PATCH] Init ONFI get/set features in a more logical place

For some reason the ONFI get/set feature handlers were
being initialized in nand_scan_tail(), making it rather
difficult to use the default handlers after nand_scan_ident(),
which determines whether or not the NAND chip is ONFI-compliant.

Move the handler initialization into nand_set_defaults(),
where most handlers are initialized.

Change-Id: I055179ab3d4ea23780879197f4add02e27e6c0d1
Signed-off-by: Brian Foster <brian.foster@maximintegrated.com>
Cc: Florian Fainelli <ffainelli@freebox.fr>
---
 drivers/mtd/nand/nand_base.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index dfcd0a5..aab79f7 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2814,6 +2814,11 @@ static void nand_set_defaults(struct nand_chip *chip, int busw)
 		init_waitqueue_head(&chip->controller->wq);
 	}
 
+	/* set for ONFI nand */
+	if (!chip->onfi_set_features)
+		chip->onfi_set_features = nand_onfi_set_features;
+	if (!chip->onfi_get_features)
+		chip->onfi_get_features = nand_onfi_get_features;
 }
 
 /* Sanitize ONFI strings so we can safely print them */
@@ -3446,12 +3451,6 @@ int nand_scan_tail(struct mtd_info *mtd)
 	if (!chip->write_page)
 		chip->write_page = nand_write_page;
 
-	/* set for ONFI nand */
-	if (!chip->onfi_set_features)
-		chip->onfi_set_features = nand_onfi_set_features;
-	if (!chip->onfi_get_features)
-		chip->onfi_get_features = nand_onfi_get_features;
-
 	/*
 	 * Check ECC mode, default to software if 3byte/512byte hardware ECC is
 	 * selected and we have 256 byte pagesize fallback to software ECC
-- 
1.7.9.5

Re: [PATCH] Init ONFI get/set features in a more logical place

[Artem Bityutskiy]

On Thu, 2013-07-04 at 14:05 +0200, Brian Foster wrote:
>  Attached (SORRY! see below) is a Patch based on some
>  work being done on a v2.6.36-based kernel (with parts
>  of more recent MTD/NAND support backported), to add
>  support for Micron on-die ECC to Maxim Integrated's
>  MAX32590 (JIBE) reference boards.

Would you please resend it in-line?

-- 
Best Regards,
Artem Bityutskiy

Re: [Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[Brian Foster]

On Wednesday 03-July-2013 09:20:47 David Mosberger-Tang wrote:
> I think you're right.  We use UBI which doesn't use oob at all
> (apart from ECC bytes) so we're not worried about errors in the
> OOB-areas that are ECC-protected.

David,

 Ok.  Pedantically, you are using the OOB in one very small
 sense:  The OOB is where the BBT marker and version is kept.
 So UBI does use OOB in, typically, just two (2) blocks.  ;-)

 In our case, whilst _we_ are like you on our reference boards,
 potentially some people may use JFFS2 or YAFFS, so I suspect
 I will include those EEC-checked bytes in the OOB in bit-flip
 counting.

cheers!
	-blf-

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated               |  http://www.maximintegrated.com/

Re: [Q] Using Micron 4-bit on-die ECC with v2.6.36 kernel?

[Brian Foster]

On Thursday 04-July-2013 05:37:23 David Mosberger-Tang wrote:
> BBT markers don't assume ECC correction (there are multiple copies).

 At least in v2.6.36, that is misleading albeit not
 incorrect.  There _is_ a duplicate BBT block (called
 the “mirror”), but neither it nor the primary BBT block
 have multiple copies of the BBT marker (or the version).
 (And, as it happens, the markers differ between the two
 blocks (I'm not too sure why?).)

 So, unless both BBT blocks fail, you _should_ always
 be able to find and use one.  What happens if both
 happen to fail (in v2.6.36) is not-clear (to me).

 I have not checked to see what the situation is in the
 latest kernel.

> However, I certainly agree that it'd be better to check all bits
> covered by ECC.  It just didn't even cross my mind when we wrote
> the code in question.  Had much bigger fish to fry at the time,
> like do we need to recall all the devices in the field...

 I quite understand!  Fortunately for us, there's no reason
 to recall any of our devices, since they are reference
 boards that come with the full source, which can be updated
 from our GIT servers.  Hence, the users/developers can do
 what they like.  What it does mean is I have to be a bit
 careful not to break the existing (“legacy”) situation!

cheers!
	-blf-

-- 
Brian Foster
Principal MTS, Software        |  La Ciotat, France
Maxim Integrated               |  http://www.maximintegrated.com/