* v4:
1. Added patch to make other ECC configurations function static.
2. Clubbed the DT update patches.
3. Removed the bad block related patch. Discussion is going on
related with for proper solution so planning to submit separate
patch series for all bad block related changes.
4. Made the single codeword raw read function and used the same
for raw page read.
5. Changes in erased codeword detection to raw read function.
* v3:
1. Addressed all review comments in v2.
2. Added patch for removing redundant nand-ecc-step-size DT property.
3. Renamed ECC configuration setup function with minor code changes.
4. Modified comments and commit message for few patches.
* v2:
1. Addressed all review comments in v1.
1. Make the generic helper function for NAND ECC parameters setup
and used this helper function for QCOM and Denali nand driver
for ECC setup.
2. Modified commit message for some of the patches and added more
comments.
3. Added new patch for fixing ‘return 0’ for raw read.
4. Removed the read last codeword part for nand oob write.
5. Reorganized bad block check function and removed the
read_last_cw function completely.
* v1:
This patch series mainly deals with error handling and erased page
bitflip detection for QCOM NAND driver.
1. The error handling was missing for some of the cases so fixed
the same.
2. Add the support for taking ECC strength from ONFI parameter.
The earlier QCOM boards were coming with 4-bit ECC chip but
now the same boards are coming with 8-bit ECC chip since the
earlier 4-bit parts are obsolete from some vendors.
3. We got few issues related with NAND erased page bitflips. The
QCOM NAND controller can’t detect the bitflip in completely erased
page so added the support to detect the same. It implemented the
logic mentioned in patch [1] which didn’t go in mainline and later
the generic functions were provided [2] to count the number of
bitflips and make all 0xff. This patch series did some optimization
logic to prevent the unnecessary full page raw read and data copy
from QCOM NAND controller to DMA.
4. Following are the testing done for these patches in QCOM IPQ8074
HK01 (4-bit and 8-bit ECC chip) and IPQ806x AP148 boards.
a. Run all mtd test and check if it passes
b. Introduce custom bitflips in erased page and check if it
returns no error/EUCLEAN/EBADMSG depending upon number of
bitflips and position.
c. Introduce failure condition for operational failure and
check if it detects the same.
[1]: https://patchwork.ozlabs.org/patch/328994/
[2]: https://patchwork.ozlabs.org/patch/509970/
Abhishek Sahu (15):
mtd: rawnand: helper function for setting up ECC configuration
mtd: rawnand: denali: use helper function for ecc setup
dt-bindings: qcom_nandc: update for ECC strength and step size
mtd: rawnand: qcom: remove dt property nand-ecc-step-size
mtd: rawnand: qcom: use the ecc strength from device parameter
mtd: rawnand: qcom: wait for desc completion in all BAM channels
mtd: rawnand: qcom: erased page detection for uncorrectable errors
only
mtd: rawnand: qcom: fix null pointer access for erased page detection
mtd: rawnand: qcom: parse read errors for read oob also
mtd: rawnand: qcom: modify write_oob to remove read codeword part
mtd: rawnand: qcom: fix return value for raw page read
mtd: rawnand: qcom: check for operation errors in case of raw read
mtd: rawnand: qcom: code reorganization for raw read
mtd: rawnand: qcom: erased page bitflips detection
mtd: rawnand: provide only single helper function for ECC conf
.../devicetree/bindings/mtd/qcom_nandc.txt | 7 +-
drivers/mtd/nand/raw/denali.c | 30 +-
drivers/mtd/nand/raw/nand_base.c | 72 ++-
drivers/mtd/nand/raw/qcom_nandc.c | 491 ++++++++++++++-------
include/linux/mtd/rawnand.h | 10 +-
5 files changed, 380 insertions(+), 230 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
QCOM NAND controller supports only one step size (512) so
nand-ecc-step-size DT property is redundant. This property
can be removed and ecc step size can be assigned with 512 value.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Minor change in comment
(s/512 bytes of data in each step/512 bytes data steps)
* Changes from v2:
NEW CHANGE
1. Removed the custom logic and used the helper fuction.
drivers/mtd/nand/raw/qcom_nandc.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 6a5519f..bf80a61 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2325,15 +2325,8 @@ static int qcom_nand_host_setup(struct qcom_nand_host *host)
bool wide_bus;
int ecc_mode = 1;
- /*
- * the controller requires each step consists of 512 bytes of data.
- * bail out if DT has populated a wrong step size.
- */
- if (ecc->size != NANDC_STEP_SIZE) {
- dev_err(nandc->dev, "invalid ecc size\n");
- return -EINVAL;
- }
-
+ /* controller only supports 512 bytes data steps */
+ ecc->size = NANDC_STEP_SIZE;
wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
if (ecc->strength >= 8) {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Fix value returned by ->read_page_raw() to be the
actual operation status, instead of always 0.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
NONE
* Changes from v2:
1. Changed commit message
* Changes from v1:
NEW CHANGE
drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 28361b5..887b1f6 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1898,7 +1898,7 @@ static int qcom_nandc_read_page_raw(struct mtd_info *mtd,
free_descs(nandc);
- return 0;
+ return ret;
}
/* implements ecc->read_oob() */
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
read_page and read_oob both calls the read_page_ecc function.
The QCOM NAND controller protect the OOB available bytes with
ECC so read errors should be checked for read_oob also.
This patch moves the error checking code inside read_page_ecc
so caller does not have to check explicitly for read errors.
Reviewed-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
NONE
* Changes from v2:
NONE
* Changes from v1:
1. Minor code change for return early in case of error
drivers/mtd/nand/raw/qcom_nandc.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index a831f9c..285b2ad3 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1698,6 +1698,7 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
struct nand_chip *chip = &host->chip;
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
+ u8 *data_buf_start = data_buf, *oob_buf_start = oob_buf;
int i, ret;
config_nand_page_read(nandc);
@@ -1758,12 +1759,14 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
}
ret = submit_descs(nandc);
- if (ret)
+ free_descs(nandc);
+
+ if (ret) {
dev_err(nandc->dev, "failure to read page/oob\n");
+ return ret;
+ }
- free_descs(nandc);
-
- return ret;
+ return parse_read_errors(host, data_buf_start, oob_buf_start);
}
/*
@@ -1808,20 +1811,14 @@ static int qcom_nandc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
struct qcom_nand_host *host = to_qcom_nand_host(chip);
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
u8 *data_buf, *oob_buf = NULL;
- int ret;
nand_read_page_op(chip, page, 0, NULL, 0);
data_buf = buf;
oob_buf = oob_required ? chip->oob_poi : NULL;
clear_bam_transaction(nandc);
- ret = read_page_ecc(host, data_buf, oob_buf);
- if (ret) {
- dev_err(nandc->dev, "failure to read page\n");
- return ret;
- }
- return parse_read_errors(host, data_buf, oob_buf);
+ return read_page_ecc(host, data_buf, oob_buf);
}
/* implements ecc->read_page_raw() */
@@ -1911,7 +1908,6 @@ static int qcom_nandc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
struct qcom_nand_host *host = to_qcom_nand_host(chip);
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
- int ret;
clear_read_regs(nandc);
clear_bam_transaction(nandc);
@@ -1920,11 +1916,7 @@ static int qcom_nandc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
set_address(host, 0, page);
update_rw_regs(host, ecc->steps, true);
- ret = read_page_ecc(host, NULL, chip->oob_poi);
- if (ret)
- dev_err(nandc->dev, "failure to read oob\n");
-
- return ret;
+ return read_page_ecc(host, NULL, chip->oob_poi);
}
/* implements ecc->write_page() */
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
NAND parts can have bitflips in an erased page due to the
process technology used. In this case, QCOM NAND controller
is not able to identify that page as an erased page.
Currently the driver calls nand_check_erased_ecc_chunk() for
identifying the erased pages but this won’t work always since the
checking is being with ECC engine returned data. In case of
bitflips, the ECC engine tries to correct the data and then it
generates the uncorrectable error. Now, this data is not equal to
original raw data. For erased CW identification, the raw data
should be read again from NAND device and this
nand_check_erased_ecc_chunk function() should be called for raw
data only.
Now following logic is being added to identify the erased
codeword bitflips.
1. In most of the cases, not all the codewords will have bitflips
and only single CW will have bitflips. So, there is no need to
read the complete raw page data. The NAND raw read can be
scheduled for any CW in page. The NAND controller works on CW
basis and it will update the status register after each CW read.
Maintain the bitmask for the CW which generated the uncorrectable
error.
2. Do raw read for all the CW's which generated the uncorrectable
error.
3. Both DATA and OOB need to be checked for number of 0. The
top-level API can be called with only data buf or OOB buf so use
chip->databuf if data buf is null and chip->oob_poi if
OOB buf is null for copying the raw bytes temporarily.
4. For each CW, check the number of 0 in cw_data and usable
oob bytes, The bbm and spare (unused) bytes bit flip won’t
affect the ECC so don’t check the number of bitflips in this area.
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Major changes in erased codeword detection for
raw read function
* Changes from v2:
NONE
* Changes from v1:
1. Minor change in commit message
2. invalidate pagebuf if databuf or oob_poi is used
drivers/mtd/nand/raw/qcom_nandc.c | 127 +++++++++++++++++++++++++++-----------
1 file changed, 90 insertions(+), 37 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 160acdf..e34edf1 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1656,20 +1656,95 @@ static int check_flash_errors(struct qcom_nand_host *host, int cw_cnt)
}
/*
+ * Bitflips can happen in erased codewords also so this function counts the
+ * number of 0 in each CW for which ECC engine returns the uncorrectable
+ * error. The page will be assumed as erased if this count is less than or
+ * equal to the ecc->strength for each CW.
+ *
+ * 1. Both DATA and OOB need to be checked for number of 0. The
+ * top-level API can be called with only data buf or OOB buf so use
+ * chip->data_buf if data buf is null and chip->oob_poi if oob buf
+ * is null for copying the raw bytes.
+ * 2. Perform raw read for all the CW which has uncorrectable errors.
+ * 3. For each CW, check the number of 0 in cw_data and usable OOB bytes.
+ * The BBM and spare bytes bit flip won’t affect the ECC so don’t check
+ * the number of bitflips in this area.
+ */
+static int
+check_for_erased_page(struct qcom_nand_host *host, u8 *data_buf,
+ u8 *oob_buf, unsigned long uncorrectable_cws,
+ int page, unsigned int max_bitflips)
+{
+ struct nand_chip *chip = &host->chip;
+ struct mtd_info *mtd = nand_to_mtd(chip);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ int cw, data_size, oob_size, ret = 0;
+
+ if (!data_buf) {
+ data_buf = chip->data_buf;
+ chip->pagebuf = -1;
+ }
+
+ if (!oob_buf) {
+ oob_buf = chip->oob_poi;
+ chip->pagebuf = -1;
+ }
+
+ for (cw = 0; cw < ecc->steps && uncorrectable_cws; cw++) {
+ if (cw == (ecc->steps - 1)) {
+ data_size = ecc->size - ((ecc->steps - 1) * 4);
+ oob_size = (ecc->steps * 4) + host->ecc_bytes_hw;
+ } else {
+ data_size = host->cw_data;
+ oob_size = host->ecc_bytes_hw;
+ }
+
+ if (uncorrectable_cws & BIT(0)) {
+ ret = qcom_nandc_read_cw_raw(mtd, chip, data_buf,
+ oob_buf, page, cw);
+ if (ret)
+ return 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_size, oob_buf + host->bbm_size,
+ oob_size, NULL,
+ 0, ecc->strength);
+ if (ret < 0) {
+ mtd->ecc_stats.failed++;
+ } else {
+ mtd->ecc_stats.corrected += ret;
+ max_bitflips =
+ max_t(unsigned int, max_bitflips, ret);
+ }
+ }
+
+ data_buf += data_size;
+ oob_buf += ecc->bytes;
+ uncorrectable_cws >>= 1;
+ }
+
+ return max_bitflips;
+}
+
+/*
* reads back status registers set by the controller to notify page read
* errors. this is equivalent to what 'ecc->correct()' would do.
*/
static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
- u8 *oob_buf)
+ u8 *oob_buf, int page)
{
struct nand_chip *chip = &host->chip;
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
- unsigned int max_bitflips = 0;
+ unsigned int max_bitflips = 0, uncorrectable_cws = 0;
struct read_stats *buf;
- bool flash_op_err = false;
+ bool flash_op_err = false, erased;
int i;
+ u8 *data_buf_start = data_buf, *oob_buf_start = oob_buf;
buf = (struct read_stats *)nandc->reg_read_buf;
nandc_read_buffer_sync(nandc, true);
@@ -1699,10 +1774,6 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
* codeword detection check will be done.
*/
if ((flash & FS_OP_ERR) && (buffer & BS_UNCORRECTABLE_BIT)) {
- bool erased;
- int ret, ecclen, extraooblen;
- void *eccbuf;
-
/*
* For BCH ECC, ignore erased codeword errors, if
* ERASED_CW bits are set.
@@ -1723,31 +1794,8 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
erased = false;
}
- if (erased) {
- data_buf += data_len;
- if (oob_buf)
- oob_buf += oob_len + ecc->bytes;
- continue;
- }
-
- 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);
- }
+ if (!erased)
+ uncorrectable_cws |= BIT(i);
/*
* Check if MPU or any other operational error (timeout,
* device failure, etc.) happened for this codeword and
@@ -1777,7 +1825,12 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
if (flash_op_err)
return -EIO;
- return max_bitflips;
+ if (!uncorrectable_cws)
+ return max_bitflips;
+
+ return check_for_erased_page(host, data_buf_start, oob_buf_start,
+ uncorrectable_cws, page,
+ max_bitflips);
}
/*
@@ -1785,7 +1838,7 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
* ecc->read_oob()
*/
static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
- u8 *oob_buf)
+ u8 *oob_buf, int page)
{
struct nand_chip *chip = &host->chip;
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
@@ -1858,7 +1911,7 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
return ret;
}
- return parse_read_errors(host, data_buf_start, oob_buf_start);
+ return parse_read_errors(host, data_buf_start, oob_buf_start, page);
}
/*
@@ -1910,7 +1963,7 @@ static int qcom_nandc_read_page(struct mtd_info *mtd, struct nand_chip *chip,
clear_bam_transaction(nandc);
- return read_page_ecc(host, data_buf, oob_buf);
+ return read_page_ecc(host, data_buf, oob_buf, page);
}
/* implements ecc->read_page_raw() */
@@ -1951,7 +2004,7 @@ static int qcom_nandc_read_oob(struct mtd_info *mtd, struct nand_chip *chip,
set_address(host, 0, page);
update_rw_regs(host, ecc->steps, true);
- return read_page_ecc(host, NULL, chip->oob_poi);
+ return read_page_ecc(host, NULL, chip->oob_poi, page);
}
/* implements ecc->write_page() */
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Currently there is no error checking for raw read. For raw
reads, there won’t be any ECC failure but the operational
failures are possible, so schedule the NAND_FLASH_STATUS read
after each codeword.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Since bad block reorganization patch has removed from this
patch series so following change is required in copy_last_cw)
config_nand_single_cw_page_read(nandc);
-> config_nand_single_cw_page_read(nandc, host->use_ecc);
* Changes from v2:
NONE
* Changes from v1:
1. Removed the code for copy_last_cw.
drivers/mtd/nand/raw/qcom_nandc.c | 58 +++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 887b1f6..5999c39 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1096,7 +1096,8 @@ static void config_nand_page_read(struct qcom_nand_controller *nandc)
* Helper to prepare DMA descriptors for configuring registers
* before reading each codeword in NAND page.
*/
-static void config_nand_cw_read(struct qcom_nand_controller *nandc)
+static void
+config_nand_cw_read(struct qcom_nand_controller *nandc, bool use_ecc)
{
if (nandc->props->is_bam)
write_reg_dma(nandc, NAND_READ_LOCATION_0, 4,
@@ -1105,19 +1106,25 @@ static void config_nand_cw_read(struct qcom_nand_controller *nandc)
write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
- read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
- read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
- NAND_BAM_NEXT_SGL);
+ if (use_ecc) {
+ read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
+ read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
+ NAND_BAM_NEXT_SGL);
+ } else {
+ read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
+ }
}
/*
* Helper to prepare dma descriptors to configure registers needed for reading a
* single codeword in page
*/
-static void config_nand_single_cw_page_read(struct qcom_nand_controller *nandc)
+static void
+config_nand_single_cw_page_read(struct qcom_nand_controller *nandc,
+ bool use_ecc)
{
config_nand_page_read(nandc);
- config_nand_cw_read(nandc);
+ config_nand_cw_read(nandc, use_ecc);
}
/*
@@ -1198,7 +1205,7 @@ static int nandc_param(struct qcom_nand_host *host)
nandc->buf_count = 512;
memset(nandc->data_buffer, 0xff, nandc->buf_count);
- config_nand_single_cw_page_read(nandc);
+ config_nand_single_cw_page_read(nandc, false);
read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer,
nandc->buf_count, 0);
@@ -1563,6 +1570,23 @@ struct read_stats {
__le32 erased_cw;
};
+/* reads back FLASH_STATUS register set by the controller */
+static int check_flash_errors(struct qcom_nand_host *host, int cw_cnt)
+{
+ struct nand_chip *chip = &host->chip;
+ struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+ int i;
+
+ for (i = 0; i < cw_cnt; i++) {
+ u32 flash = le32_to_cpu(nandc->reg_read_buf[i]);
+
+ if (flash & (FS_OP_ERR | FS_MPU_ERR))
+ return -EIO;
+ }
+
+ return 0;
+}
+
/*
* reads back status registers set by the controller to notify page read
* errors. this is equivalent to what 'ecc->correct()' would do.
@@ -1729,7 +1753,7 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
}
}
- config_nand_cw_read(nandc);
+ config_nand_cw_read(nandc, true);
if (data_buf)
read_data_dma(nandc, FLASH_BUF_ACC, data_buf,
@@ -1791,7 +1815,7 @@ static int copy_last_cw(struct qcom_nand_host *host, int page)
set_address(host, host->cw_size * (ecc->steps - 1), page);
update_rw_regs(host, 1, true);
- config_nand_single_cw_page_read(nandc);
+ config_nand_single_cw_page_read(nandc, host->use_ecc);
read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, size, 0);
@@ -1874,7 +1898,7 @@ static int qcom_nandc_read_page_raw(struct mtd_info *mtd,
nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1);
}
- config_nand_cw_read(nandc);
+ config_nand_cw_read(nandc, false);
read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
reg_off += data_size1;
@@ -1893,12 +1917,13 @@ static int qcom_nandc_read_page_raw(struct mtd_info *mtd,
}
ret = submit_descs(nandc);
- if (ret)
+ free_descs(nandc);
+ if (ret) {
dev_err(nandc->dev, "failure to read raw page\n");
+ return ret;
+ }
- free_descs(nandc);
-
- return ret;
+ return check_flash_errors(host, ecc->steps);
}
/* implements ecc->read_oob() */
@@ -2117,7 +2142,6 @@ static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs)
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
int page, ret, bbpos, bad = 0;
- u32 flash_status;
page = (int)(ofs >> chip->page_shift) & chip->pagemask;
@@ -2134,9 +2158,7 @@ static int qcom_nandc_block_bad(struct mtd_info *mtd, loff_t ofs)
if (ret)
goto err;
- flash_status = le32_to_cpu(nandc->reg_read_buf[0]);
-
- if (flash_status & (FS_OP_ERR | FS_MPU_ERR)) {
+ if (check_flash_errors(host, 1)) {
dev_warn(nandc->dev, "error when trying to read BBM\n");
goto err;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
commit 2c8f8afa7f92 ("mtd: nand: add generic helpers to check,
match, maximize ECC settings") provides generic helpers which
drivers can use for setting up ECC parameters.
Since same board can have different ECC strength nand chips so
following is the logic for setting up ECC strength and ECC step
size, which can be used by most of the drivers.
1. If both ECC step size and ECC strength are already set
(usually by DT) then just check whether this setting
is supported by NAND controller.
2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength
supported by NAND controller.
3. Otherwise, try to match the ECC step size and ECC strength closest
to the chip's requirement. If available OOB size can't fit the chip
requirement then select maximum ECC strength which can be fit with
available OOB size.
This patch introduces nand_ecc_choose_conf function which calls the
required helper functions for the above logic. The drivers can use
this single function instead of calling the 3 helper functions
individually.
CC: Masahiro Yamada <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. call nand_maximize_ecc() 2 times to make code more clear.
* Changes from v2:
1. Renamed function to nand_ecc_choose_conf.
2. Minor code reorganization to remove warning and 2 function calls
for nand_maximize_ecc.
* Changes from v1:
NEW PATCH
drivers/mtd/nand/raw/nand_base.c | 33 +++++++++++++++++++++++++++++++++
include/linux/mtd/rawnand.h | 3 +++
2 files changed, 36 insertions(+)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 10c4f991..c64e3fc 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -6287,6 +6287,39 @@ int nand_maximize_ecc(struct nand_chip *chip,
}
EXPORT_SYMBOL_GPL(nand_maximize_ecc);
+/**
+ * nand_ecc_choose_conf - Set the ECC strength and ECC step size
+ * @chip: nand chip info structure
+ * @caps: ECC engine caps info structure
+ * @oobavail: OOB size that the ECC engine can use
+ *
+ * Choose the ECC configuration according to following logic
+ *
+ * 1. If both ECC step size and ECC strength are already set (usually by DT)
+ * then check if it is supported by this controller.
+ * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength.
+ * 3. Otherwise, try to match the ECC step size and ECC strength closest
+ * to the chip's requirement. If available OOB size can't fit the chip
+ * requirement then fallback to the maximum ECC step size and ECC strength.
+ *
+ * On success, the chosen ECC settings are set.
+ */
+int nand_ecc_choose_conf(struct nand_chip *chip,
+ const struct nand_ecc_caps *caps, int oobavail)
+{
+ if (chip->ecc.size && chip->ecc.strength)
+ return nand_check_ecc_caps(chip, caps, oobavail);
+
+ if (chip->ecc.options & NAND_ECC_MAXIMIZE)
+ return nand_maximize_ecc(chip, caps, oobavail);
+
+ if (!nand_match_ecc_req(chip, caps, oobavail))
+ return 0;
+
+ return nand_maximize_ecc(chip, caps, oobavail);
+}
+EXPORT_SYMBOL_GPL(nand_ecc_choose_conf);
+
/*
* Check if the chip configuration meet the datasheet requirements.
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 3e8ec3b..03a0061 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1650,6 +1650,9 @@ int nand_match_ecc_req(struct nand_chip *chip,
int nand_maximize_ecc(struct nand_chip *chip,
const struct nand_ecc_caps *caps, int oobavail);
+int nand_ecc_choose_conf(struct nand_chip *chip,
+ const struct nand_ecc_caps *caps, int oobavail);
+
/* Default write_oob implementation */
int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
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 <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* 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
parse_read_errors can be called with only oob_buf in which case
data_buf will be NULL. If data_buf is NULL, then don’t
treat this page as completely erased in case of ECC uncorrectable
error for RS ECC. For BCH ECC, the controller itself tells
regarding erased page in status register.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
NONE
* Changes from v2:
NONE
* Changes from v1:
1. Added more detail in commit message
2. Added comment before each if/else
drivers/mtd/nand/raw/qcom_nandc.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 0d931d5..a831f9c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1611,13 +1611,24 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
int ret, ecclen, extraooblen;
void *eccbuf;
- /* ignore erased codeword errors */
+ /*
+ * For BCH ECC, ignore erased codeword errors, if
+ * ERASED_CW bits are set.
+ */
if (host->bch_enabled) {
erased = (erased_cw & ERASED_CW) == ERASED_CW ?
true : false;
- } else {
+ /*
+ * For RS ECC, HW reports the erased CW by placing
+ * special characters at certain offsets in the buffer.
+ * These special characters will be valid only if
+ * complete page is read i.e. data_buf is not NULL.
+ */
+ } else if (data_buf) {
erased = erased_chunk_check_and_fixup(data_buf,
data_len);
+ } else {
+ erased = false;
}
if (erased) {
@@ -1665,7 +1676,8 @@ static int parse_read_errors(struct qcom_nand_host *host, u8 *data_buf,
max_bitflips = max(max_bitflips, stat);
}
- data_buf += data_len;
+ if (data_buf)
+ data_buf += data_len;
if (oob_buf)
oob_buf += oob_len + ecc->bytes;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Use the NAND core helper function nand_ecc_choose_conf to tune
the ECC parameters instead of the function locally defined.
Acked-by: Miquel Raynal <[email protected]>
Acked-by: Masahiro Yamada <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
NONE
* Changes from v2:
1. Changed commit message
* Changes from v1:
NEW PATCH
drivers/mtd/nand/raw/denali.c | 30 ++----------------------------
1 file changed, 2 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
index 2a302a1..a586a1d 100644
--- a/drivers/mtd/nand/raw/denali.c
+++ b/drivers/mtd/nand/raw/denali.c
@@ -1120,33 +1120,6 @@ int denali_calc_ecc_bytes(int step_size, int strength)
}
EXPORT_SYMBOL(denali_calc_ecc_bytes);
-static int denali_ecc_setup(struct mtd_info *mtd, struct nand_chip *chip,
- struct denali_nand_info *denali)
-{
- int oobavail = mtd->oobsize - denali->oob_skip_bytes;
- int ret;
-
- /*
- * If .size and .strength are already set (usually by DT),
- * check if they are supported by this controller.
- */
- if (chip->ecc.size && chip->ecc.strength)
- return nand_check_ecc_caps(chip, denali->ecc_caps, oobavail);
-
- /*
- * We want .size and .strength closest to the chip's requirement
- * unless NAND_ECC_MAXIMIZE is requested.
- */
- if (!(chip->ecc.options & NAND_ECC_MAXIMIZE)) {
- ret = nand_match_ecc_req(chip, denali->ecc_caps, oobavail);
- if (!ret)
- return 0;
- }
-
- /* Max ECC strength is the last thing we can do */
- return nand_maximize_ecc(chip, denali->ecc_caps, oobavail);
-}
-
static int denali_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
{
@@ -1317,7 +1290,8 @@ int denali_init(struct denali_nand_info *denali)
chip->ecc.mode = NAND_ECC_HW_SYNDROME;
chip->options |= NAND_NO_SUBPAGE_WRITE;
- ret = denali_ecc_setup(mtd, chip, denali);
+ ret = nand_ecc_choose_conf(chip, denali->ecc_caps,
+ mtd->oobsize - denali->oob_skip_bytes);
if (ret) {
dev_err(denali->dev, "Failed to setup ECC settings.\n");
goto disable_irq;
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Function nand_ecc_choose_conf() will be help for all the cases, so
other helper functions can be made static.
nand_check_ecc_caps(): Invoke nand_ecc_choose_conf() with
both chip->ecc.size and chip->ecc.strength
value set.
nand_maximize_ecc(): Invoke nand_ecc_choose_conf() with
NAND_ECC_MAXIMIZE flag.
nand_match_ecc_req(): Invoke nand_ecc_choose_conf() with either
chip->ecc.size or chip->ecc.strength value
set and without NAND_ECC_MAXIMIZE flag.
CC: Masahiro Yamada <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
Changes from v3:
NEW PATCH
drivers/mtd/nand/raw/nand_base.c | 39 +++++++++++++++------------------------
include/linux/mtd/rawnand.h | 9 ---------
2 files changed, 15 insertions(+), 33 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c64e3fc..f620236 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -6077,24 +6077,17 @@ static int nand_set_ecc_soft_ops(struct mtd_info *mtd)
* by the controller and the calculated ECC bytes fit within the chip's OOB.
* On success, the calculated ECC bytes is set.
*/
-int nand_check_ecc_caps(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail)
+static int
+nand_check_ecc_caps(struct nand_chip *chip,
+ const struct nand_ecc_caps *caps, int oobavail)
{
struct mtd_info *mtd = nand_to_mtd(chip);
const struct nand_ecc_step_info *stepinfo;
int preset_step = chip->ecc.size;
int preset_strength = chip->ecc.strength;
- int nsteps, ecc_bytes;
+ int ecc_bytes, nsteps = mtd->writesize / preset_step;
int i, j;
- if (WARN_ON(oobavail < 0))
- return -EINVAL;
-
- if (!preset_step || !preset_strength)
- return -ENODATA;
-
- nsteps = mtd->writesize / preset_step;
-
for (i = 0; i < caps->nstepinfos; i++) {
stepinfo = &caps->stepinfos[i];
@@ -6127,7 +6120,6 @@ int nand_check_ecc_caps(struct nand_chip *chip,
return -ENOTSUPP;
}
-EXPORT_SYMBOL_GPL(nand_check_ecc_caps);
/**
* nand_match_ecc_req - meet the chip's requirement with least ECC bytes
@@ -6139,8 +6131,9 @@ int nand_check_ecc_caps(struct nand_chip *chip,
* number of ECC bytes (i.e. with the largest number of OOB-free bytes).
* On success, the chosen ECC settings are set.
*/
-int nand_match_ecc_req(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail)
+static int
+nand_match_ecc_req(struct nand_chip *chip,
+ const struct nand_ecc_caps *caps, int oobavail)
{
struct mtd_info *mtd = nand_to_mtd(chip);
const struct nand_ecc_step_info *stepinfo;
@@ -6151,9 +6144,6 @@ int nand_match_ecc_req(struct nand_chip *chip,
int best_ecc_bytes_total = INT_MAX;
int i, j;
- if (WARN_ON(oobavail < 0))
- return -EINVAL;
-
/* No information provided by the NAND chip */
if (!req_step || !req_strength)
return -ENOTSUPP;
@@ -6212,7 +6202,6 @@ int nand_match_ecc_req(struct nand_chip *chip,
return 0;
}
-EXPORT_SYMBOL_GPL(nand_match_ecc_req);
/**
* nand_maximize_ecc - choose the max ECC strength available
@@ -6223,8 +6212,9 @@ int nand_match_ecc_req(struct nand_chip *chip,
* Choose the max ECC strength that is supported on the controller, and can fit
* within the chip's OOB. On success, the chosen ECC settings are set.
*/
-int nand_maximize_ecc(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail)
+static int
+nand_maximize_ecc(struct nand_chip *chip,
+ const struct nand_ecc_caps *caps, int oobavail)
{
struct mtd_info *mtd = nand_to_mtd(chip);
const struct nand_ecc_step_info *stepinfo;
@@ -6234,9 +6224,6 @@ int nand_maximize_ecc(struct nand_chip *chip,
int best_strength, best_ecc_bytes;
int i, j;
- if (WARN_ON(oobavail < 0))
- return -EINVAL;
-
for (i = 0; i < caps->nstepinfos; i++) {
stepinfo = &caps->stepinfos[i];
step_size = stepinfo->stepsize;
@@ -6285,7 +6272,6 @@ int nand_maximize_ecc(struct nand_chip *chip,
return 0;
}
-EXPORT_SYMBOL_GPL(nand_maximize_ecc);
/**
* nand_ecc_choose_conf - Set the ECC strength and ECC step size
@@ -6307,6 +6293,11 @@ int nand_maximize_ecc(struct nand_chip *chip,
int nand_ecc_choose_conf(struct nand_chip *chip,
const struct nand_ecc_caps *caps, int oobavail)
{
+ struct mtd_info *mtd = nand_to_mtd(chip);
+
+ if (WARN_ON(oobavail < 0 || oobavail > mtd->oobsize))
+ return -EINVAL;
+
if (chip->ecc.size && chip->ecc.strength)
return nand_check_ecc_caps(chip, caps, oobavail);
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 03a0061..0945868 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1641,15 +1641,6 @@ int nand_check_erased_ecc_chunk(void *data, int datalen,
void *extraoob, int extraooblen,
int threshold);
-int nand_check_ecc_caps(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail);
-
-int nand_match_ecc_req(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail);
-
-int nand_maximize_ecc(struct nand_chip *chip,
- const struct nand_ecc_caps *caps, int oobavail);
-
int nand_ecc_choose_conf(struct nand_chip *chip,
const struct nand_ecc_caps *caps, int oobavail);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Currently the driver uses the ECC strength specified in DT.
The QPIC/EBI2 NAND supports 4 or 8-bit ECC correction. The same
kind of board can have different NAND parts so use the ECC
strength from device parameters if it is not specified in DT.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Added parenthesis around (cwperpage * 4)
* Changes from v2:
1. s/<< 2/* 4/
2. Updated the cwperpage location
3. The block handling the ecc-step-size property has been
removed in a previous patch
* Changes from v1:
1. Removed the custom logic and used the helper fuction.
drivers/mtd/nand/raw/qcom_nandc.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index bf80a61..2375780 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2315,19 +2315,39 @@ static int qcom_nand_ooblayout_free(struct mtd_info *mtd, int section,
.free = qcom_nand_ooblayout_free,
};
+static int
+qcom_nandc_calc_ecc_bytes(int step_size, int strength)
+{
+ return strength == 4 ? 12 : 16;
+}
+NAND_ECC_CAPS_SINGLE(qcom_nandc_ecc_caps, qcom_nandc_calc_ecc_bytes,
+ NANDC_STEP_SIZE, 4, 8);
+
static int qcom_nand_host_setup(struct qcom_nand_host *host)
{
struct nand_chip *chip = &host->chip;
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_ecc_ctrl *ecc = &chip->ecc;
struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
- int cwperpage, bad_block_byte;
+ int cwperpage, bad_block_byte, ret;
bool wide_bus;
int ecc_mode = 1;
/* controller only supports 512 bytes data steps */
ecc->size = NANDC_STEP_SIZE;
wide_bus = chip->options & NAND_BUSWIDTH_16 ? true : false;
+ cwperpage = mtd->writesize / NANDC_STEP_SIZE;
+
+ /*
+ * Each CW has 4 available OOB bytes which will be protected with ECC
+ * so remaining bytes can be used for ECC.
+ */
+ ret = nand_ecc_choose_conf(chip, &qcom_nandc_ecc_caps,
+ mtd->oobsize - (cwperpage * 4));
+ if (ret) {
+ dev_err(nandc->dev, "No valid ECC settings possible\n");
+ return ret;
+ }
if (ecc->strength >= 8) {
/* 8 bit ECC defaults to BCH ECC on all platforms */
@@ -2396,7 +2416,6 @@ static int qcom_nand_host_setup(struct qcom_nand_host *host)
mtd_set_ooblayout(mtd, &qcom_nand_ooblayout_ops);
- cwperpage = mtd->writesize / ecc->size;
nandc->max_cwperpage = max_t(unsigned int, nandc->max_cwperpage,
cwperpage);
@@ -2412,12 +2431,6 @@ static int qcom_nand_host_setup(struct qcom_nand_host *host)
* for 8 bit ECC
*/
host->cw_size = host->cw_data + ecc->bytes;
-
- if (ecc->bytes * (mtd->writesize / ecc->size) > mtd->oobsize) {
- dev_err(nandc->dev, "ecc data doesn't fit in OOB area\n");
- return -EINVAL;
- }
-
bad_block_byte = mtd->writesize - host->cw_size * (cwperpage - 1) + 1;
host->cfg0 = (cwperpage - 1) << CW_PER_PAGE
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
1. If nand-ecc-strength specified in DT, then controller will use
this ECC strength otherwise ECC strength will be calculated
according to chip requirement and available OOB size.
2. QCOM NAND controller supports only one step size (512 bytes) but
nand-ecc-step-size is required property in DT. This DT property
can be removed and ecc step size can be assigned in driver with
512 bytes value.
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Clubbed following 2 patches into one
https://patchwork.ozlabs.org/patch/920465/
https://patchwork.ozlabs.org/patch/920467/
* Changes from v2:
NONE
* Changes from v1:
NEW PATCH
Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
index 73d336be..1123cc6 100644
--- a/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
+++ b/Documentation/devicetree/bindings/mtd/qcom_nandc.txt
@@ -45,11 +45,12 @@ Required properties:
number (e.g., 0, 1, 2, etc.)
- #address-cells: see partition.txt
- #size-cells: see partition.txt
-- nand-ecc-strength: see nand.txt
-- nand-ecc-step-size: must be 512. see nand.txt for more details.
Optional properties:
- nand-bus-width: see nand.txt
+- nand-ecc-strength: see nand.txt. If not specified, then ECC strength will
+ be used according to chip requirement and available
+ OOB size.
Each nandcs device node may optionally contain a 'partitions' sub-node, which
further contains sub-nodes describing the flash partition mapping. See
@@ -77,7 +78,6 @@ nand-controller@1ac00000 {
reg = <0>;
nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
nand-bus-width = <8>;
partitions {
@@ -117,7 +117,6 @@ nand-controller@79b0000 {
nand@0 {
reg = <0>;
nand-ecc-strength = <4>;
- nand-ecc-step-size = <512>;
nand-bus-width = <8>;
partitions {
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
Make separate function to perform raw read for one codeword and
call this function multiple times for each codeword in case of
raw page read. This separate function will help in subsequent
patches related with erased codeword bitflip detection.
It will decrease throughput for raw page read. Raw page read
is used for debug purpose so it won't affect normal flash
operations.
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. Major code changes for making separate function for raw_read
2. Changed commit message
* Changes from v2:
NONE
* Changes from v1:
1. Included more detail in function comment
drivers/mtd/nand/raw/qcom_nandc.c | 146 ++++++++++++++++++++------------------
1 file changed, 78 insertions(+), 68 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5999c39..160acdf 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1587,6 +1587,74 @@ static int check_flash_errors(struct qcom_nand_host *host, int cw_cnt)
return 0;
}
+/* performs raw read for one codeword */
+static int
+qcom_nandc_read_cw_raw(struct mtd_info *mtd, struct nand_chip *chip,
+ u8 *data_buf, u8 *oob_buf, int page, int cw)
+{
+ struct qcom_nand_host *host = to_qcom_nand_host(chip);
+ struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
+ struct nand_ecc_ctrl *ecc = &chip->ecc;
+ int data_size1, data_size2, oob_size1, oob_size2;
+ int ret, reg_off = FLASH_BUF_ACC, read_loc = 0;
+
+ nand_read_page_op(chip, page, 0, NULL, 0);
+ host->use_ecc = false;
+
+ clear_bam_transaction(nandc);
+ set_address(host, host->cw_size * cw, page);
+ update_rw_regs(host, 1, true);
+ config_nand_page_read(nandc);
+
+ data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1);
+ oob_size1 = host->bbm_size;
+
+ if (cw == (ecc->steps - 1)) {
+ data_size2 = ecc->size - data_size1 -
+ ((ecc->steps - 1) * 4);
+ oob_size2 = (ecc->steps * 4) + host->ecc_bytes_hw +
+ host->spare_bytes;
+ } else {
+ data_size2 = host->cw_data - data_size1;
+ oob_size2 = host->ecc_bytes_hw + host->spare_bytes;
+ }
+
+ if (nandc->props->is_bam) {
+ nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0);
+ read_loc += data_size1;
+
+ nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0);
+ read_loc += oob_size1;
+
+ nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0);
+ read_loc += data_size2;
+
+ nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1);
+ }
+
+ config_nand_cw_read(nandc, false);
+
+ read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
+ reg_off += data_size1;
+
+ read_data_dma(nandc, reg_off, oob_buf, oob_size1, 0);
+ reg_off += oob_size1;
+
+ read_data_dma(nandc, reg_off, data_buf + data_size1, data_size2, 0);
+ reg_off += data_size2;
+
+ read_data_dma(nandc, reg_off, oob_buf + oob_size1, oob_size2, 0);
+
+ ret = submit_descs(nandc);
+ free_descs(nandc);
+ if (ret) {
+ dev_err(nandc->dev, "failure to read raw cw %d\n", cw);
+ return ret;
+ }
+
+ return check_flash_errors(host, 1);
+}
+
/*
* reads back status registers set by the controller to notify page read
* errors. this is equivalent to what 'ecc->correct()' would do.
@@ -1851,79 +1919,21 @@ static int qcom_nandc_read_page_raw(struct mtd_info *mtd,
int oob_required, int page)
{
struct qcom_nand_host *host = to_qcom_nand_host(chip);
- struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
- u8 *data_buf, *oob_buf;
struct nand_ecc_ctrl *ecc = &chip->ecc;
- int i, ret;
- int read_loc;
+ int cw, ret;
+ u8 *data_buf = buf, *oob_buf = chip->oob_poi;
- nand_read_page_op(chip, page, 0, NULL, 0);
- data_buf = buf;
- oob_buf = chip->oob_poi;
+ for (cw = 0; cw < ecc->steps; cw++) {
+ ret = qcom_nandc_read_cw_raw(mtd, chip, data_buf, oob_buf,
+ page, cw);
+ if (ret)
+ return ret;
- host->use_ecc = false;
-
- clear_bam_transaction(nandc);
- update_rw_regs(host, ecc->steps, true);
- config_nand_page_read(nandc);
-
- for (i = 0; i < ecc->steps; i++) {
- int data_size1, data_size2, oob_size1, oob_size2;
- int reg_off = FLASH_BUF_ACC;
-
- data_size1 = mtd->writesize - host->cw_size * (ecc->steps - 1);
- oob_size1 = host->bbm_size;
-
- if (i == (ecc->steps - 1)) {
- data_size2 = ecc->size - data_size1 -
- ((ecc->steps - 1) << 2);
- oob_size2 = (ecc->steps << 2) + host->ecc_bytes_hw +
- host->spare_bytes;
- } else {
- data_size2 = host->cw_data - data_size1;
- oob_size2 = host->ecc_bytes_hw + host->spare_bytes;
- }
-
- if (nandc->props->is_bam) {
- read_loc = 0;
- nandc_set_read_loc(nandc, 0, read_loc, data_size1, 0);
- read_loc += data_size1;
-
- nandc_set_read_loc(nandc, 1, read_loc, oob_size1, 0);
- read_loc += oob_size1;
-
- nandc_set_read_loc(nandc, 2, read_loc, data_size2, 0);
- read_loc += data_size2;
-
- nandc_set_read_loc(nandc, 3, read_loc, oob_size2, 1);
- }
-
- config_nand_cw_read(nandc, false);
-
- read_data_dma(nandc, reg_off, data_buf, data_size1, 0);
- reg_off += data_size1;
- data_buf += data_size1;
-
- read_data_dma(nandc, reg_off, oob_buf, oob_size1, 0);
- reg_off += oob_size1;
- oob_buf += oob_size1;
-
- read_data_dma(nandc, reg_off, data_buf, data_size2, 0);
- reg_off += data_size2;
- data_buf += data_size2;
-
- read_data_dma(nandc, reg_off, oob_buf, oob_size2, 0);
- oob_buf += oob_size2;
- }
-
- ret = submit_descs(nandc);
- free_descs(nandc);
- if (ret) {
- dev_err(nandc->dev, "failure to read raw page\n");
- return ret;
+ data_buf += host->cw_data;
+ oob_buf += ecc->bytes;
}
- return check_flash_errors(host, ecc->steps);
+ return 0;
}
/* implements ecc->read_oob() */
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
QCOM NAND controller layout protects available OOB data bytes with
ECC also so when ecc->write_oob() is being called then it
can't update just OOB bytes. Currently, it first reads the last
codeword which includes old OOB bytes. Then it updates the old OOB
bytes with new ones and then again writes the codeword back.
The reading codeword is unnecessary since user is responsible to
have these bytes cleared to 0xFF.
This patch removes the read part and updates the OOB bytes with
data area padded with OxFF.
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
NONE
* Changes from v2:
1. Changed commit message and comments slightly
2. Changed host->use_ecc assignment place
* Changes from v1:
NEW CHANGE
drivers/mtd/nand/raw/qcom_nandc.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 285b2ad3..28361b5 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2064,11 +2064,9 @@ static int qcom_nandc_write_page_raw(struct mtd_info *mtd,
/*
* implements ecc->write_oob()
*
- * the NAND controller cannot write only data or only oob within a codeword,
- * since ecc is calculated for the combined codeword. we first copy the
- * entire contents for the last codeword(data + oob), replace the old oob
- * with the new one in chip->oob_poi, and then write the entire codeword.
- * this read-copy-write operation results in a slight performance loss.
+ * the NAND controller cannot write only data or only OOB within a codeword
+ * since ECC is calculated for the combined codeword. So update the OOB from
+ * chip->oob_poi, and pad the data area with OxFF before writing.
*/
static int qcom_nandc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
int page)
@@ -2081,19 +2079,13 @@ static int qcom_nandc_write_oob(struct mtd_info *mtd, struct nand_chip *chip,
int ret;
host->use_ecc = true;
-
- clear_bam_transaction(nandc);
- ret = copy_last_cw(host, page);
- if (ret)
- return ret;
-
- clear_read_regs(nandc);
clear_bam_transaction(nandc);
/* calculate the data and oob size for the last codeword/step */
data_size = ecc->size - ((ecc->steps - 1) << 2);
oob_size = mtd->oobavail;
+ memset(nandc->data_buffer, 0xff, host->cw_data);
/* override new oob content to last codeword */
mtd_ooblayout_get_databytes(mtd, nandc->data_buffer + data_size, oob,
0, mtd->oobavail);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
The BAM has 3 channels - tx, rx and command. command channel
is used for register read/writes, tx channel for data writes
and rx channel for data reads. Currently, the driver assumes the
transfer completion once it gets all the command descriptors
completed. Sometimes, there is race condition between data channel
(tx/rx) and command channel completion. In these cases,
the data present in buffer is not valid during small window
between command descriptor completion and data descriptor
completion.
This patch generates NAND transfer completion when both
(Data and Command) DMA channels have completed all its DMA
descriptors. It assigns completion callback in last
DMA descriptors of that channel and wait for completion.
Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
Cc: [email protected]
Acked-by: Miquel Raynal <[email protected]>
Signed-off-by: Abhishek Sahu <[email protected]>
---
* Changes from v3:
1. NONE
* Changes from v2:
1. Changed commit message and comments slightly
2. Renamed wait_second_completion from first_chan_done and set
it before submit desc
3. Mark for stable tree
* Changes from v1:
NONE
drivers/mtd/nand/raw/qcom_nandc.c | 53 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 2375780..fc20149 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -213,6 +213,8 @@
#define QPIC_PER_CW_CMD_SGL 32
#define QPIC_PER_CW_DATA_SGL 8
+#define QPIC_NAND_COMPLETION_TIMEOUT msecs_to_jiffies(2000)
+
/*
* Flags used in DMA descriptor preparation helper functions
* (i.e. read_reg_dma/write_reg_dma/read_data_dma/write_data_dma)
@@ -245,6 +247,11 @@
* @tx_sgl_start - start index in data sgl for tx.
* @rx_sgl_pos - current index in data sgl for rx.
* @rx_sgl_start - start index in data sgl for rx.
+ * @wait_second_completion - wait for second DMA desc completion before making
+ * the NAND transfer completion.
+ * @txn_done - completion for NAND transfer.
+ * @last_data_desc - last DMA desc in data channel (tx/rx).
+ * @last_cmd_desc - last DMA desc in command channel.
*/
struct bam_transaction {
struct bam_cmd_element *bam_ce;
@@ -258,6 +265,10 @@ struct bam_transaction {
u32 tx_sgl_start;
u32 rx_sgl_pos;
u32 rx_sgl_start;
+ bool wait_second_completion;
+ struct completion txn_done;
+ struct dma_async_tx_descriptor *last_data_desc;
+ struct dma_async_tx_descriptor *last_cmd_desc;
};
/*
@@ -504,6 +515,8 @@ static void free_bam_transaction(struct qcom_nand_controller *nandc)
bam_txn->data_sgl = bam_txn_buf;
+ init_completion(&bam_txn->txn_done);
+
return bam_txn;
}
@@ -523,11 +536,33 @@ static void clear_bam_transaction(struct qcom_nand_controller *nandc)
bam_txn->tx_sgl_start = 0;
bam_txn->rx_sgl_pos = 0;
bam_txn->rx_sgl_start = 0;
+ bam_txn->last_data_desc = NULL;
+ bam_txn->wait_second_completion = false;
sg_init_table(bam_txn->cmd_sgl, nandc->max_cwperpage *
QPIC_PER_CW_CMD_SGL);
sg_init_table(bam_txn->data_sgl, nandc->max_cwperpage *
QPIC_PER_CW_DATA_SGL);
+
+ reinit_completion(&bam_txn->txn_done);
+}
+
+/* Callback for DMA descriptor completion */
+static void qpic_bam_dma_done(void *data)
+{
+ struct bam_transaction *bam_txn = data;
+
+ /*
+ * In case of data transfer with NAND, 2 callbacks will be generated.
+ * One for command channel and another one for data channel.
+ * If current transaction has data descriptors
+ * (i.e. wait_second_completion is true), then set this to false
+ * and wait for second DMA descriptor completion.
+ */
+ if (bam_txn->wait_second_completion)
+ bam_txn->wait_second_completion = false;
+ else
+ complete(&bam_txn->txn_done);
}
static inline struct qcom_nand_host *to_qcom_nand_host(struct nand_chip *chip)
@@ -756,6 +791,12 @@ static int prepare_bam_async_desc(struct qcom_nand_controller *nandc,
desc->dma_desc = dma_desc;
+ /* update last data/command descriptor */
+ if (chan == nandc->cmd_chan)
+ bam_txn->last_cmd_desc = dma_desc;
+ else
+ bam_txn->last_data_desc = dma_desc;
+
list_add_tail(&desc->node, &nandc->desc_list);
return 0;
@@ -1273,10 +1314,20 @@ static int submit_descs(struct qcom_nand_controller *nandc)
cookie = dmaengine_submit(desc->dma_desc);
if (nandc->props->is_bam) {
+ bam_txn->last_cmd_desc->callback = qpic_bam_dma_done;
+ bam_txn->last_cmd_desc->callback_param = bam_txn;
+ if (bam_txn->last_data_desc) {
+ bam_txn->last_data_desc->callback = qpic_bam_dma_done;
+ bam_txn->last_data_desc->callback_param = bam_txn;
+ bam_txn->wait_second_completion = true;
+ }
+
dma_async_issue_pending(nandc->tx_chan);
dma_async_issue_pending(nandc->rx_chan);
+ dma_async_issue_pending(nandc->cmd_chan);
- if (dma_sync_wait(nandc->cmd_chan, cookie) != DMA_COMPLETE)
+ if (!wait_for_completion_timeout(&bam_txn->txn_done,
+ QPIC_NAND_COMPLETION_TIMEOUT))
return -ETIMEDOUT;
} else {
if (dma_sync_wait(nandc->chan, cookie) != DMA_COMPLETE)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc.
is a member of Code Aurora Forum, hosted by The Linux Foundation
On Wed, Jun 20, 2018 at 12:57:30PM +0530, Abhishek Sahu wrote:
> 1. If nand-ecc-strength specified in DT, then controller will use
> this ECC strength otherwise ECC strength will be calculated
> according to chip requirement and available OOB size.
>
> 2. QCOM NAND controller supports only one step size (512 bytes) but
> nand-ecc-step-size is required property in DT. This DT property
> can be removed and ecc step size can be assigned in driver with
> 512 bytes value.
>
> Signed-off-by: Abhishek Sahu <[email protected]>
> ---
>
> * Changes from v3:
>
> 1. Clubbed following 2 patches into one
> https://patchwork.ozlabs.org/patch/920465/
> https://patchwork.ozlabs.org/patch/920467/
>
> * Changes from v2:
> NONE
>
> * Changes from v1:
> NEW PATCH
>
> Documentation/devicetree/bindings/mtd/qcom_nandc.txt | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Rob Herring <[email protected]>
2018-06-20 16:27 GMT+09:00 Abhishek Sahu <[email protected]>:
> commit 2c8f8afa7f92 ("mtd: nand: add generic helpers to check,
> match, maximize ECC settings") provides generic helpers which
> drivers can use for setting up ECC parameters.
>
> Since same board can have different ECC strength nand chips so
> following is the logic for setting up ECC strength and ECC step
> size, which can be used by most of the drivers.
>
> 1. If both ECC step size and ECC strength are already set
> (usually by DT) then just check whether this setting
> is supported by NAND controller.
> 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength
> supported by NAND controller.
> 3. Otherwise, try to match the ECC step size and ECC strength closest
> to the chip's requirement. If available OOB size can't fit the chip
> requirement then select maximum ECC strength which can be fit with
> available OOB size.
>
> This patch introduces nand_ecc_choose_conf function which calls the
> required helper functions for the above logic. The drivers can use
> this single function instead of calling the 3 helper functions
> individually.
>
> CC: Masahiro Yamada <[email protected]>
You can replace the CC with my
Reviewed-by: Masahiro Yamada <[email protected]>
Thanks.
> Signed-off-by: Abhishek Sahu <[email protected]>
> ---
> * Changes from v3:
> 1. call nand_maximize_ecc() 2 times to make code more clear.
>
> * Changes from v2:
>
> 1. Renamed function to nand_ecc_choose_conf.
> 2. Minor code reorganization to remove warning and 2 function calls
> for nand_maximize_ecc.
>
> * Changes from v1:
> NEW PATCH
>
> drivers/mtd/nand/raw/nand_base.c | 33 +++++++++++++++++++++++++++++++++
> include/linux/mtd/rawnand.h | 3 +++
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 10c4f991..c64e3fc 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -6287,6 +6287,39 @@ int nand_maximize_ecc(struct nand_chip *chip,
> }
> EXPORT_SYMBOL_GPL(nand_maximize_ecc);
>
> +/**
> + * nand_ecc_choose_conf - Set the ECC strength and ECC step size
> + * @chip: nand chip info structure
> + * @caps: ECC engine caps info structure
> + * @oobavail: OOB size that the ECC engine can use
> + *
> + * Choose the ECC configuration according to following logic
> + *
> + * 1. If both ECC step size and ECC strength are already set (usually by DT)
> + * then check if it is supported by this controller.
> + * 2. If NAND_ECC_MAXIMIZE is set, then select maximum ECC strength.
> + * 3. Otherwise, try to match the ECC step size and ECC strength closest
> + * to the chip's requirement. If available OOB size can't fit the chip
> + * requirement then fallback to the maximum ECC step size and ECC strength.
> + *
> + * On success, the chosen ECC settings are set.
> + */
> +int nand_ecc_choose_conf(struct nand_chip *chip,
> + const struct nand_ecc_caps *caps, int oobavail)
> +{
> + if (chip->ecc.size && chip->ecc.strength)
> + return nand_check_ecc_caps(chip, caps, oobavail);
> +
> + if (chip->ecc.options & NAND_ECC_MAXIMIZE)
> + return nand_maximize_ecc(chip, caps, oobavail);
> +
> + if (!nand_match_ecc_req(chip, caps, oobavail))
> + return 0;
> +
> + return nand_maximize_ecc(chip, caps, oobavail);
> +}
> +EXPORT_SYMBOL_GPL(nand_ecc_choose_conf);
> +
> /*
> * Check if the chip configuration meet the datasheet requirements.
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 3e8ec3b..03a0061 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1650,6 +1650,9 @@ int nand_match_ecc_req(struct nand_chip *chip,
> int nand_maximize_ecc(struct nand_chip *chip,
> const struct nand_ecc_caps *caps, int oobavail);
>
> +int nand_ecc_choose_conf(struct nand_chip *chip,
> + const struct nand_ecc_caps *caps, int oobavail);
> +
> /* Default write_oob implementation */
> int nand_write_oob_std(struct mtd_info *mtd, struct nand_chip *chip, int page);
>
> --
> 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/
--
Best Regards
Masahiro Yamada
Hi Abhishek,
On Wed, 20 Jun 2018 12:57:41 +0530, Abhishek Sahu
<[email protected]> wrote:
> NAND parts can have bitflips in an erased page due to the
> process technology used. In this case, QCOM NAND controller
> is not able to identify that page as an erased page.
> Currently the driver calls nand_check_erased_ecc_chunk() for
> identifying the erased pages but this won’t work always since the
> checking is being with ECC engine returned data. In case of
> bitflips, the ECC engine tries to correct the data and then it
> generates the uncorrectable error. Now, this data is not equal to
> original raw data. For erased CW identification, the raw data
> should be read again from NAND device and this
> nand_check_erased_ecc_chunk function() should be called for raw
> data only.
>
> Now following logic is being added to identify the erased
> codeword bitflips.
>
> 1. In most of the cases, not all the codewords will have bitflips
> and only single CW will have bitflips. So, there is no need to
> read the complete raw page data. The NAND raw read can be
> scheduled for any CW in page. The NAND controller works on CW
> basis and it will update the status register after each CW read.
> Maintain the bitmask for the CW which generated the uncorrectable
> error.
> 2. Do raw read for all the CW's which generated the uncorrectable
> error.
> 3. Both DATA and OOB need to be checked for number of 0. The
> top-level API can be called with only data buf or OOB buf so use
> chip->databuf if data buf is null and chip->oob_poi if
> OOB buf is null for copying the raw bytes temporarily.
> 4. For each CW, check the number of 0 in cw_data and usable
> oob bytes, The bbm and spare (unused) bytes bit flip won’t
> affect the ECC so don’t check the number of bitflips in this area.
>
> Signed-off-by: Abhishek Sahu <[email protected]>
> ---
> * Changes from v3:
>
> 1. Major changes in erased codeword detection for
> raw read function
I really prefer this version, much more readable from my point of view!
>
> * Changes from v2:
> NONE
>
> * Changes from v1:
> 1. Minor change in commit message
> 2. invalidate pagebuf if databuf or oob_poi is used
>
> drivers/mtd/nand/raw/qcom_nandc.c | 127 +++++++++++++++++++++++++++-----------
> 1 file changed, 90 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 160acdf..e34edf1 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -1656,20 +1656,95 @@ static int check_flash_errors(struct qcom_nand_host *host, int cw_cnt)
> }
>
> /*
> + * Bitflips can happen in erased codewords also so this function counts the
> + * number of 0 in each CW for which ECC engine returns the uncorrectable
> + * error. The page will be assumed as erased if this count is less than or
> + * equal to the ecc->strength for each CW.
> + *
> + * 1. Both DATA and OOB need to be checked for number of 0. The
> + * top-level API can be called with only data buf or OOB buf so use
> + * chip->data_buf if data buf is null and chip->oob_poi if oob buf
> + * is null for copying the raw bytes.
> + * 2. Perform raw read for all the CW which has uncorrectable errors.
> + * 3. For each CW, check the number of 0 in cw_data and usable OOB bytes.
> + * The BBM and spare bytes bit flip won’t affect the ECC so don’t check
> + * the number of bitflips in this area.
> + */
> +static int
> +check_for_erased_page(struct qcom_nand_host *host, u8 *data_buf,
> + u8 *oob_buf, unsigned long uncorrectable_cws,
> + int page, unsigned int max_bitflips)
> +{
> + struct nand_chip *chip = &host->chip;
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + struct nand_ecc_ctrl *ecc = &chip->ecc;
> + int cw, data_size, oob_size, ret = 0;
> +
> + if (!data_buf) {
> + data_buf = chip->data_buf;
> + chip->pagebuf = -1;
> + }
> +
> + if (!oob_buf) {
> + oob_buf = chip->oob_poi;
> + chip->pagebuf = -1;
> + }
> +
> + for (cw = 0; cw < ecc->steps && uncorrectable_cws; cw++) {
Last nitpick:
Could you have a look to bitmap.c and bitops.h and use a
for_each_set_bit() loop?
No need to resend all the patches, you can send a v5 just for this
patch, the others are fine for me.
Thanks,
Miquèl
Hi Abhishek,
Abhishek Sahu <[email protected]> wrote on Wed, 20 Jun 2018
12:57:27 +0530:
> * v4:
>
> 1. Added patch to make other ECC configurations function static.
> 2. Clubbed the DT update patches.
> 3. Removed the bad block related patch. Discussion is going on
> related with for proper solution so planning to submit separate
> patch series for all bad block related changes.
> 4. Made the single codeword raw read function and used the same
> for raw page read.
> 5. Changes in erased codeword detection to raw read function.
>
> * v3:
>
> 1. Addressed all review comments in v2.
> 2. Added patch for removing redundant nand-ecc-step-size DT property.
> 3. Renamed ECC configuration setup function with minor code changes.
> 4. Modified comments and commit message for few patches.
>
> * v2:
>
> 1. Addressed all review comments in v1.
> 1. Make the generic helper function for NAND ECC parameters setup
> and used this helper function for QCOM and Denali nand driver
> for ECC setup.
> 2. Modified commit message for some of the patches and added more
> comments.
> 3. Added new patch for fixing ‘return 0’ for raw read.
> 4. Removed the read last codeword part for nand oob write.
> 5. Reorganized bad block check function and removed the
> read_last_cw function completely.
>
> * v1:
>
> This patch series mainly deals with error handling and erased page
> bitflip detection for QCOM NAND driver.
>
> 1. The error handling was missing for some of the cases so fixed
> the same.
>
> 2. Add the support for taking ECC strength from ONFI parameter.
> The earlier QCOM boards were coming with 4-bit ECC chip but
> now the same boards are coming with 8-bit ECC chip since the
> earlier 4-bit parts are obsolete from some vendors.
>
> 3. We got few issues related with NAND erased page bitflips. The
> QCOM NAND controller can’t detect the bitflip in completely erased
> page so added the support to detect the same. It implemented the
> logic mentioned in patch [1] which didn’t go in mainline and later
> the generic functions were provided [2] to count the number of
> bitflips and make all 0xff. This patch series did some optimization
> logic to prevent the unnecessary full page raw read and data copy
> from QCOM NAND controller to DMA.
>
> 4. Following are the testing done for these patches in QCOM IPQ8074
> HK01 (4-bit and 8-bit ECC chip) and IPQ806x AP148 boards.
> a. Run all mtd test and check if it passes
> b. Introduce custom bitflips in erased page and check if it
> returns no error/EUCLEAN/EBADMSG depending upon number of
> bitflips and position.
> c. Introduce failure condition for operational failure and
> check if it detects the same.
>
> [1]: https://patchwork.ozlabs.org/patch/328994/
> [2]: https://patchwork.ozlabs.org/patch/509970/
>
> Abhishek Sahu (15):
> mtd: rawnand: helper function for setting up ECC configuration
> mtd: rawnand: denali: use helper function for ecc setup
> dt-bindings: qcom_nandc: update for ECC strength and step size
> mtd: rawnand: qcom: remove dt property nand-ecc-step-size
> mtd: rawnand: qcom: use the ecc strength from device parameter
> mtd: rawnand: qcom: wait for desc completion in all BAM channels
> mtd: rawnand: qcom: erased page detection for uncorrectable errors
> only
> mtd: rawnand: qcom: fix null pointer access for erased page detection
> mtd: rawnand: qcom: parse read errors for read oob also
> mtd: rawnand: qcom: modify write_oob to remove read codeword part
> mtd: rawnand: qcom: fix return value for raw page read
> mtd: rawnand: qcom: check for operation errors in case of raw read
> mtd: rawnand: qcom: code reorganization for raw read
> mtd: rawnand: qcom: erased page bitflips detection
> mtd: rawnand: provide only single helper function for ECC conf
>
> .../devicetree/bindings/mtd/qcom_nandc.txt | 7 +-
> drivers/mtd/nand/raw/denali.c | 30 +-
> drivers/mtd/nand/raw/nand_base.c | 72 ++-
> drivers/mtd/nand/raw/qcom_nandc.c | 491 ++++++++++++++-------
> include/linux/mtd/rawnand.h | 10 +-
> 5 files changed, 380 insertions(+), 230 deletions(-)
>
Thank you very much for the series and the changes you have done.
Applied all patches but "erased page bitflips detection" because I'm
waiting for one small modification on this one.
Thanks,
Miquèl
On 2018-07-01 23:39, Miquel Raynal wrote:
> Hi Abhishek,
>
> Abhishek Sahu <[email protected]> wrote on Wed, 20 Jun 2018
> 12:57:27 +0530:
>
>> * v4:
>>
>> 1. Added patch to make other ECC configurations function static.
>> 2. Clubbed the DT update patches.
>> 3. Removed the bad block related patch. Discussion is going on
>> related with for proper solution so planning to submit separate
>> patch series for all bad block related changes.
>> 4. Made the single codeword raw read function and used the same
>> for raw page read.
>> 5. Changes in erased codeword detection to raw read function.
>>
>> * v3:
>>
>> 1. Addressed all review comments in v2.
>> 2. Added patch for removing redundant nand-ecc-step-size DT property.
>> 3. Renamed ECC configuration setup function with minor code changes.
>> 4. Modified comments and commit message for few patches.
>>
>> * v2:
>>
>> 1. Addressed all review comments in v1.
>> 1. Make the generic helper function for NAND ECC parameters setup
>> and used this helper function for QCOM and Denali nand driver
>> for ECC setup.
>> 2. Modified commit message for some of the patches and added more
>> comments.
>> 3. Added new patch for fixing ‘return 0’ for raw read.
>> 4. Removed the read last codeword part for nand oob write.
>> 5. Reorganized bad block check function and removed the
>> read_last_cw function completely.
>>
>> * v1:
>>
>> This patch series mainly deals with error handling and erased page
>> bitflip detection for QCOM NAND driver.
>>
>> 1. The error handling was missing for some of the cases so fixed
>> the same.
>>
>> 2. Add the support for taking ECC strength from ONFI parameter.
>> The earlier QCOM boards were coming with 4-bit ECC chip but
>> now the same boards are coming with 8-bit ECC chip since the
>> earlier 4-bit parts are obsolete from some vendors.
>>
>> 3. We got few issues related with NAND erased page bitflips. The
>> QCOM NAND controller can’t detect the bitflip in completely erased
>> page so added the support to detect the same. It implemented the
>> logic mentioned in patch [1] which didn’t go in mainline and later
>> the generic functions were provided [2] to count the number of
>> bitflips and make all 0xff. This patch series did some optimization
>> logic to prevent the unnecessary full page raw read and data copy
>> from QCOM NAND controller to DMA.
>>
>> 4. Following are the testing done for these patches in QCOM IPQ8074
>> HK01 (4-bit and 8-bit ECC chip) and IPQ806x AP148 boards.
>> a. Run all mtd test and check if it passes
>> b. Introduce custom bitflips in erased page and check if it
>> returns no error/EUCLEAN/EBADMSG depending upon number of
>> bitflips and position.
>> c. Introduce failure condition for operational failure and
>> check if it detects the same.
>>
>> [1]: https://patchwork.ozlabs.org/patch/328994/
>> [2]: https://patchwork.ozlabs.org/patch/509970/
>>
>> Abhishek Sahu (15):
>> mtd: rawnand: helper function for setting up ECC configuration
>> mtd: rawnand: denali: use helper function for ecc setup
>> dt-bindings: qcom_nandc: update for ECC strength and step size
>> mtd: rawnand: qcom: remove dt property nand-ecc-step-size
>> mtd: rawnand: qcom: use the ecc strength from device parameter
>> mtd: rawnand: qcom: wait for desc completion in all BAM channels
>> mtd: rawnand: qcom: erased page detection for uncorrectable errors
>> only
>> mtd: rawnand: qcom: fix null pointer access for erased page
>> detection
>> mtd: rawnand: qcom: parse read errors for read oob also
>> mtd: rawnand: qcom: modify write_oob to remove read codeword part
>> mtd: rawnand: qcom: fix return value for raw page read
>> mtd: rawnand: qcom: check for operation errors in case of raw read
>> mtd: rawnand: qcom: code reorganization for raw read
>> mtd: rawnand: qcom: erased page bitflips detection
>> mtd: rawnand: provide only single helper function for ECC conf
>>
>> .../devicetree/bindings/mtd/qcom_nandc.txt | 7 +-
>> drivers/mtd/nand/raw/denali.c | 30 +-
>> drivers/mtd/nand/raw/nand_base.c | 72 ++-
>> drivers/mtd/nand/raw/qcom_nandc.c | 491
>> ++++++++++++++-------
>> include/linux/mtd/rawnand.h | 10 +-
>> 5 files changed, 380 insertions(+), 230 deletions(-)
>>
>
> Thank you very much for the series and the changes you have done.
>
Thanks a lot Miquel for your great support and help in
reviewing and merging these patches :-)
> Applied all patches but "erased page bitflips detection" because I'm
> waiting for one small modification on this one.
Sure. I will send that updated patch.
Regards,
Abhishek