[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Peter Tyser]

The driver-specific verify_buf() function can be replaced with the
standard read_page_raw() function to verify writes.  This will allow
verify_buf() to be removed from individual drivers.  verify_buf() is no
longer supported in mainline Linux, so it is a pain to continue
supporting.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---

 drivers/mtd/nand/nand_base.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 63bdf65..788846a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2394,6 +2394,7 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 		int oob_required, int page, int cached, int raw)
 {
 	int status, subpage;
+	__maybe_unused uint8_t *vfy_buf;
 
 	if (!(chip->options & NAND_NO_SUBPAGE_WRITE) &&
 		chip->ecc.write_subpage)
@@ -2443,10 +2444,19 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 #ifdef __UBOOT__
 #if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
+	vfy_buf = malloc(mtd->writesize);
+	if (!vfy_buf)
+		return -ENOMEM;
+
 	/* Send command to read back the data */
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+
+	status = memcmp(buf, vfy_buf, mtd->writesize);
 
-	if (chip->verify_buf(mtd, buf, mtd->writesize))
+	free(vfy_buf);
+
+	if (status)
 		return -EIO;
 
 	/* Make sure the next page prog is preceded by a status read */
-- 
1.9.1

[U-Boot] [PATCH 2/5] mtd: davinci_nand: Use common read function instead of verify_buf()

[Peter Tyser]

The driver-specific verify_buf() function can be replaced with the
standard read_page_raw() function to verify writes.  This will allow
verify_buf() to be removed.  verify_buf() is no longer supported in
mainline Linux, so it is a pain to continue supporting.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
I don't have davinci hardware to test on, but the change should
mirror the nand_base.c change.

 drivers/mtd/nand/davinci_nand.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 41689b5..b4e22d1 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -370,6 +370,7 @@ static int nand_davinci_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	int status;
 	int ret = 0;
 	struct nand_ecclayout *saved_ecc_layout;
+	__maybe_unused uint8_t *vfy_buf;
 
 	/* save current ECC layout and assign Keystone RBL ECC layout */
 	if (page < CONFIG_KEYSTONE_NAND_MAX_RBL_PAGE) {
@@ -406,16 +407,24 @@ static int nand_davinci_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 	}
 
 #ifdef CONFIG_MTD_NAND_VERIFY_WRITE
+	vfy_buf = malloc(mtd->writesize);
+	if (!vfy_buf) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
 	/* Send command to read back the data */
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
+	chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
 
-	if (chip->verify_buf(mtd, buf, mtd->writesize)) {
+	if (memcmp(buf, vfy_buf, mtd->writesize))
 		ret = -EIO;
-		goto err;
-	}
+
+	free(vfy_buf);
 
 	/* Make sure the next page prog is preceded by a status read */
-	chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
+	if (!ret)
+		chip->cmdfunc(mtd, NAND_CMD_STATUS, -1, -1);
 #endif
 err:
 	/* restore ECC layout */
-- 
1.9.1

[U-Boot] [PATCH 3/5] mtd: nand: Remove nand_verify_buf() function

[Peter Tyser]

The nand_verify_buf() function is no longer used, so remove it.  This
function has been removed in mainline Linux for a long time, so it
brings U-Boot's NAND implementation a bit closer to its source.

Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---

 board/prodrive/alpr/nand.c       | 16 -------------
 board/socrates/nand.c            | 25 --------------------
 drivers/mtd/nand/fsl_elbc_nand.c | 38 ------------------------------
 drivers/mtd/nand/fsl_ifc_nand.c  | 38 ------------------------------
 drivers/mtd/nand/fsl_upm.c       | 19 +--------------
 drivers/mtd/nand/mpc5121_nfc.c   | 26 --------------------
 drivers/mtd/nand/mxc_nand.c      | 33 --------------------------
 drivers/mtd/nand/nand_base.c     | 51 ----------------------------------------
 drivers/mtd/nand/ndfc.c          | 20 +---------------
 include/linux/mtd/nand.h         |  5 ----
 10 files changed, 2 insertions(+), 269 deletions(-)

diff --git a/board/prodrive/alpr/nand.c b/board/prodrive/alpr/nand.c
index 5427de5..ca40cea 100644
--- a/board/prodrive/alpr/nand.c
+++ b/board/prodrive/alpr/nand.c
@@ -93,19 +93,6 @@ static void alpr_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 	}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int alpr_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
-{
-	int i;
-
-	for (i = 0; i < len; i++)
-		if (buf[i] != readb(&(alpr_ndfc->data)))
-			return i;
-
-	return 0;
-}
-#endif
-
 static int alpr_nand_dev_ready(struct mtd_info *mtd)
 {
 	/*
@@ -130,9 +117,6 @@ int board_nand_init(struct nand_chip *nand)
 	nand->read_byte  = alpr_nand_read_byte;
 	nand->write_buf  = alpr_nand_write_buf;
 	nand->read_buf   = alpr_nand_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	nand->verify_buf = alpr_nand_verify_buf;
-#endif
 	nand->dev_ready  = alpr_nand_dev_ready;
 
 	return 0;
diff --git a/board/socrates/nand.c b/board/socrates/nand.c
index 7394478..15e6ea6 100644
--- a/board/socrates/nand.c
+++ b/board/socrates/nand.c
@@ -18,9 +18,6 @@ static void sc_nand_write_buf(struct mtd_info *mtd, const u_char *buf, int len);
 static u_char sc_nand_read_byte(struct mtd_info *mtd);
 static u16 sc_nand_read_word(struct mtd_info *mtd);
 static void sc_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len);
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len);
-#endif
 static int sc_nand_device_ready(struct mtd_info *mtdinfo);
 
 #define FPGA_NAND_CMD_MASK		(0x7 << 28)
@@ -102,25 +99,6 @@ static void sc_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 	}
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/**
- * sc_nand_verify_buf -  Verify chip data against buffer
- * @mtd:	MTD device structure
- * @buf:	buffer containing the data to compare
- * @len:	number of bytes to compare
- */
-static int sc_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
-{
-	int i;
-
-	for (i = 0; i < len; i++) {
-		if (buf[i] != sc_nand_read_byte(mtd));
-			return -EFAULT;
-	}
-	return 0;
-}
-#endif
-
 /**
  * sc_nand_device_ready - Check the NAND device is ready for next command.
  * @mtd:	MTD device structure
@@ -178,9 +156,6 @@ int board_nand_init(struct nand_chip *nand)
 	nand->read_word = sc_nand_read_word;
 	nand->write_buf = sc_nand_write_buf;
 	nand->read_buf = sc_nand_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	nand->verify_buf = sc_nand_verify_buf;
-#endif
 
 	return 0;
 }
diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
index 3372b64..e85832d 100644
--- a/drivers/mtd/nand/fsl_elbc_nand.c
+++ b/drivers/mtd/nand/fsl_elbc_nand.c
@@ -561,41 +561,6 @@ static void fsl_elbc_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 		       len, avail);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/*
- * Verify buffer against the FCM Controller Data Buffer
- */
-static int fsl_elbc_verify_buf(struct mtd_info *mtd,
-			       const u_char *buf, int len)
-{
-	struct nand_chip *chip = mtd->priv;
-	struct fsl_elbc_mtd *priv = chip->priv;
-	struct fsl_elbc_ctrl *ctrl = priv->ctrl;
-	int i;
-
-	if (len < 0) {
-		printf("write_buf of %d bytes", len);
-		return -EINVAL;
-	}
-
-	if ((unsigned int)len > ctrl->read_bytes - ctrl->index) {
-		printf("verify_buf beyond end of buffer "
-		       "(%d requested, %u available)\n",
-		       len, ctrl->read_bytes - ctrl->index);
-
-		ctrl->index = ctrl->read_bytes;
-		return -EINVAL;
-	}
-
-	for (i = 0; i < len; i++)
-		if (in_8(&ctrl->addr[ctrl->index + i]) != buf[i])
-			break;
-
-	ctrl->index += len;
-	return i == len && ctrl->status == LTESR_CC ? 0 : -EIO;
-}
-#endif
-
 /* This function is called after Program and Erase Operations to
  * check for success or failure.
  */
@@ -727,9 +692,6 @@ static int fsl_elbc_chip_init(int devnum, u8 *addr)
 	nand->read_byte = fsl_elbc_read_byte;
 	nand->write_buf = fsl_elbc_write_buf;
 	nand->read_buf = fsl_elbc_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	nand->verify_buf = fsl_elbc_verify_buf;
-#endif
 	nand->select_chip = fsl_elbc_select_chip;
 	nand->cmdfunc = fsl_elbc_cmdfunc;
 	nand->waitfunc = fsl_elbc_wait;
diff --git a/drivers/mtd/nand/fsl_ifc_nand.c b/drivers/mtd/nand/fsl_ifc_nand.c
index b283eae..7903eeb 100644
--- a/drivers/mtd/nand/fsl_ifc_nand.c
+++ b/drivers/mtd/nand/fsl_ifc_nand.c
@@ -683,41 +683,6 @@ static void fsl_ifc_read_buf(struct mtd_info *mtd, u8 *buf, int len)
 		       __func__, len, avail);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/*
- * Verify buffer against the IFC Controller Data Buffer
- */
-static int fsl_ifc_verify_buf(struct mtd_info *mtd,
-			       const u_char *buf, int len)
-{
-	struct nand_chip *chip = mtd->priv;
-	struct fsl_ifc_mtd *priv = chip->priv;
-	struct fsl_ifc_ctrl *ctrl = priv->ctrl;
-	int i;
-
-	if (len < 0) {
-		printf("%s of %d bytes", __func__, len);
-		return -EINVAL;
-	}
-
-	if ((unsigned int)len > ctrl->read_bytes - ctrl->index) {
-		printf("%s beyond end of buffer "
-		       "(%d requested, %u available)\n",
-		       __func__, len, ctrl->read_bytes - ctrl->index);
-
-		ctrl->index = ctrl->read_bytes;
-		return -EINVAL;
-	}
-
-	for (i = 0; i < len; i++)
-		if (in_8(&ctrl->addr[ctrl->index + i]) != buf[i])
-			break;
-
-	ctrl->index += len;
-	return i == len && ctrl->status == IFC_NAND_EVTER_STAT_OPC ? 0 : -EIO;
-}
-#endif
-
 /* This function is called after Program and Erase Operations to
  * check for success or failure.
  */
