All of lore.kernel.org
 help / color / mirror / Atom feed
From: Abhishek Sahu <absahu@codeaurora.org>
To: Boris Brezillon <boris.brezillon@bootlin.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Archit Taneja <architt@codeaurora.org>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	Marek Vasut <marek.vasut@gmail.com>,
	Abhishek Sahu <absahu@codeaurora.org>,
	linux-mtd@lists.infradead.org,
	Richard Weinberger <richard@nod.at>,
	Andy Gross <andy.gross@linaro.org>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>
Subject: [PATCH v4 07/15] mtd: rawnand: qcom: erased page detection for uncorrectable errors only
Date: Wed, 20 Jun 2018 12:57:34 +0530	[thread overview]
Message-ID: <1529479662-4026-8-git-send-email-absahu@codeaurora.org> (raw)
In-Reply-To: <1529479662-4026-1-git-send-email-absahu@codeaurora.org>

Following is the flow in the HW if controller tries to read erased
page:

1. First ECC uncorrectable error will be generated from ECC engine
   since ECC engine first calculates the ECC with all 0xff and match
   the calculated ECC with ECC code in OOB (which is again all 0xff).
2. After getting ECC error, erased CW detection logic will be
   applied which is different for BCH and RS ECC
    a. For BCH, HW checks if all the bytes in page are 0xff and then
       it updates the status in separate register
       NAND_ERASED_CW_DETECT_STATUS.
    b. For RS ECC, the HW reports the same error when reading an
       erased CW, but it notifies that it is an erased CW by
       placing special characters at certain offsets in the
       buffer.

So the erased CW detect status should be checked only if ECC engine
generated the uncorrectable error.

Currently for all other operational errors also (like TIMEOUT, MPU
errors, etc.), the erased CW detect logic is being applied so fix this
and return EIO for other operational errors.

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
---
* Changes from v3:
  NONE

* Changes from v2:
  1. Changed commit message slightly

* Changes from v1:
  1. Added more detail in commit message
  2. Added comment before each if/else
  3. Removed redundant check for BS_UNCORRECTABLE_BIT

 drivers/mtd/nand/raw/qcom_nandc.c | 65 ++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index fc20149..0d931d5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1576,6 +1576,7 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	unsigned int max_bitflips = 0;
 	struct read_stats *buf;
+	bool flash_op_err = false;
 	int i;
 
 	buf = (struct read_stats *)nandc->reg_read_buf;
@@ -1597,8 +1598,18 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 		buffer = le32_to_cpu(buf->buffer);
 		erased_cw = le32_to_cpu(buf->erased_cw);
 
