* [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes
@ 2014-03-18 13:26 Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 1/5] mtd: nand: omap: add field to indicate current ecc-scheme in 'struct omap_nand_info' Pekon Gupta
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
*changes v8 -> v9*
[PATCH v9 1/5] <no change>
[PATCH v9 2/5] <no change>
[PATCH v9 3/5]
- dropped earlier versions [PATCH v8 3/6] and PATCH v8 4/6], as per feedbacks from
MTD maintainer 'Brian Norris <computersforpeace@gmail.com>'
http://lists.infradead.org/pipermail/linux-mtd/2014-March/052472.html
- Current patch uses existing method of comparing calc_ecc[] with pre-known ECC(all 0xff data + OOB)
to identify erased-pages. Thereby removing dependency on 'reserved-marker'.
[PATCH v9 4/5] <same as [PATCH v8 5/6]>
[PATCH v9 5/5] <same as [PATCH v8 6/6]>
*changes v7 -> v8*
[PATCH v8 1/6] <no change>
[PATCH v8 2/6] <minor cleanup>
[PATCH v8 3/6]
- use reference instead of local variables as suggested by Brian Norris <computersforpeace@gmail.com>
- fix: report bit-flips in DATA region for 'confirmed' erased-page
[PATCH v8 4/6] <minor cleanup>
[PATCH v8 5/6] <no change>
[PATCH v8 6/6] <no change>
*changes v6 -> v7*
[PATCH v7 1/6] <no change>
[PATCH v7 2/6] <no change>
[PATCH v7 3/6] <dropped older version, refer commit message>
- erased-page detection done based on count of zero-bits in OOB & DATA
- incorporated some feedbacks from Brian Norris <computersforpeace@gmail.com>
http://lists.infradead.org/pipermail/linux-mtd/2014-January/051533.html
http://lists.infradead.org/pipermail/linux-mtd/2014-January/051534.html
[PATCH v7 4/6] <new> remove redundant bit-flip counting for erased-page
[PATCH v7 5/6] <minor cleanup>
[PATCH v7 6/6] <minor cleanup>
(As there are functional changes in this version, so not attaching
Tested-by: Stefan Roese <sr@denx.de>)
*changes v5 -> v6*
[PATCH v6 1/6] <no change>
[PATCH v6 2/6] introduced variable 'actual_eccbytes' to omit reserved byte-position
[PATCH v6 3/6] split from [PATCH v5 3/5] fix erased-page bit-flip correction
[PATCH v6 4/6] split from [PATCH v5 3/5] fix erased-page detection
[PATCH v6 5/6] <minor cleanup>
[PATCH v6 6/6] <no change>
*changes v4 -> v5*
This patch series was split version of earlier patch:
http://lists.infradead.org/pipermail/linux-mtd/2013-November/050241.html
chip->ecc.correct() is used for detecting and correcting bit-flips during read
operations. In OMAP NAND driver different ecc-schemes have different callbacks:
- omap_correct_data() for HAM1_HW ecc-schemes (Untouched)
- nand_bch_correct_data() for BCHx_HW_DETECTION_SW ecc-schemes (Untouched)
- omap_elm_correct_data() for BCHx_HW ecc-schemes
This patch-series fixes following issues in omap_elm_correct_data():
(1) Dependency on a specific reserved byte-position in OOB area
to differentiates between erased-pages v/s programmed-pages.
Problem: reserved byte-position cannot be accomodated in all ecc-schemes
Problem: reserved byte-position can itself be subjected upto 8 bit-flips
causing the 0xff to become 0x00, causing page to be
mis-recognized as erased-page.
(2) Bit-flips in erased-pages are detected by comparing each byte of Data & OOB
with 0xff in check_erased_page().
Problem: This is causes performance penalty when erased-pages are checked.
(3) Current code is not scalable for future ECC schemes due to presence of
tweaks for BCH4_ECC and BCH8_ECC at multiple places.
(4) Currently, bit-flips are evaluated and fixed even when ELM reports them as
un-correctable bit-flips, this should not happen as 'number-of-error' field
in ELM_LOCATION_STATUS becomes invalid when un-correctable flag is set.
(5) Driver should return with error-code = '-EBADMSG' when
uncorrectable bit-flip is detected
bit-flip outside valid Data and OOB region is detected
Pekon Gupta (5):
mtd: nand: omap: add field to indicate current ecc-scheme in 'struct
omap_nand_info'
mtd: nand: omap: ecc.correct: omap_elm_correct_data: rename ambiguous
variable 'eccsize' and 'ecc_vector_size'
mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page
detection for BCHx_HW ECC schemes
mtd: nand: omap: ecc.correct: omap_elm_correct_data: cleanup for
future enhancements
mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix
programmed-page bit-flip correction logic
drivers/mtd/nand/omap2.c | 187 ++++++++++++++++++++++-------------------------
1 file changed, 87 insertions(+), 100 deletions(-)
--
1.8.5.1.163.gd7aced9
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v9 1/5] mtd: nand: omap: add field to indicate current ecc-scheme in 'struct omap_nand_info'
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
@ 2014-03-18 13:26 ` Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 2/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: rename ambiguous variable 'eccsize' and 'ecc_vector_size' Pekon Gupta
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
Information of currently selected ECC scheme 'enum omap_ecc ecc_opt' should
available outside platform-data, so that single nand_chip->ecc callback can
support multiple ecc-scheme configurations.
Tested-by: Stefan Roese <sr@denx.de>
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index bf642ce..403afa7 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -160,6 +160,7 @@ struct omap_nand_info {
int gpmc_cs;
unsigned long phys_base;
unsigned long mem_size;
+ enum omap_ecc ecc_opt;
struct completion comp;
struct dma_chan *dma;
int gpmc_irq_fifo;
@@ -1657,6 +1658,7 @@ static int omap_nand_probe(struct platform_device *pdev)
info->gpmc_cs = pdata->cs;
info->reg = pdata->reg;
info->of_node = pdata->of_node;
+ info->ecc_opt = pdata->ecc_opt;
mtd = &info->mtd;
mtd->priv = &info->nand;
mtd->name = dev_name(&pdev->dev);
@@ -1812,7 +1814,7 @@ static int omap_nand_probe(struct platform_device *pdev)
/* populate MTD interface based on ECC scheme */
nand_chip->ecc.layout = &omap_oobinfo;
ecclayout = &omap_oobinfo;
- switch (pdata->ecc_opt) {
+ switch (info->ecc_opt) {
case OMAP_ECC_HAM1_CODE_HW:
pr_info("nand: using OMAP_ECC_HAM1_CODE_HW\n");
nand_chip->ecc.mode = NAND_ECC_HW;
--
1.8.5.1.163.gd7aced9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v9 2/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: rename ambiguous variable 'eccsize' and 'ecc_vector_size'
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 1/5] mtd: nand: omap: add field to indicate current ecc-scheme in 'struct omap_nand_info' Pekon Gupta
@ 2014-03-18 13:26 ` Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes Pekon Gupta
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
renaming following variables as they cause confusion due to resemblence to
another similar field in 'struct nand_ecc_ctrl' (nand_chip->ecc.size).
renaming: ecc_vector_size --> ecc->bytes (info->nand.ecc.bytes)
renaming: eccsize --> actual_eccbytes (info->nand.ecc.bytes - 1) for BCH4 and BCH8
Tested-by: Stefan Roese <sr@denx.de>
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 403afa7..4bf2f76d 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1359,9 +1359,10 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
{
struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
mtd);
+ struct nand_ecc_ctrl *ecc = &info->nand.ecc;
int eccsteps = info->nand.ecc.steps;
int i , j, stat = 0;
- int eccsize, eccflag, ecc_vector_size;
+ int eccflag, actual_eccbytes;
struct elm_errorvec err_vec[ERROR_VECTOR_MAX];
u_char *ecc_vec = calc_ecc;
u_char *spare_ecc = read_ecc;
@@ -1369,6 +1370,20 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
enum bch_ecc type;
bool is_error_reported = false;
+ switch (info->ecc_opt) {
+ case OMAP_ECC_BCH4_CODE_HW:
+ /* omit 7th ECC byte reserved for ROM code compatibility */
+ actual_eccbytes = ecc->bytes - 1;
+ break;
+ case OMAP_ECC_BCH8_CODE_HW:
+ /* omit 14th ECC byte reserved for ROM code compatibility */
+ actual_eccbytes = ecc->bytes - 1;
+ break;
+ default:
+ pr_err("invalid driver configuration\n");
+ return -EINVAL;
+ }
+
/* Initialize elm error vector to zero */
memset(err_vec, 0, sizeof(err_vec));
@@ -1380,14 +1395,6 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
erased_ecc_vec = bch4_vector;
}
- ecc_vector_size = info->nand.ecc.bytes;
-
- /*
- * Remove extra byte padding for BCH8 RBL
- * compatibility and erased page handling
- */
- eccsize = ecc_vector_size - 1;
-
for (i = 0; i < eccsteps ; i++) {
eccflag = 0; /* initialize eccflag */
@@ -1395,8 +1402,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
* Check any error reported,
* In case of error, non zero ecc reported.
*/
-
- for (j = 0; (j < eccsize); j++) {
+ for (j = 0; j < actual_eccbytes; j++) {
if (calc_ecc[j] != 0) {
eccflag = 1; /* non zero ecc, error present */
break;
@@ -1421,7 +1427,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
* zeros are more than threshold erased page, either
* case page reported as uncorrectable.
*/
- if (hweight8(~read_ecc[eccsize]) >= threshold) {
+ if (hweight8(~read_ecc[actual_eccbytes]) >= threshold) {
/*
* Update elm error vector as
* data area is programmed
@@ -1433,7 +1439,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
int bitflip_count;
u_char *buf = &data[info->nand.ecc.size * i];
- if (memcmp(calc_ecc, erased_ecc_vec, eccsize)) {
+ if (memcmp(calc_ecc, erased_ecc_vec,
+ actual_eccbytes)) {
bitflip_count = erased_sector_bitflips(
buf, read_ecc, info);
@@ -1446,8 +1453,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
}
/* Update the ecc vector */
- calc_ecc += ecc_vector_size;
- read_ecc += ecc_vector_size;
+ calc_ecc += ecc->bytes;
+ read_ecc += ecc->bytes;
}
/* Check if any error reported */
@@ -1496,7 +1503,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
/* Update page data with sector size */
data += info->nand.ecc.size;
- spare_ecc += ecc_vector_size;
+ spare_ecc += ecc->bytes;
}
for (i = 0; i < eccsteps; i++)
--
1.8.5.1.163.gd7aced9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 1/5] mtd: nand: omap: add field to indicate current ecc-scheme in 'struct omap_nand_info' Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 2/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: rename ambiguous variable 'eccsize' and 'ecc_vector_size' Pekon Gupta
@ 2014-03-18 13:26 ` Pekon Gupta
2014-03-20 9:27 ` Brian Norris
2014-03-18 13:26 ` [PATCH v9 4/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: cleanup for future enhancements Pekon Gupta
` (2 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
As erased-pages do not have ECC stored in their OOB area, so they need to be
seperated out from programmed-pages, before doing BCH ECC correction.
In current implementation of omap_elm_correct_data() which does ECC correction
for BCHx ECC schemes, this erased-pages are detected based on specific marker
byte (reserved as 0x00) in ecc-layout.
However, this approach has some limitation like;
1) All ecc-scheme layouts do not have such Reserved byte marker to
differentiate between erased-page v/s programmed-page. Thus this is a
customized solution.
2) Reserved marker byte can itself be subjected to bit-flips causing
erased-page to be misunderstood as programmed-page.
This patch removes dependency on any marker byte in ecc-layout, instead it
compares calc_ecc[] with pattern of ECC-of-all(0xff). This implicitely
means that both 'data + oob == all(0xff).
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 85 +++++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 52 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 4bf2f76d..a6c979f 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1338,21 +1338,8 @@ static int erased_sector_bitflips(u_char *data, u_char *oob,
* @calc_ecc: ecc read from HW ECC registers
*
* Calculated ecc vector reported as zero in case of non-error pages.
- * In case of error/erased pages non-zero error vector is reported.
- * In case of non-zero ecc vector, check read_ecc at fixed offset
- * (x = 13/7 in case of BCH8/4 == 0) to find page programmed or not.
- * To handle bit flips in this data, count the number of 0's in
- * read_ecc[x] and check if it greater than 4. If it is less, it is
- * programmed page, else erased page.
- *
- * 1. If page is erased, check with standard ecc vector (ecc vector
- * for erased page to find any bit flip). If check fails, bit flip
- * is present in erased page. Count the bit flips in erased page and
- * if it falls under correctable level, report page with 0xFF and
- * update the correctable bit information.
- * 2. If error is reported on programmed page, update elm error
- * vector and correct the page with ELM error correction routine.
- *
+ * In case of non-zero ecc vector, first filter out erased-pages, and
+ * then process data via ELM to detect bit-flips.
*/
static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
u_char *read_ecc, u_char *calc_ecc)
@@ -1367,6 +1354,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
u_char *ecc_vec = calc_ecc;
u_char *spare_ecc = read_ecc;
u_char *erased_ecc_vec;
+ u_char *buf;
+ int bitflip_count;
enum bch_ecc type;
bool is_error_reported = false;
@@ -1374,10 +1363,12 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
case OMAP_ECC_BCH4_CODE_HW:
/* omit 7th ECC byte reserved for ROM code compatibility */
actual_eccbytes = ecc->bytes - 1;
+ erased_ecc_vec = bch4_vector;
break;
case OMAP_ECC_BCH8_CODE_HW:
/* omit 14th ECC byte reserved for ROM code compatibility */
actual_eccbytes = ecc->bytes - 1;
+ erased_ecc_vec = bch8_vector;
break;
default:
pr_err("invalid driver configuration\n");
@@ -1389,10 +1380,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
if (info->nand.ecc.strength == BCH8_MAX_ERROR) {
type = BCH8_ECC;
- erased_ecc_vec = bch8_vector;
} else {
type = BCH4_ECC;
- erased_ecc_vec = bch4_vector;
}
for (i = 0; i < eccsteps ; i++) {
@@ -1410,44 +1399,36 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
}
if (eccflag == 1) {
- /*
- * Set threshold to minimum of 4, half of ecc.strength/2
- * to allow max bit flip in byte to 4
- */
- unsigned int threshold = min_t(unsigned int, 4,
- info->nand.ecc.strength / 2);
-
- /*
- * Check data area is programmed by counting
- * number of 0's at fixed offset in spare area.
- * Checking count of 0's against threshold.
- * In case programmed page expects at least threshold
- * zeros in byte.
- * If zeros are less than threshold for programmed page/
- * zeros are more than threshold erased page, either
- * case page reported as uncorrectable.
- */
- if (hweight8(~read_ecc[actual_eccbytes]) >= threshold) {
+ if (memcmp(calc_ecc, erased_ecc_vec,
+ actual_eccbytes) == 0) {
/*
- * Update elm error vector as
- * data area is programmed
+ * calc_ecc[] matches pattern for ECC(all 0xff)
+ * so this is definitely an erased-page
*/
- err_vec[i].error_reported = true;
- is_error_reported = true;
} else {
- /* Error reported in erased page */
- int bitflip_count;
- u_char *buf = &data[info->nand.ecc.size * i];
-
- if (memcmp(calc_ecc, erased_ecc_vec,
- actual_eccbytes)) {
- bitflip_count = erased_sector_bitflips(
- buf, read_ecc, info);
-
- if (bitflip_count)
- stat += bitflip_count;
- else
- return -EINVAL;
+ buf = &data[info->nand.ecc.size * i];
+ /*
+ * count number of 0-bits in read_buf.
+ * This check can be removed once a similar
+ * check is introduced in generic NAND driver
+ */
+ bitflip_count = erased_sector_bitflips(
+ buf, read_ecc, info);
+ if (bitflip_count) {
+ /*
+ * number of 0-bits within ECC limits
+ * So this may be an erased-page
+ */
+ stat += bitflip_count;
+ } else {
+ /*
+ * Too many 0-bits. It may be a
+ * - programmed-page, OR
+ * - erased-page with many bit-flips
+ * So this page requires check by ELM
+ */
+ err_vec[i].error_reported = true;
+ is_error_reported = true;
}
}
}
--
1.8.5.1.163.gd7aced9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v9 4/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: cleanup for future enhancements
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
` (2 preceding siblings ...)
2014-03-18 13:26 ` [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes Pekon Gupta
@ 2014-03-18 13:26 ` Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 5/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix programmed-page bit-flip correction logic Pekon Gupta
2014-03-20 9:31 ` [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Brian Norris
5 siblings, 0 replies; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
Current omap_elm_correct_data() code is not scalable for future ecc-schemes
due to presence of tweaks and hard-coded macros for BCH4_ECC and BCH8_ECC
ecc-schemes at multiple places.
This patch:
- replaces 'ecc_opt' with '(info->nand.ecc.strength == BCH8_MAX_ERROR)
used to differentiate between BCH8_HW and BCH4_SW
- replaces macros (defining magic number for specific ecc-scheme) with
generic variables
- removes dependency on macros defined in elm.h (like BCHx_ECC_OOB_BYTES)
Tested-by: Stefan Roese <sr@denx.de>
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 38 +++++++++++++-------------------------
1 file changed, 13 insertions(+), 25 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index a6c979f..39e3f50 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -118,14 +118,9 @@
#define OMAP24XX_DMA_GPMC 4
-#define BCH8_MAX_ERROR 8 /* upto 8 bit correctable */
-#define BCH4_MAX_ERROR 4 /* upto 4 bit correctable */
-
#define SECTOR_BYTES 512
/* 4 bit padding to make byte aligned, 56 = 52 + 4 */
#define BCH4_BIT_PAD 4
-#define BCH8_ECC_MAX ((SECTOR_BYTES + BCH8_ECC_OOB_BYTES) * 8)
-#define BCH4_ECC_MAX ((SECTOR_BYTES + BCH4_ECC_OOB_BYTES) * 8)
/* GPMC ecc engine settings for read */
#define BCH_WRAPMODE_1 1 /* BCH wrap mode 1 */
@@ -1356,8 +1351,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
u_char *erased_ecc_vec;
u_char *buf;
int bitflip_count;
- enum bch_ecc type;
bool is_error_reported = false;
+ u32 bit_pos, byte_pos, error_max, pos;
switch (info->ecc_opt) {
case OMAP_ECC_BCH4_CODE_HW:
@@ -1378,12 +1373,6 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
/* Initialize elm error vector to zero */
memset(err_vec, 0, sizeof(err_vec));
- if (info->nand.ecc.strength == BCH8_MAX_ERROR) {
- type = BCH8_ECC;
- } else {
- type = BCH4_ECC;
- }
-
for (i = 0; i < eccsteps ; i++) {
eccflag = 0; /* initialize eccflag */
@@ -1448,20 +1437,19 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
for (i = 0; i < eccsteps; i++) {
if (err_vec[i].error_reported) {
for (j = 0; j < err_vec[i].error_count; j++) {
- u32 bit_pos, byte_pos, error_max, pos;
-
- if (type == BCH8_ECC)
- error_max = BCH8_ECC_MAX;
- else
- error_max = BCH4_ECC_MAX;
-
- if (info->nand.ecc.strength == BCH8_MAX_ERROR)
- pos = err_vec[i].error_loc[j];
- else
- /* Add 4 to take care 4 bit padding */
+ switch (info->ecc_opt) {
+ case OMAP_ECC_BCH4_CODE_HW:
+ /* Add 4 bits to take care of padding */
pos = err_vec[i].error_loc[j] +
BCH4_BIT_PAD;
-
+ break;
+ case OMAP_ECC_BCH8_CODE_HW:
+ pos = err_vec[i].error_loc[j];
+ break;
+ default:
+ return -EINVAL;
+ }
+ error_max = (ecc->size + actual_eccbytes) * 8;
/* Calculate bit position of error */
bit_pos = pos % 8;
@@ -1483,7 +1471,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
stat += err_vec[i].error_count;
/* Update page data with sector size */
- data += info->nand.ecc.size;
+ data += ecc->size;
spare_ecc += ecc->bytes;
}
--
1.8.5.1.163.gd7aced9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v9 5/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix programmed-page bit-flip correction logic
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
` (3 preceding siblings ...)
2014-03-18 13:26 ` [PATCH v9 4/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: cleanup for future enhancements Pekon Gupta
@ 2014-03-18 13:26 ` Pekon Gupta
2014-03-20 9:31 ` [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Brian Norris
5 siblings, 0 replies; 9+ messages in thread
From: Pekon Gupta @ 2014-03-18 13:26 UTC (permalink / raw)
To: Brian Norris
Cc: Stefan Roese, linux-mtd, Pekon Gupta, Ezequiel Garcia, Artem Bityutskiy
This patch updates following checks when bit-flips are detected by ELM:
- Do not evaluate bit-flips when un-correctable bit-flips is reported by ELM,
because as per [1] when ELM reports an un-correctable bit-flips,
'number of error' field in its ELM_LOCATION_STATUS register is also invalid.
- Return with error-code '-EBADMSG' on detection of un-correctable bit-flip.
- Return with error-code '-EBADMSG' when bit-flips position is outside current
Sector and OOB area.
[1] ELM IP spec Table-25 ELM_LOCATION_STATUS Register.
ELM_LOCATION_STATUS[8] = ECC_CORRECTABLE: Error location process exit status
0x0: ECC error location process failed.
Number of errors and error locations are invalid.
0x1: all errors were successfully located.
Number of errors and error locations are valid.
Tested-by: Stefan Roese <sr@denx.de>
Signed-off-by: Pekon Gupta <pekon@ti.com>
---
drivers/mtd/nand/omap2.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 39e3f50..4f067f0 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1353,6 +1353,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
int bitflip_count;
bool is_error_reported = false;
u32 bit_pos, byte_pos, error_max, pos;
+ int err;
switch (info->ecc_opt) {
case OMAP_ECC_BCH4_CODE_HW:
@@ -1434,8 +1435,12 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
/* Decode BCH error using ELM module */
elm_decode_bch_error_page(info->elm_dev, ecc_vec, err_vec);
+ err = 0;
for (i = 0; i < eccsteps; i++) {
- if (err_vec[i].error_reported) {
+ if (err_vec[i].error_uncorrectable) {
+ pr_err("nand: uncorrectable bit-flips found\n");
+ err = -EBADMSG;
+ } else if (err_vec[i].error_reported) {
for (j = 0; j < err_vec[i].error_count; j++) {
switch (info->ecc_opt) {
case OMAP_ECC_BCH4_CODE_HW:
@@ -1457,13 +1462,22 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
byte_pos = (error_max - pos - 1) / 8;
if (pos < error_max) {
- if (byte_pos < 512)
+ if (byte_pos < 512) {
+ pr_debug("bitflip@dat[%d]=%x\n",
+ byte_pos, data[byte_pos]);
data[byte_pos] ^= 1 << bit_pos;
- else
+ } else {
+ pr_debug("bitflip@oob[%d]=%x\n",
+ (byte_pos - 512),
+ spare_ecc[byte_pos - 512]);
spare_ecc[byte_pos - 512] ^=
1 << bit_pos;
+ }
+ } else {
+ pr_err("invalid bit-flip @ %d:%d\n",
+ byte_pos, bit_pos);
+ err = -EBADMSG;
}
- /* else, not interested to correct ecc */
}
}
@@ -1475,12 +1489,7 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
spare_ecc += ecc->bytes;
}
- for (i = 0; i < eccsteps; i++)
- /* Return error if uncorrectable error present */
- if (err_vec[i].error_uncorrectable)
- return -EINVAL;
-
- return stat;
+ return (err) ? err : stat;
}
/**
--
1.8.5.1.163.gd7aced9
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes
2014-03-18 13:26 ` [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes Pekon Gupta
@ 2014-03-20 9:27 ` Brian Norris
2014-03-20 13:22 ` Gupta, Pekon
0 siblings, 1 reply; 9+ messages in thread
From: Brian Norris @ 2014-03-20 9:27 UTC (permalink / raw)
To: Pekon Gupta; +Cc: Stefan Roese, linux-mtd, Ezequiel Garcia, Artem Bityutskiy
Hi Pekon,
Thanks for reworking this. I see one issue, but it's not actually a new
issue with your patch. Maybe it deserves a follow-up patch?
On Tue, Mar 18, 2014 at 06:56:44PM +0530, Pekon Gupta wrote:
> As erased-pages do not have ECC stored in their OOB area, so they need to be
> seperated out from programmed-pages, before doing BCH ECC correction.
>
> In current implementation of omap_elm_correct_data() which does ECC correction
> for BCHx ECC schemes, this erased-pages are detected based on specific marker
> byte (reserved as 0x00) in ecc-layout.
> However, this approach has some limitation like;
> 1) All ecc-scheme layouts do not have such Reserved byte marker to
> differentiate between erased-page v/s programmed-page. Thus this is a
> customized solution.
> 2) Reserved marker byte can itself be subjected to bit-flips causing
> erased-page to be misunderstood as programmed-page.
>
> This patch removes dependency on any marker byte in ecc-layout, instead it
> compares calc_ecc[] with pattern of ECC-of-all(0xff). This implicitely
> means that both 'data + oob == all(0xff).
>
> Signed-off-by: Pekon Gupta <pekon@ti.com>
> ---
> drivers/mtd/nand/omap2.c | 85 +++++++++++++++++++-----------------------------
> 1 file changed, 33 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 4bf2f76d..a6c979f 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -1338,21 +1338,8 @@ static int erased_sector_bitflips(u_char *data, u_char *oob,
> * @calc_ecc: ecc read from HW ECC registers
> *
> * Calculated ecc vector reported as zero in case of non-error pages.
> - * In case of error/erased pages non-zero error vector is reported.
> - * In case of non-zero ecc vector, check read_ecc at fixed offset
> - * (x = 13/7 in case of BCH8/4 == 0) to find page programmed or not.
> - * To handle bit flips in this data, count the number of 0's in
> - * read_ecc[x] and check if it greater than 4. If it is less, it is
> - * programmed page, else erased page.
> - *
> - * 1. If page is erased, check with standard ecc vector (ecc vector
> - * for erased page to find any bit flip). If check fails, bit flip
> - * is present in erased page. Count the bit flips in erased page and
> - * if it falls under correctable level, report page with 0xFF and
> - * update the correctable bit information.
> - * 2. If error is reported on programmed page, update elm error
> - * vector and correct the page with ELM error correction routine.
> - *
> + * In case of non-zero ecc vector, first filter out erased-pages, and
> + * then process data via ELM to detect bit-flips.
> */
> static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
> u_char *read_ecc, u_char *calc_ecc)
> @@ -1367,6 +1354,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
> u_char *ecc_vec = calc_ecc;
> u_char *spare_ecc = read_ecc;
> u_char *erased_ecc_vec;
> + u_char *buf;
> + int bitflip_count;
> enum bch_ecc type;
> bool is_error_reported = false;
>
> @@ -1374,10 +1363,12 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
> case OMAP_ECC_BCH4_CODE_HW:
> /* omit 7th ECC byte reserved for ROM code compatibility */
> actual_eccbytes = ecc->bytes - 1;
> + erased_ecc_vec = bch4_vector;
> break;
> case OMAP_ECC_BCH8_CODE_HW:
> /* omit 14th ECC byte reserved for ROM code compatibility */
> actual_eccbytes = ecc->bytes - 1;
> + erased_ecc_vec = bch8_vector;
> break;
> default:
> pr_err("invalid driver configuration\n");
> @@ -1389,10 +1380,8 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
>
> if (info->nand.ecc.strength == BCH8_MAX_ERROR) {
> type = BCH8_ECC;
> - erased_ecc_vec = bch8_vector;
> } else {
> type = BCH4_ECC;
> - erased_ecc_vec = bch4_vector;
> }
>
> for (i = 0; i < eccsteps ; i++) {
> @@ -1410,44 +1399,36 @@ static int omap_elm_correct_data(struct mtd_info *mtd, u_char *data,
> }
>
> if (eccflag == 1) {
> - /*
> - * Set threshold to minimum of 4, half of ecc.strength/2
> - * to allow max bit flip in byte to 4
> - */
> - unsigned int threshold = min_t(unsigned int, 4,
> - info->nand.ecc.strength / 2);
> -
> - /*
> - * Check data area is programmed by counting
> - * number of 0's at fixed offset in spare area.
> - * Checking count of 0's against threshold.
> - * In case programmed page expects at least threshold
> - * zeros in byte.
> - * If zeros are less than threshold for programmed page/
> - * zeros are more than threshold erased page, either
> - * case page reported as uncorrectable.
> - */
> - if (hweight8(~read_ecc[actual_eccbytes]) >= threshold) {
> + if (memcmp(calc_ecc, erased_ecc_vec,
> + actual_eccbytes) == 0) {
> /*
> - * Update elm error vector as
> - * data area is programmed
> + * calc_ecc[] matches pattern for ECC(all 0xff)
> + * so this is definitely an erased-page
> */
> - err_vec[i].error_reported = true;
> - is_error_reported = true;
> } else {
> - /* Error reported in erased page */
> - int bitflip_count;
> - u_char *buf = &data[info->nand.ecc.size * i];
> -
> - if (memcmp(calc_ecc, erased_ecc_vec,
> - actual_eccbytes)) {
> - bitflip_count = erased_sector_bitflips(
> - buf, read_ecc, info);
> -
> - if (bitflip_count)
> - stat += bitflip_count;
> - else
> - return -EINVAL;
> + buf = &data[info->nand.ecc.size * i];
> + /*
> + * count number of 0-bits in read_buf.
> + * This check can be removed once a similar
> + * check is introduced in generic NAND driver
> + */
> + bitflip_count = erased_sector_bitflips(
> + buf, read_ecc, info);
> + if (bitflip_count) {
> + /*
> + * number of 0-bits within ECC limits
> + * So this may be an erased-page
> + */
> + stat += bitflip_count;
I see we update 'stat', but in the case that this is a correctable
erased page, don't we just ignore it? i.e., you'll hit this case below
(not shown here):
/* Check if any error reported */
if (!is_error_reported)
return 0;
Perhaps this should be changed to:
if (!is_error_reported)
return stat;
?
> + } else {
> + /*
> + * Too many 0-bits. It may be a
> + * - programmed-page, OR
> + * - erased-page with many bit-flips
> + * So this page requires check by ELM
> + */
> + err_vec[i].error_reported = true;
> + is_error_reported = true;
> }
> }
> }
Brian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
` (4 preceding siblings ...)
2014-03-18 13:26 ` [PATCH v9 5/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix programmed-page bit-flip correction logic Pekon Gupta
@ 2014-03-20 9:31 ` Brian Norris
5 siblings, 0 replies; 9+ messages in thread
From: Brian Norris @ 2014-03-20 9:31 UTC (permalink / raw)
To: Pekon Gupta; +Cc: Stefan Roese, linux-mtd, Ezequiel Garcia, Artem Bityutskiy
On Tue, Mar 18, 2014 at 06:56:41PM +0530, Pekon Gupta wrote:
> *changes v8 -> v9*
> [PATCH v9 1/5] <no change>
> [PATCH v9 2/5] <no change>
> [PATCH v9 3/5]
> - dropped earlier versions [PATCH v8 3/6] and PATCH v8 4/6], as per feedbacks from
> MTD maintainer 'Brian Norris <computersforpeace@gmail.com>'
> http://lists.infradead.org/pipermail/linux-mtd/2014-March/052472.html
> - Current patch uses existing method of comparing calc_ecc[] with pre-known ECC(all 0xff data + OOB)
> to identify erased-pages. Thereby removing dependency on 'reserved-marker'.
> [PATCH v9 4/5] <same as [PATCH v8 5/6]>
> [PATCH v9 5/5] <same as [PATCH v8 6/6]>
Pushed all 5 to l2-mtd.git. Thanks!
Brian
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes
2014-03-20 9:27 ` Brian Norris
@ 2014-03-20 13:22 ` Gupta, Pekon
0 siblings, 0 replies; 9+ messages in thread
From: Gupta, Pekon @ 2014-03-20 13:22 UTC (permalink / raw)
To: Brian Norris; +Cc: Stefan Roese, linux-mtd, Ezequiel Garcia, Artem Bityutskiy
>From: Brian Norris [mailto:computersforpeace@gmail.com]
[...]
>I see we update 'stat', but in the case that this is a correctable
>erased page, don't we just ignore it? i.e., you'll hit this case below
>(not shown here):
>
> /* Check if any error reported */
> if (!is_error_reported)
> return 0;
>
>Perhaps this should be changed to:
>
> if (!is_error_reported)
> return stat;
>
>?
>
Thanks much. nice catch. I missed this scenario. I have just sent a fix for this.
[PATCH] mtd: nand: omap: ecc.correct: omap_elm_correct_data: return number of bit-flips detected in erased-page
with regards, pekon
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-03-20 13:22 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-18 13:26 [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 1/5] mtd: nand: omap: add field to indicate current ecc-scheme in 'struct omap_nand_info' Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 2/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: rename ambiguous variable 'eccsize' and 'ecc_vector_size' Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 3/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix erased-page detection for BCHx_HW ECC schemes Pekon Gupta
2014-03-20 9:27 ` Brian Norris
2014-03-20 13:22 ` Gupta, Pekon
2014-03-18 13:26 ` [PATCH v9 4/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: cleanup for future enhancements Pekon Gupta
2014-03-18 13:26 ` [PATCH v9 5/5] mtd: nand: omap: ecc.correct: omap_elm_correct_data: fix programmed-page bit-flip correction logic Pekon Gupta
2014-03-20 9:31 ` [PATCH v9 0/5] mtd: nand: omap: optimized chip->ecc.correct() for H/W ECC schemes Brian Norris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).