mtd: nand: localize ECC failures per page

Signed-off-by Brian Norris <[email protected]>

---
No changes since v1

 include/linux/mtd/nand.h | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

include/linux/mtd/nand.h +22 -1

index f3ea8daf08ee..029fe5948dc4 100644
@@ -285,7 +285,8 @@ struct nand_onfi_params {
 	u8 reserved4[7];
 
 	/* vendor */
-	u8 reserved5[90];
+	__le16 vendor_revision;
+	u8 vendor[88];
 
 	__le16 crc;
 } __attribute__((packed));

@@ -326,6 +327,26 @@ struct onfi_ext_param_page {
 	 */
 } __packed;
 
+struct nand_onfi_vendor_micron {
+	u8 two_plane_read;
+	u8 read_cache;
+	u8 read_unique_id;
+	u8 dq_imped;
+	u8 dq_imped_num_settings;
+	u8 dq_imped_feat_addr;
+	u8 rb_pulldown_strength;
+	u8 rb_pulldown_strength_feat_addr;
+	u8 rb_pulldown_strength_num_settings;
+	u8 otp_mode;
+	u8 otp_page_start;
+	u8 otp_data_prot_addr;
+	u8 otp_num_pages;
+	u8 otp_feat_addr;
+	u8 read_retry_options;
+	u8 reserved[72];
+	u8 param_revision;
+} __packed;
+
 /**
  * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independent devices
  * @lock:               protection lock

Signature · 1 line

1.8.3.2

drivers/mtd/nand/nand_base.c +2 -2

+/**
  * nand_do_read_ops - [INTERN] Read data with ECC
  * @mtd: MTD device structure
  * @from: offset to read from

@@ -1431,6 +1453,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	uint8_t *bufpoi, *oob, *buf;
 	unsigned int max_bitflips = 0;
 
+	int retry_mode = 0;
 	bool ecc_fail = false;
 
 	chipnr = (int)(from >> chip->chip_shift);

@@ -1494,8 +1517,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 				memcpy(buf, chip->buffers->databuf + col, bytes);
 			}
 
-			buf += bytes;
-
 			if (unlikely(oob)) {
 				int toread = min(oobreadlen, max_oobsize);
 

drivers/mtd/nand/nand_base.c +24 -0

+
+					ret = nand_set_read_retry(mtd,
+							retry_mode);
+					if (ret < 0)
+						break;
+
+					/* Reset failures */
+					mtd->ecc_stats.failed = ecc_failures;
+					continue;
+				} else {
+					/* No more retry modes; real failure */
+					ecc_fail = true;
+				}
+			}
+
+			buf += bytes;
 		} else {
 			memcpy(buf, chip->buffers->databuf + col, bytes);
 			buf += bytes;

@@ -1525,6 +1565,14 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 
 		readlen -= bytes;
 
+		/* Reset to retry mode 0 */
+		if (retry_mode) {
+			ret = nand_set_read_retry(mtd, 0);
+			if (ret < 0)
+				break;
+			retry_mode = 0;
+		}
+
 		if (!readlen)
 			break;
 

include/linux/mtd/nand.h +0 -0

index 029fe5948dc4..ef70505dade1 100644

Quoted 4 lines · Brian Norris

drivers/mtd/nand/nand_base.c +0 -0

@@ -1514,8 +1535,27 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 					nand_wait_ready(mtd);
 			}
 

