Hi Boris, 2017-03-23 5:56 GMT+09:00 Boris Brezillon <boris.brezil...@free-electrons.com>: > On Wed, 22 Mar 2017 23:07:17 +0900 > Masahiro Yamada <yamada.masah...@socionext.com> wrote: >> dev_err(denali->dev, >> @@ -1148,12 +1136,15 @@ static int denali_read_page(struct mtd_info *mtd, >> struct nand_chip *chip, >> if (check_erased_page) { >> read_oob_data(mtd, chip->oob_poi, denali->page); >> >> - /* check ECC failures that may have occurred on erased pages */ >> - if (check_erased_page) { >> - if (!is_erased(buf, mtd->writesize)) >> - mtd->ecc_stats.failed++; >> - if (!is_erased(buf, mtd->oobsize)) >> - mtd->ecc_stats.failed++; >> + stat = nand_check_erased_ecc_chunk( >> + buf, mtd->writesize, >> + chip->oob_poi, mtd->oobsize, >> + NULL, 0, >> + chip->ecc.strength * chip->ecc.steps); > > That's not how it's supposed to be done. Each chunk should be checked > independently. Here is a simple example explaining why this is > important: > > Let's consider the following setup: > - 4k pages > - 16bits/1024bytes ECC > > With your approach, you turn this into: > - 4k pages > - 64bits/4096bytes ECC > > Now suppose you have 32 bitflips in the first 1024 bytes. The real ECC > config is expected to report uncorrectable errors, but your approach > will just report that 32 bits have been fixed, which is wrong.
OK. How about adding a helper like follows: static int denali_check_erased_page(struct mtd_info *mtd, struct nand_chip *chip, uint8_t *buf) { uint8_t *ecc_code = chip->buffers->ecccode; int ecc_steps = chip->ecc.steps; int ecc_size = chip->ecc.size; int ecc_bytes = chip->ecc.bytes; int i, ret; ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0, chip->ecc.total); if (ret) return ret; for (i = 0; i < ecc_steps; i++) { ret = nand_check_erased_ecc_chunk(buf, ecc_size, ecc_code, ecc_bytes, NULL, 0, chip->ecc.strength); if (ret < 0) return ret; buf += ecc_size; ecc_code += ecc_bytes; } return 0; } Then, stat = denali_check_erased_page(mtd, chip, buf); if (stat < 0) { mtd->ecc_stats.failed++; /* return 0 for uncorrectable bitflips */ stat = 0; } -- Best Regards Masahiro Yamada