@@ -940,9 +905,6 @@ static int fsl_ifc_chip_init(int devnum, u8 *addr)
 
 	nand->write_buf = fsl_ifc_write_buf;
 	nand->read_buf = fsl_ifc_read_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	nand->verify_buf = fsl_ifc_verify_buf;
-#endif
 	nand->select_chip = fsl_ifc_select_chip;
 	nand->cmdfunc = fsl_ifc_cmdfunc;
 	nand->waitfunc = fsl_ifc_wait;
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index 65ce98a..da52522 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -153,21 +153,6 @@ static void upm_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 		buf[i] = in_8(chip->IO_ADDR_R);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int upm_nand_verify_buf(struct mtd_info *mtd, const u_char *buf, int len)
-{
-	int i;
-	struct nand_chip *chip = mtd->priv;
-
-	for (i = 0; i < len; i++) {
-		if (buf[i] != in_8(chip->IO_ADDR_R))
-			return -EFAULT;
-	}
-
-	return 0;
-}
-#endif
-
 static int nand_dev_ready(struct mtd_info *mtd)
 {
 	struct nand_chip *chip = mtd->priv;
@@ -193,9 +178,7 @@ int fsl_upm_nand_init(struct nand_chip *chip, struct fsl_upm_nand *fun)
 	chip->read_byte = upm_nand_read_byte;
 	chip->read_buf = upm_nand_read_buf;
 	chip->write_buf = upm_nand_write_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	chip->verify_buf = upm_nand_verify_buf;
-#endif
+
 	if (fun->dev_ready)
 		chip->dev_ready = nand_dev_ready;
 
diff --git a/drivers/mtd/nand/mpc5121_nfc.c b/drivers/mtd/nand/mpc5121_nfc.c
index 7233bfc..e621c36 100644
--- a/drivers/mtd/nand/mpc5121_nfc.c
+++ b/drivers/mtd/nand/mpc5121_nfc.c
@@ -459,29 +459,6 @@ static void mpc5121_nfc_write_buf(struct mtd_info *mtd,
 	mpc5121_nfc_buf_copy(mtd, (u_char *) buf, len, 1);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/* Compare buffer with NAND flash */
-static int mpc5121_nfc_verify_buf(struct mtd_info *mtd,
-				  const u_char * buf, int len)
-{
-	u_char tmp[256];
-	uint bsize;
-
-	while (len) {
-		bsize = min(len, 256);
-		mpc5121_nfc_read_buf(mtd, tmp, bsize);
-
-		if (memcmp(buf, tmp, bsize))
-			return 1;
-
-		buf += bsize;
-		len -= bsize;
-	}
-
-	return 0;
-}
-#endif
-
 /* Read byte from NFC buffers */
 static u8 mpc5121_nfc_read_byte(struct mtd_info *mtd)
 {
@@ -609,9 +586,6 @@ int board_nand_init(struct nand_chip *chip)
 	chip->read_word = mpc5121_nfc_read_word;
 	chip->read_buf = mpc5121_nfc_read_buf;
 	chip->write_buf = mpc5121_nfc_write_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	chip->verify_buf = mpc5121_nfc_verify_buf;
-#endif
 	chip->select_chip = mpc5121_nfc_select_chip;
 	chip->bbt_options = NAND_BBT_USE_FLASH;
 	chip->ecc.mode = NAND_ECC_SOFT;
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index 2e5b5b9..f12b07e 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -949,34 +949,6 @@ static void mxc_nand_read_buf(struct mtd_info *mtd, u_char *buf, int len)
 	host->col_addr = col;
 }
 
-#ifdef __UBOOT__
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/*
- * Used by the upper layer to verify the data in NAND Flash
- * with the data in the buf.
- */
-static int mxc_nand_verify_buf(struct mtd_info *mtd,
-				const u_char *buf, int len)
-{
-	u_char tmp[256];
-	uint bsize;
-
-	while (len) {
-		bsize = min(len, 256);
-		mxc_nand_read_buf(mtd, tmp, bsize);
-
-		if (memcmp(buf, tmp, bsize))
-			return 1;
-
-		buf += bsize;
-		len -= bsize;
-	}
-
-	return 0;
-}
-#endif
-#endif
-
 /*
  * This function is used by upper layer for select and
  * deselect of the NAND chip
@@ -1207,11 +1179,6 @@ int board_nand_init(struct nand_chip *this)
 	this->read_word = mxc_nand_read_word;
 	this->write_buf = mxc_nand_write_buf;
 	this->read_buf = mxc_nand_read_buf;
-#ifdef __UBOOT__
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	this->verify_buf = mxc_nand_verify_buf;
-#endif
-#endif
 
 	host->regs = (struct mxc_nand_regs __iomem *)CONFIG_MXC_NAND_REGS_BASE;
 #ifdef MXC_NFC_V3_2
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 788846a..abcb84a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -361,51 +361,6 @@ void nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
 	ioread8_rep(chip->IO_ADDR_R, buf, len);
 }
 
-#ifdef __UBOOT__
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-/**
- * nand_verify_buf - [DEFAULT] Verify chip data against buffer
- * @mtd: MTD device structure
- * @buf: buffer containing the data to compare
- * @len: number of bytes to compare
- *
- * Default verify function for 8bit buswidth.
- */
-static int nand_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int len)
-{
-	int i;
-	struct nand_chip *chip = mtd->priv;
-
-	for (i = 0; i < len; i++)
-		if (buf[i] != readb(chip->IO_ADDR_R))
-			return -EFAULT;
-	return 0;
-}
-
-/**
- * nand_verify_buf16 - [DEFAULT] Verify chip data against buffer
- * @mtd: MTD device structure
- * @buf: buffer containing the data to compare
- * @len: number of bytes to compare
- *
- * Default verify function for 16bit buswidth.
- */
-static int nand_verify_buf16(struct mtd_info *mtd, const uint8_t *buf, int len)
-{
-	int i;
-	struct nand_chip *chip = mtd->priv;
-	u16 *p = (u16 *) buf;
-	len >>= 1;
-
-	for (i = 0; i < len; i++)
-		if (p[i] != readw(chip->IO_ADDR_R))
-			return -EFAULT;
-
-	return 0;
-}
-#endif
-#endif
-
 /**
  * nand_write_buf16 - [DEFAULT] write buffer to chip
  * @mtd: MTD device structure
@@ -3154,12 +3109,6 @@ static void nand_set_defaults(struct nand_chip *chip, int busw)
 		chip->read_buf = busw ? nand_read_buf16 : nand_read_buf;
 	if (!chip->scan_bbt)
 		chip->scan_bbt = nand_default_bbt;
-#ifdef __UBOOT__
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	if (!chip->verify_buf)
-		chip->verify_buf = busw ? nand_verify_buf16 : nand_verify_buf;
-#endif
-#endif
 
 	if (!chip->controller) {
 		chip->controller = &chip->hwcontrol;
diff --git a/drivers/mtd/nand/ndfc.c b/drivers/mtd/nand/ndfc.c
index 2659595..3bbf12e 100644
--- a/drivers/mtd/nand/ndfc.c
+++ b/drivers/mtd/nand/ndfc.c
@@ -88,7 +88,7 @@ static int ndfc_calculate_ecc(struct mtd_info *mtdinfo,
 }
 
 /*
- * Speedups for buffer read/write/verify
+ * Speedups for buffer read/write
  *
  * NDFC allows 32bit read/write of data. So we can speed up the buffer
  * functions. No further checking, as nand_base will always read/write
@@ -118,21 +118,6 @@ static void ndfc_write_buf(struct mtd_info *mtdinfo, const uint8_t *buf, int len
 		out_be32((u32 *)(base + NDFC_DATA), *p++);
 }
 
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-static int ndfc_verify_buf(struct mtd_info *mtdinfo, const uint8_t *buf, int len)
-{
-	struct nand_chip *this = mtdinfo->priv;
-	ulong base = (ulong) this->IO_ADDR_W & 0xffffff00;
-	uint32_t *p = (uint32_t *) buf;
-
-	for (; len > 0; len -= 4)
-		if (*p++ != in_be32((u32 *)(base + NDFC_DATA)))
-			return -1;
-
-	return 0;
-}
-#endif
-
 /*
  * Read a byte from the NDFC.
  */
@@ -207,9 +192,6 @@ int board_nand_init(struct nand_chip *nand)
 #endif
 
 	nand->write_buf  = ndfc_write_buf;
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-	nand->verify_buf = ndfc_verify_buf;
-#endif
 	nand->read_byte = ndfc_read_byte;
 
 	chip++;
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 8438490..bc927ec 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -678,11 +678,6 @@ struct nand_chip {
 	void (*write_byte)(struct mtd_info *mtd, uint8_t byte);
 	void (*write_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
 	void (*read_buf)(struct mtd_info *mtd, uint8_t *buf, int len);
-#ifdef __UBOOT__
-#if defined(CONFIG_MTD_NAND_VERIFY_WRITE)
-        int (*verify_buf)(struct mtd_info *mtd, const uint8_t *buf, int len);
-#endif
-#endif
 	void (*select_chip)(struct mtd_info *mtd, int chip);
 	int (*block_bad)(struct mtd_info *mtd, loff_t ofs, int getchip);
 	int (*block_markbad)(struct mtd_info *mtd, loff_t ofs);
-- 
1.9.1

[U-Boot] [PATCH 4/5] mtd: nand: Use ECC for NAND write verification

[Peter Tyser]

From: Joe Schaack <jschaack@xes-inc.com>

Modify the nand_write_page() function to use ECC when appropriate to verify
writes.  Previously if a single bit error occured and software ECC was
used the write verification would report a failure.  However, the write
really did succeed, since ECC can handle the error.

The issue can be simulated with a sequence of commands such as:

   # Inject a fake bit that is stuck low (2K page flash being used)
   nand erase 0 0x20000
   mw.b 0x10000 0xff 0x1000
   mw.b 0x10000 0xfe 1
   nand write.raw 0x10000 0x0 0x1

   # Write some data which needs to toggle the fake stuck bit
   mw.b 0x10000 0xab 1
   nand write 0x10000 0x0 0x800

An error will occur:
    NAND write: device 0 offset 0x0, size 0x800
    NAND write to offset 0 failed -5
    0 bytes written: ERROR

But you can verify data was correctly written:
    # Show data is correct when ECC is used
    nand read 0x10000 0x0 0x800
    md 0x10000

    # Show the bit is still stuck low
    nand read.raw 0x10000 0x0 0x1
    md 0x10000

Change the behavior so ECC is used when verifying non-raw writes.
This only impacts boards that have COFNIG_MTD_NAND_VERIFY_WRITE
defined.

Signed-off-by: Joe Schaack <jschaack@xes-inc.com>
Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---

 drivers/mtd/nand/nand_base.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index abcb84a..aa039ef 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2405,7 +2405,10 @@ static int nand_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	/* Send command to read back the data */
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+	if (unlikely(raw))
+		chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+	else
+		chip->ecc.read_page(mtd, chip, vfy_buf, oob_required, page);
 
 	status = memcmp(buf, vfy_buf, mtd->writesize);
 
-- 
1.9.1

[U-Boot] [PATCH 5/5] mtd: davinci nand: Use ECC for NAND write verification

[Peter Tyser]

From: Joe Schaack <jschaack@xes-inc.com>

Modify the nand_davinci_write_page() function to use ECC when appropriate
to verify writes.  Previously if a single bit error occured and software
ECC was used the write verification would report a failure.  However,
the write really did succeed, since ECC can handle the error.

Signed-off-by: Joe Schaack <jschaack@xes-inc.com>
Signed-off-by: Peter Tyser <ptyser@xes-inc.com>
---
I don't have davinci hardware to test on, but the change should
mirror the nand_base.c change.

 drivers/mtd/nand/davinci_nand.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index b4e22d1..cd0b07c 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -415,7 +415,10 @@ static int nand_davinci_write_page(struct mtd_info *mtd, struct nand_chip *chip,
 
 	/* Send command to read back the data */
 	chip->cmdfunc(mtd, NAND_CMD_READ0, 0, page);
-	chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+	if (unlikely(raw))
+		chip->ecc.read_page_raw(mtd, chip, vfy_buf, oob_required, page);
+	else
+		chip->ecc.read_page(mtd, chip, vfy_buf, oob_required, page);
 
 	if (memcmp(buf, vfy_buf, mtd->writesize))
 		ret = -EIO;
-- 
1.9.1

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Scott Wood]

On Mon, 2015-01-26 at 16:24 -0600, Peter Tyser wrote:
> The driver-specific verify_buf() function can be replaced with the
> standard read_page_raw() function to verify writes.  This will allow
> verify_buf() to be removed from individual drivers.  verify_buf() is no
> longer supported in mainline Linux, so it is a pain to continue
> supporting.

Any reason not to just remove this feature from U-Boot, as Linux has
done?

-Scott

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Peter Tyser]

On Mon, 2015-01-26 at 16:33 -0600, Scott Wood wrote:
> On Mon, 2015-01-26 at 16:24 -0600, Peter Tyser wrote:
> > The driver-specific verify_buf() function can be replaced with the
> > standard read_page_raw() function to verify writes.  This will 
> > allow
> > verify_buf() to be removed from individual drivers.  verify_buf() 
> > is no
> > longer supported in mainline Linux, so it is a pain to continue
> > supporting.
> 
> Any reason not to just remove this feature from U-Boot, as Linux has
> done?

The Linux change for reference: 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=657f28f8811c92724db10d18bbbec70d540147d6

I waffled about removing it, but leaned towards leaving it in because:
- I didn't want to change the existing U-Boot behavior for other 
users.  A google of 'u-boot "nand write"' shows a lot of examples that 
don't include verification of writes, and they should if we remove 
auto-verification.

- The reason it was removed in Linux was "Both UBI and JFFS2 are able 
to read verify what they wrote already.  There are also MTD tests 
which do this verification."  I thought U-Boot was more likely than 
Linux to use raw NAND writes without a filesystem, so leaving it in U-
Boot made sense since the UBI/JFFS2 logic didn't apply as much here.



- I didn't think a lot of people would know they have to explicitly 
verify NAND contents after a write, since they'd assume it was like 
other memories that aren't as lossy.

- The penalty of slightly different code from Linux and a small 
performance hit was worth the gain of auto-verification to me.  I 
viewed consolidating it into one small chunk of code as a happy medium.


The philosophical side of me said to remove it altogether, so I can 
see that perspective too.
Peter

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Scott Wood]

On Mon, 2015-01-26 at 17:17 -0600, Peter Tyser wrote:
> On Mon, 2015-01-26 at 16:33 -0600, Scott Wood wrote:
> > On Mon, 2015-01-26 at 16:24 -0600, Peter Tyser wrote:
> > > The driver-specific verify_buf() function can be replaced with the
> > > standard read_page_raw() function to verify writes.  This will 
> > > allow
> > > verify_buf() to be removed from individual drivers.  verify_buf() 
> > > is no
> > > longer supported in mainline Linux, so it is a pain to continue
> > > supporting.
> > 
> > Any reason not to just remove this feature from U-Boot, as Linux has
> > done?
> 
> The Linux change for reference: 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=657f28f8811c92724db10d18bbbec70d540147d6
> 
> I waffled about removing it, but leaned towards leaving it in because:
> - I didn't want to change the existing U-Boot behavior for other 
> users.  A google of 'u-boot "nand write"' shows a lot of examples that 
> don't include verification of writes, and they should if we remove 
> auto-verification.

How many configs actually enable this option?  I don't see many beyond
the FSL PPC boards (which are so full of copy-and-paste that it probably
wasn't deliberate).

> - The reason it was removed in Linux was "Both UBI and JFFS2 are able 
> to read verify what they wrote already.  There are also MTD tests 
> which do this verification."  I thought U-Boot was more likely than 
> Linux to use raw NAND writes without a filesystem, so leaving it in U-
> Boot made sense since the UBI/JFFS2 logic didn't apply as much here.

Right, though raw writes ought to be limited to blocks that aren't
written often enough to fail.

> - I didn't think a lot of people would know they have to explicitly 
> verify NAND contents after a write, since they'd assume it was like 
> other memories that aren't as lossy.
> 
> - The penalty of slightly different code from Linux and a small 
> performance hit was worth the gain of auto-verification to me.  I 
> viewed consolidating it into one small chunk of code as a happy medium.

The davinci patches show that there can still be driver dependencies
depending on what the driver overrides.  I'm not hugely opposed, but it
seems like it would be better to do it at a higher level (e.g. in
nand_util.c with a flag to enable, and either make support mandatory, or
if you try to use that command variant without support it fails rather
than silently not verifying).

-Scott

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Peter Tyser]

Hi Scott,


> > I waffled about removing it, but leaned towards leaving it in because:
> > - I didn't want to change the existing U-Boot behavior for other
> > users.  A google of 'u-boot "nand write"' shows a lot of examples that
> > don't include verification of writes, and they should if we remove
> > auto-verification.
> 
> How many configs actually enable this option?  I don't see many beyond
> the FSL PPC boards (which are so full of copy-and-paste that it probably
> wasn't deliberate).

Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.

> > - The reason it was removed in Linux was "Both UBI and JFFS2 are able
> > to read verify what they wrote already.  There are also MTD tests
> > which do this verification."  I thought U-Boot was more likely than
> > Linux to use raw NAND writes without a filesystem, so leaving it in U-
> > Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
> 
> Right, though raw writes ought to be limited to blocks that aren't
> written often enough to fail.
> 
> > - I didn't think a lot of people would know they have to explicitly
> > verify NAND contents after a write, since they'd assume it was like
> > other memories that aren't as lossy.
> > 
> > - The penalty of slightly different code from Linux and a small
> > performance hit was worth the gain of auto-verification to me.  I
> > viewed consolidating it into one small chunk of code as a happy medium.
> 
> The davinci patches show that there can still be driver dependencies
> depending on what the driver overrides.  I'm not hugely opposed, but it
> seems like it would be better to do it at a higher level (e.g. in
> nand_util.c with a flag to enable, and either make support mandatory, or
> if you try to use that command variant without support it fails rather
> than silently not verifying).

That seems like a good idea.  How about:
- Remove all CONFIG_MTD_NAND_VERIFY_WRITE references

- Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in 
nand_util.c verify writes only when it is set.

- Update the calls to nand_write_skip_bad() in cmd_nand.c to include
the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
but let me know if you disagree.

That would make all "nand write" commands verify writes, with the
exception of "nand write.raw".  Any opinion on if this should also
be verified?  I only use it for development/testing, so don't have
a strong opinion.

Regards,
Peter

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Scott Wood]

On Tue, 2015-01-27 at 17:47 -0600, Peter Tyser wrote:
> Hi Scott,
> 
> 
> > > I waffled about removing it, but leaned towards leaving it in because:
> > > - I didn't want to change the existing U-Boot behavior for other
> > > users.  A google of 'u-boot "nand write"' shows a lot of examples that
> > > don't include verification of writes, and they should if we remove
> > > auto-verification.
> > 
> > How many configs actually enable this option?  I don't see many beyond
> > the FSL PPC boards (which are so full of copy-and-paste that it probably
> > wasn't deliberate).
> 
> Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.
> 
> > > - The reason it was removed in Linux was "Both UBI and JFFS2 are able
> > > to read verify what they wrote already.  There are also MTD tests
> > > which do this verification."  I thought U-Boot was more likely than
> > > Linux to use raw NAND writes without a filesystem, so leaving it in U-
> > > Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
> > 
> > Right, though raw writes ought to be limited to blocks that aren't
> > written often enough to fail.
> > 
> > > - I didn't think a lot of people would know they have to explicitly
> > > verify NAND contents after a write, since they'd assume it was like
> > > other memories that aren't as lossy.
> > > 
> > > - The penalty of slightly different code from Linux and a small
> > > performance hit was worth the gain of auto-verification to me.  I
> > > viewed consolidating it into one small chunk of code as a happy medium.
> > 
> > The davinci patches show that there can still be driver dependencies
> > depending on what the driver overrides.  I'm not hugely opposed, but it
> > seems like it would be better to do it at a higher level (e.g. in
> > nand_util.c with a flag to enable, and either make support mandatory, or
> > if you try to use that command variant without support it fails rather
> > than silently not verifying).
> 
> That seems like a good idea.  How about:
> - Remove all CONFIG_MTD_NAND_VERIFY_WRITE references
> 
> - Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in 
> nand_util.c verify writes only when it is set.
> 
> - Update the calls to nand_write_skip_bad() in cmd_nand.c to include
> the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
> but let me know if you disagree.
> 
> That would make all "nand write" commands verify writes, with the
> exception of "nand write.raw".  Any opinion on if this should also
> be verified?  I only use it for development/testing, so don't have
> a strong opinion.

"raw" refers to the absence of ECC, and I'd rather not overload it to
mean "don't verify".  Should it also be possible to request non-raw
non-verified accesses?  Or should we always verify and wait until
someone complains about performance?

What about DFU and other non-cmd_nand NAND accesses?

-Scott

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Peter Tyser]

On Thu, 2015-01-29 at 17:02 -0600, Scott Wood wrote:
> On Tue, 2015-01-27 at 17:47 -0600, Peter Tyser wrote:
> > Hi Scott,
> > 
> > 
> > > > I waffled about removing it, but leaned towards leaving it in because:
> > > > - I didn't want to change the existing U-Boot behavior for other
> > > > users.  A google of 'u-boot "nand write"' shows a lot of examples that
> > > > don't include verification of writes, and they should if we remove
> > > > auto-verification.
> > > 
> > > How many configs actually enable this option?  I don't see many beyond
> > > the FSL PPC boards (which are so full of copy-and-paste that it probably
> > > wasn't deliberate).
> > 
> > Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.
> > 
> > > > - The reason it was removed in Linux was "Both UBI and JFFS2 are able
> > > > to read verify what they wrote already.  There are also MTD tests
> > > > which do this verification."  I thought U-Boot was more likely than
> > > > Linux to use raw NAND writes without a filesystem, so leaving it in U-
> > > > Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
> > > 
> > > Right, though raw writes ought to be limited to blocks that aren't
> > > written often enough to fail.
> > > 
> > > > - I didn't think a lot of people would know they have to explicitly
> > > > verify NAND contents after a write, since they'd assume it was like
> > > > other memories that aren't as lossy.
> > > > 
> > > > - The penalty of slightly different code from Linux and a small
> > > > performance hit was worth the gain of auto-verification to me.  I
> > > > viewed consolidating it into one small chunk of code as a happy medium.
> > > 
> > > The davinci patches show that there can still be driver dependencies
> > > depending on what the driver overrides.  I'm not hugely opposed, but it
> > > seems like it would be better to do it at a higher level (e.g. in
> > > nand_util.c with a flag to enable, and either make support mandatory, or
> > > if you try to use that command variant without support it fails rather
> > > than silently not verifying).
> > 
> > That seems like a good idea.  How about:
> > - Remove all CONFIG_MTD_NAND_VERIFY_WRITE references
> > 
> > - Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in
> > nand_util.c verify writes only when it is set.
> > 
> > - Update the calls to nand_write_skip_bad() in cmd_nand.c to include
> > the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
> > but let me know if you disagree.
> > 
> > That would make all "nand write" commands verify writes, with the
> > exception of "nand write.raw".  Any opinion on if this should also
> > be verified?  I only use it for development/testing, so don't have
> > a strong opinion.
> 
> "raw" refers to the absence of ECC, and I'd rather not overload it to
> mean "don't verify".  Should it also be possible to request non-raw
> non-verified accesses?  Or should we always verify and wait until
> someone complains about performance?

OK, I'll add verification to the "nand write.raw" functionality too.
I'd lean towards always verifying and waiting until/if someone
complains about performance.  I doubt many (any?) people are doing
timing critical writes in U-Boot.  I think the argument that writes
should be verified carries some water, and it'd be nice to not
make the command arguments more complicated than they already are.

> What about DFU and other non-cmd_nand NAND accesses?

Are there other non-cmd NAND accesses other than DFU?  None jumped
out at me.  I'm not too familiar with DFU, but in theory the DFU
programming utilities could already be doing their own verification.
I took a quick look at the dfu-util source, and it doesn't appear
to be doing its own.  I'd vote to verify the DFU writes too, since
even more than "nand write" its performance shouldn't be very
critical.  I'll break DFU verification out into a separate patch,
and you or others can ACK or reject it then.

Let me know if the above sounds good and I'll make the changes.

Thanks,
Peter

[U-Boot] [PATCH 1/5] nand: Use common read function instead of verify_buf()

[Scott Wood]

On Thu, 2015-01-29 at 17:37 -0600, Peter Tyser wrote:
> On Thu, 2015-01-29 at 17:02 -0600, Scott Wood wrote:
> > On Tue, 2015-01-27 at 17:47 -0600, Peter Tyser wrote:
> > > Hi Scott,
> > > 
> > > 
> > > > > I waffled about removing it, but leaned towards leaving it in because:
> > > > > - I didn't want to change the existing U-Boot behavior for other
> > > > > users.  A google of 'u-boot "nand write"' shows a lot of examples that
> > > > > don't include verification of writes, and they should if we remove
> > > > > auto-verification.
> > > > 
> > > > How many configs actually enable this option?  I don't see many beyond
> > > > the FSL PPC boards (which are so full of copy-and-paste that it probably
> > > > wasn't deliberate).
> > > 
> > > Yeah, the majority are FSL 83xx and 85xx, with 2 or so random ARM boards.
> > > 
> > > > > - The reason it was removed in Linux was "Both UBI and JFFS2 are able
> > > > > to read verify what they wrote already.  There are also MTD tests
> > > > > which do this verification."  I thought U-Boot was more likely than
> > > > > Linux to use raw NAND writes without a filesystem, so leaving it in U-
> > > > > Boot made sense since the UBI/JFFS2 logic didn't apply as much here.
> > > > 
> > > > Right, though raw writes ought to be limited to blocks that aren't
> > > > written often enough to fail.
> > > > 
> > > > > - I didn't think a lot of people would know they have to explicitly
> > > > > verify NAND contents after a write, since they'd assume it was like
> > > > > other memories that aren't as lossy.
> > > > > 
> > > > > - The penalty of slightly different code from Linux and a small
> > > > > performance hit was worth the gain of auto-verification to me.  I
> > > > > viewed consolidating it into one small chunk of code as a happy medium.
> > > > 
> > > > The davinci patches show that there can still be driver dependencies
> > > > depending on what the driver overrides.  I'm not hugely opposed, but it
> > > > seems like it would be better to do it at a higher level (e.g. in
> > > > nand_util.c with a flag to enable, and either make support mandatory, or
> > > > if you try to use that command variant without support it fails rather
> > > > than silently not verifying).
> > > 
> > > That seems like a good idea.  How about:
> > > - Remove all CONFIG_MTD_NAND_VERIFY_WRITE references
> > > 
> > > - Add a new flag WITH_WR_VERIFY and have nand_write_skip_bad() in
> > > nand_util.c verify writes only when it is set.
> > > 
> > > - Update the calls to nand_write_skip_bad() in cmd_nand.c to include
> > > the new WITH_WR_VERIFY flag.  I'd vote to enable it for all boards,
> > > but let me know if you disagree.
> > > 
> > > That would make all "nand write" commands verify writes, with the
> > > exception of "nand write.raw".  Any opinion on if this should also
> > > be verified?  I only use it for development/testing, so don't have
> > > a strong opinion.
> > 
> > "raw" refers to the absence of ECC, and I'd rather not overload it to
> > mean "don't verify".  Should it also be possible to request non-raw
> > non-verified accesses?  Or should we always verify and wait until
> > someone complains about performance?
> 
> OK, I'll add verification to the "nand write.raw" functionality too.
> I'd lean towards always verifying and waiting until/if someone
> complains about performance.  I doubt many (any?) people are doing
> timing critical writes in U-Boot.  I think the argument that writes
> should be verified carries some water, and it'd be nice to not
> make the command arguments more complicated than they already are.
> 
> > What about DFU and other non-cmd_nand NAND accesses?
> 
> Are there other non-cmd NAND accesses other than DFU?  None jumped
> out at me.

There's ubi/jffs2/yaffs2, which are responsible for their own
verification, and a read-only access in compulab board code, but
otherwise maybe not.  The MTD interface is harder to grep for, though.

>   I'm not too familiar with DFU, but in theory the DFU
> programming utilities could already be doing their own verification.
> I took a quick look at the dfu-util source, and it doesn't appear
> to be doing its own.  I'd vote to verify the DFU writes too, since
> even more than "nand write" its performance shouldn't be very
> critical.  I'll break DFU verification out into a separate patch,
> and you or others can ACK or reject it then.
> 
> Let me know if the above sounds good and I'll make the changes.

Fine with me.

-Scott