drivers/mtd/nand/nand_base.c static int nand_do_read_ops(struct mtd_info *mtd, loff_t from, · line 1542

Quoted 7 lines

drivers/mtd/nand/nand_base.c +5 -2

-			if (mtd->ecc_stats.failed - ecc_failures)
-				ecc_fail = true;
+			if (mtd->ecc_stats.failed - ecc_failures) {
+				if (retry_mode + 1 <= chip->read_retries) {
+					retry_mode++;
+					pr_debug("ECC error; performing READ RETRY %d\n",
+							retry_mode);

you can move this pr_debug into the nand_set_read_retry().

Quoted 10 lines

drivers/mtd/nand/nand_base.c +5 -0

+
+					ret = nand_set_read_retry(mtd,
+							retry_mode);
+					if (ret < 0)
+						break;

include/linux/mtd/nand.h +0 -0

index 029fe5948dc4..ef70505dade1 100644
@@ -472,6 +472,8 @@ struct nand_buffers {

include/linux/mtd/nand.h struct nand_buffers { · line 476

Quoted 5 lines

include/linux/mtd/nand.h +2 -0

  *			commands to the chip.
  * @waitfunc:		[REPLACEABLE] hardwarespecific function for wait on
  *			ready.
+ * @set_read_retry:	[FLASHSPECIFIC] flash (vendor) specific function for
+ *			setting the read-retry mode. Mostly needed for MLC NAND.

why not use the name "read_retry"?
i think it is more clear.

It's not actually performing any "read" or a "retry"; the function is just configuring the device for a retry. So I don't think read_retry is correct. For my name, I was abbreviating "set read retry mode". How do you like "setup_read_retry" or "set_read_retry_mode"?

Modern MLC (and even SLC?) NAND can experience a large number of bitflips (beyond the recommended correctability capacity) due to drifts in the voltage threshold (Vt). These bitflips can cause ECC errors to occur well within the expected lifetime of the flash. To account for this, some manufacturers provide a mechanism for shifting the Vt threshold after a corrupted read.

The generic pattern seems to be that a particular flash has N read retry modes (where N = 0, traditionally), and after an ECC failure, the host should reconfigure the flash to use the next available mode, then retry the read operation. This process repeats until all bitfips can be corrected or until the host has tried all available retry modes.

This patch adds the infrastructure support for a vendor-specific/flash-specific callback, used for setting the read-retry mode (i.e., voltage threshold).

For now, this patch always returns the flash to mode 0 (the default mode) after a successful read-retry, according to the flowchart found in Micron's datasheets. This may need to change in the future if it is determined that eventually, mode 0 is insufficient for the majority of the flash cells (and so for performance reasons, we should leave the flash in mode 1, 2, etc.).

Signed-off-by Brian Norris <[email protected]>

---
v1 -> v2: fix a logic error when incrementing retry_mode, which caused -EINVAL
failures on flash that didn't need READ RETRY

v2 -> v3: split out the generic callback support as a separate patch; adjust #
of retry modes bounds check

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

drivers/mtd/nand/nand_base.c +27 -4

index e85b07f4293d..d47c5bbca2b3 100644
@@ -1410,6 +1410,28 @@ static uint8_t *nand_transfer_oob(struct nand_chip *chip, uint8_t *oob,
 }
 
 /**
+ * nand_set_read_retry - [INTERN] Set the READ RETRY mode
+ * @mtd: MTD device structure
+ * @retry_mode: the retry mode to use
+ *
+ * Some vendors supply a special command to shift the Vt threshold, to be used
+ * when there are too many bitflips in a page (i.e., ECC error). After setting
+ * a new threshold, the host should retry reading the page.
+ */
+static int nand_set_read_retry(struct mtd_info *mtd, int retry_mode)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	if (retry_mode >= chip->read_retries)
+		return -EINVAL;
+
+	if (!chip->set_read_retry)
+		return -EOPNOTSUPP;
+
+	return chip->set_read_retry(mtd, retry_mode);
+}
+
+/**
  * nand_do_read_ops - [INTERN] Read data with ECC
  * @mtd: MTD device structure
  * @from: offset to read from

@@ -1431,6 +1453,7 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 	uint8_t *bufpoi, *oob, *buf;
 	unsigned int max_bitflips = 0;
 
+	int retry_mode = 0;
 	bool ecc_fail = false;
 
 	chipnr = (int)(from >> chip->chip_shift);

@@ -1494,8 +1517,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 				memcpy(buf, chip->buffers->databuf + col, bytes);
 			}
 
-			buf += bytes;
-
 			if (unlikely(oob)) {
 				int toread = min(oobreadlen, max_oobsize);
 

@@ -1514,8 +1535,27 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 					nand_wait_ready(mtd);
 			}
 
-			if (mtd->ecc_stats.failed - ecc_failures)
-				ecc_fail = true;
+			if (mtd->ecc_stats.failed - ecc_failures) {
+				if (retry_mode + 1 <= chip->read_retries) {
+					retry_mode++;
+					pr_debug("ECC error; performing READ RETRY %d\n",

Note that I intentionally *haven't* accomodated a DMA-able buffer for the ONFI set-features command, so GPMI NAND may have problems with this.

drivers/mtd/nand/nand_base.c +12 -0

+static int nand_set_read_retry_micron(struct mtd_info *mtd, int retry_mode)
+{
+	struct nand_chip *chip = mtd->priv;
+	uint8_t feature[ONFI_SUBFEATURE_PARAM_LEN] = {retry_mode};
+
+	return chip->onfi_set_features(mtd, chip, ONFI_FEATURE_ADDR_READ_RETRY,
+			feature);
+}
+
+/*
+ * Configure chip properties from Micron vendor-specific ONFI table
+ */

To be consistent with the rest of include/linux/mtd/nand.h, we should use the __packed shorthand instead of __attribute__((packed)).

Signed-off-by Brian Norris <[email protected]>

---
No change since v1

 include/linux/mtd/nand.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/linux/mtd/nand.h +1 -1

index e3621aba1417..2413bac095ae 100644
@@ -292,7 +292,7 @@ struct nand_onfi_params {
 	u8 vendor[88];
 
 	__le16 crc;
-} __attribute__((packed));
+} __packed;
 
 #define ONFI_CRC_BASE	0x4F4E
 

Signature · 1 line

1.8.3.2

ECC failures can be tracked at the page level, not the do_read_ops level (i.e., a potentially multi-page transaction).

This helps prepare for READ RETRY support.

Signed-off-by Brian Norris <[email protected]>

---
No changes since v1

 drivers/mtd/nand/nand_base.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

drivers/mtd/nand/nand_base.c +0 -1

index 9b3bb3c519e9..e85b07f4293d 100644
@@ -1422,7 +1422,6 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 {
 	int chipnr, page, realpage, col, bytes, aligned, oob_required;
 	struct nand_chip *chip = mtd->priv;
-	struct mtd_ecc_stats stats;
 	int ret = 0;
 	uint32_t readlen = ops->len;
 	uint32_t oobreadlen = ops->ooblen;