-		if (flash & (FS_OP_ERR | FS_MPU_ERR)) {
+		/*
+		 * Check ECC failure for each codeword. ECC failure can
+		 * happen in either of the following conditions
+		 * 1. If number of bitflips are greater than ECC engine
+		 *    capability.
+		 * 2. If this codeword contains all 0xff for which erased
+		 *    codeword detection check will be done.
+		 */
+		if ((flash & FS_OP_ERR) && (buffer & BS_UNCORRECTABLE_BIT)) {
 			bool erased;
+			int ret, ecclen, extraooblen;
+			void *eccbuf;
 
 			/* ignore erased codeword errors */
 			if (host->bch_enabled) {
@@ -1616,29 +1627,36 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 				continue;
 			}
 
-			if (buffer & BS_UNCORRECTABLE_BIT) {
-				int ret, ecclen, extraooblen;
-				void *eccbuf;
+			eccbuf = oob_buf ? oob_buf + oob_len : NULL;
+			ecclen = oob_buf ? host->ecc_bytes_hw : 0;
+			extraooblen = oob_buf ? oob_len : 0;
 
-				eccbuf = oob_buf ? oob_buf + oob_len : NULL;
-				ecclen = oob_buf ? host->ecc_bytes_hw : 0;
-				extraooblen = oob_buf ? oob_len : 0;
-
-				/*
-				 * make sure it isn't an erased page reported
-				 * as not-erased by HW because of a few bitflips
-				 */
-				ret = nand_check_erased_ecc_chunk(data_buf,
-					data_len, eccbuf, ecclen, oob_buf,
-					extraooblen, ecc->strength);
-				if (ret < 0) {
-					mtd->ecc_stats.failed++;
-				} else {
-					mtd->ecc_stats.corrected += ret;
-					max_bitflips =
-						max_t(unsigned int, max_bitflips, ret);
-				}
+			/*
+			 * make sure it isn't an erased page reported
+			 * as not-erased by HW because of a few bitflips
+			 */
+			ret = nand_check_erased_ecc_chunk(data_buf,
+				data_len, eccbuf, ecclen, oob_buf,
+				extraooblen, ecc->strength);
+			if (ret < 0) {
+				mtd->ecc_stats.failed++;
+			} else {
+				mtd->ecc_stats.corrected += ret;
+				max_bitflips =
+					max_t(unsigned int, max_bitflips, ret);
 			}
+		/*
+		 * Check if MPU or any other operational error (timeout,
+		 * device failure, etc.) happened for this codeword and
+		 * make flash_op_err true. If flash_op_err is set, then
+		 * EIO will be returned for page read.
+		 */
+		} else if (flash & (FS_OP_ERR | FS_MPU_ERR)) {
+			flash_op_err = true;
+		/*
+		 * No ECC or operational errors happened. Check the number of
+		 * bits corrected and update the ecc_stats.corrected.
+		 */
 		} else {
 			unsigned int stat;
 
@@ -1652,6 +1670,9 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 			oob_buf += oob_len + ecc->bytes;
 	}
 
+	if (flash_op_err)
+		return -EIO;
+
 	return max_bitflips;
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Abhishek Sahu <absahu@codeaurora.org>
To: Boris Brezillon <boris.brezillon@bootlin.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Richard Weinberger <richard@nod.at>,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mtd@lists.infradead.org, Andy Gross <andy.gross@linaro.org>,
	Archit Taneja <architt@codeaurora.org>,
	Abhishek Sahu <absahu@codeaurora.org>
Subject: [PATCH v4 07/15] mtd: rawnand: qcom: erased page detection for uncorrectable errors only
Date: Wed, 20 Jun 2018 12:57:34 +0530	[thread overview]
Message-ID: <1529479662-4026-8-git-send-email-absahu@codeaurora.org> (raw)
In-Reply-To: <1529479662-4026-1-git-send-email-absahu@codeaurora.org>

Following is the flow in the HW if controller tries to read erased
page:

1. First ECC uncorrectable error will be generated from ECC engine
   since ECC engine first calculates the ECC with all 0xff and match
   the calculated ECC with ECC code in OOB (which is again all 0xff).
2. After getting ECC error, erased CW detection logic will be
   applied which is different for BCH and RS ECC
    a. For BCH, HW checks if all the bytes in page are 0xff and then
       it updates the status in separate register
       NAND_ERASED_CW_DETECT_STATUS.
    b. For RS ECC, the HW reports the same error when reading an
       erased CW, but it notifies that it is an erased CW by
       placing special characters at certain offsets in the
       buffer.

So the erased CW detect status should be checked only if ECC engine
generated the uncorrectable error.

Currently for all other operational errors also (like TIMEOUT, MPU
errors, etc.), the erased CW detect logic is being applied so fix this
and return EIO for other operational errors.

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
---
* Changes from v3:
  NONE

* Changes from v2:
  1. Changed commit message slightly

* Changes from v1:
  1. Added more detail in commit message
  2. Added comment before each if/else
  3. Removed redundant check for BS_UNCORRECTABLE_BIT

 drivers/mtd/nand/raw/qcom_nandc.c | 65 ++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index fc20149..0d931d5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1576,6 +1576,7 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	unsigned int max_bitflips = 0;
 	struct read_stats *buf;
+	bool flash_op_err = false;
 	int i;
 
 	buf = (struct read_stats *)nandc->reg_read_buf;
@@ -1597,8 +1598,18 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 		buffer = le32_to_cpu(buf->buffer);
 		erased_cw = le32_to_cpu(buf->erased_cw);
 
-		if (flash & (FS_OP_ERR | FS_MPU_ERR)) {
+		/*
+		 * Check ECC failure for each codeword. ECC failure can
+		 * happen in either of the following conditions
+		 * 1. If number of bitflips are greater than ECC engine
+		 *    capability.
+		 * 2. If this codeword contains all 0xff for which erased
+		 *    codeword detection check will be done.
+		 */
+		if ((flash & FS_OP_ERR) && (buffer & BS_UNCORRECTABLE_BIT)) {
 			bool erased;
+			int ret, ecclen, extraooblen;
+			void *eccbuf;
 
 			/* ignore erased codeword errors */
 			if (host->bch_enabled) {
@@ -1616,29 +1627,36 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 				continue;
 			}
 
-			if (buffer & BS_UNCORRECTABLE_BIT) {
-				int ret, ecclen, extraooblen;
-				void *eccbuf;
+			eccbuf = oob_buf ? oob_buf + oob_len : NULL;
+			ecclen = oob_buf ? host->ecc_bytes_hw : 0;
+			extraooblen = oob_buf ? oob_len : 0;
 
-				eccbuf = oob_buf ? oob_buf + oob_len : NULL;
-				ecclen = oob_buf ? host->ecc_bytes_hw : 0;
-				extraooblen = oob_buf ? oob_len : 0;
-
-				/*
-				 * make sure it isn't an erased page reported
-				 * as not-erased by HW because of a few bitflips
-				 */
-				ret = nand_check_erased_ecc_chunk(data_buf,
-					data_len, eccbuf, ecclen, oob_buf,
-					extraooblen, ecc->strength);
-				if (ret < 0) {
-					mtd->ecc_stats.failed++;
-				} else {
-					mtd->ecc_stats.corrected += ret;
-					max_bitflips =
-						max_t(unsigned int, max_bitflips, ret);
-				}
+			/*
+			 * make sure it isn't an erased page reported
+			 * as not-erased by HW because of a few bitflips
+			 */
+			ret = nand_check_erased_ecc_chunk(data_buf,
+				data_len, eccbuf, ecclen, oob_buf,
+				extraooblen, ecc->strength);
+			if (ret < 0) {
+				mtd->ecc_stats.failed++;
+			} else {
+				mtd->ecc_stats.corrected += ret;
+				max_bitflips =
+					max_t(unsigned int, max_bitflips, ret);
 			}
+		/*
+		 * Check if MPU or any other operational error (timeout,
+		 * device failure, etc.) happened for this codeword and
+		 * make flash_op_err true. If flash_op_err is set, then
+		 * EIO will be returned for page read.
+		 */
+		} else if (flash & (FS_OP_ERR | FS_MPU_ERR)) {
+			flash_op_err = true;
+		/*
+		 * No ECC or operational errors happened. Check the number of
+		 * bits corrected and update the ecc_stats.corrected.
+		 */
 		} else {
 			unsigned int stat;
 
@@ -1652,6 +1670,9 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
 			oob_buf += oob_len + ecc->bytes;
 	}
 
+	if (flash_op_err)
+		return -EIO;
+
 	return max_bitflips;
 }
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation


  parent reply	other threads:[~2018-06-20  7:27 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-20  7:27 [PATCH v4 00/15] Update for QCOM NAND driver Abhishek Sahu
2018-06-20  7:27 ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 01/15] mtd: rawnand: helper function for setting up ECC configuration Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-25  2:23   ` Masahiro Yamada
2018-06-20  7:27 ` [PATCH v4 02/15] mtd: rawnand: denali: use helper function for ecc setup Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 03/15] dt-bindings: qcom_nandc: update for ECC strength and step size Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20 16:01   ` Rob Herring
2018-06-20  7:27 ` [PATCH v4 04/15] mtd: rawnand: qcom: remove dt property nand-ecc-step-size Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 05/15] mtd: rawnand: qcom: use the ecc strength from device parameter Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 06/15] mtd: rawnand: qcom: wait for desc completion in all BAM channels Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` Abhishek Sahu [this message]
2018-06-20  7:27   ` [PATCH v4 07/15] mtd: rawnand: qcom: erased page detection for uncorrectable errors only Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 08/15] mtd: rawnand: qcom: fix null pointer access for erased page detection Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 09/15] mtd: rawnand: qcom: parse read errors for read oob also Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 10/15] mtd: rawnand: qcom: modify write_oob to remove read codeword part Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 11/15] mtd: rawnand: qcom: fix return value for raw page read Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 12/15] mtd: rawnand: qcom: check for operation errors in case of raw read Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 13/15] mtd: rawnand: qcom: code reorganization for " Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-20  7:27 ` [PATCH v4 14/15] mtd: rawnand: qcom: erased page bitflips detection Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-06-26 18:02   ` Miquel Raynal
2018-06-20  7:27 ` [PATCH v4 15/15] mtd: rawnand: provide only single helper function for ECC conf Abhishek Sahu
2018-06-20  7:27   ` Abhishek Sahu
2018-07-01 18:09 ` [PATCH v4 00/15] Update for QCOM NAND driver Miquel Raynal
2018-07-03  3:30   ` Abhishek Sahu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1529479662-4026-8-git-send-email-absahu@codeaurora.org \
    --to=absahu@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=architt@codeaurora.org \
    --cc=boris.brezillon@bootlin.com \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.