Re: [RFC PATCH 2/2] mtd: nand: use nand_check_erased_ecc_chunk in default ECC read functions

[Andrea Scian <rnd4@dave-tech.it>]

Dear Boris,


Il 30/07/2015 19:34, Boris Brezillon ha scritto:
> The default NAND read functions are relying on an underlying controller
> to correct bitflips, but some of those controller cannot properly fix
> bitflips in erased pages.
> In case of ECC failures, check if the page of subpage is empty before
> reporting an ECC failure.

I'm still wondering if chip->ecc.strength is the right threshold.

Did you see my comments here [1]? WDYT?

Maybe we can have this discussion in a separate thread, if you want ;-)

Kind Regards,

-- 

Andrea SCIAN

DAVE Embedded Systems

[1] http://lists.infradead.org/pipermail/linux-mtd/2015-July/060520.html

>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>   drivers/mtd/nand/nand_base.c | 63 ++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 55 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 1542ea7..d527c73 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -1396,6 +1396,19 @@ static int nand_read_subpage(struct mtd_info *mtd, struct nand_chip *chip,
>   		stat = chip->ecc.correct(mtd, p,
>   			&chip->buffers->ecccode[i], &chip->buffers->ecccalc[i]);
>   		if (stat < 0) {
> +			/* check for empty pages with bitflips */
> +			int col = (int)(p - bufpoi);
> +
> +			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, col, -1);
> +			chip->read_buf(mtd, p, chip->ecc.size);
> +			stat = nand_check_erased_ecc_chunk(p, chip->ecc.size,
> +						&chip->buffers->ecccode[i],
> +						chip->ecc.bytes,
> +						NULL, 0,
> +						chip->ecc.strength);
> +		}
> +
> +		if (stat < 0) {
>   			mtd->ecc_stats.failed++;
>   		} else {
>   			mtd->ecc_stats.corrected += stat;
> @@ -1445,6 +1458,16 @@ static int nand_read_page_hwecc(struct mtd_info *mtd, struct nand_chip *chip,
>
>   		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
>   		if (stat < 0) {
> +			/* check for empty pages with bitflips */
> +			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, i, -1);
> +			chip->read_buf(mtd, p, eccsize);
> +			stat = nand_check_erased_ecc_chunk(p, eccsize,
> +						&ecc_code[i], eccbytes,
> +						NULL, 0,
> +						chip->ecc.strength);
> +		}
> +
> +		if (stat < 0) {
>   			mtd->ecc_stats.failed++;
>   		} else {
>   			mtd->ecc_stats.corrected += stat;
> @@ -1495,7 +1518,18 @@ static int nand_read_page_hwecc_oob_first(struct mtd_info *mtd,
>   		chip->read_buf(mtd, p, eccsize);
>   		chip->ecc.calculate(mtd, p, &ecc_calc[i]);
>
> -		stat = chip->ecc.correct(mtd, p, &ecc_code[i], NULL);
> +		stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
> +		if (stat < 0) {
> +			/* check for empty pages with bitflips */
> +			chip->cmdfunc(mtd, NAND_CMD_RNDOUT,
> +				      i + mtd->oobsize, -1);
> +			chip->read_buf(mtd, p, eccsize);
> +			stat = nand_check_erased_ecc_chunk(p, eccsize,
> +						&ecc_code[i], eccbytes,
> +						NULL, 0,
> +						chip->ecc.strength);
> +		}
> +
>   		if (stat < 0) {
>   			mtd->ecc_stats.failed++;
>   		} else {
> @@ -1523,6 +1557,8 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>   	int i, eccsize = chip->ecc.size;
>   	int eccbytes = chip->ecc.bytes;
>   	int eccsteps = chip->ecc.steps;
> +	int eccstepsize = eccsize + eccbytes + chip->ecc.prepad +
> +			  chip->ecc.postpad;
>   	uint8_t *p = buf;
>   	uint8_t *oob = chip->oob_poi;
>   	unsigned int max_bitflips = 0;
> @@ -1542,19 +1578,30 @@ static int nand_read_page_syndrome(struct mtd_info *mtd, struct nand_chip *chip,
>   		chip->read_buf(mtd, oob, eccbytes);
>   		stat = chip->ecc.correct(mtd, p, oob, NULL);
>
> -		if (stat < 0) {
> -			mtd->ecc_stats.failed++;
> -		} else {
> -			mtd->ecc_stats.corrected += stat;
> -			max_bitflips = max_t(unsigned int, max_bitflips, stat);
> -		}
> -
>   		oob += eccbytes;
>
>   		if (chip->ecc.postpad) {
>   			chip->read_buf(mtd, oob, chip->ecc.postpad);
>   			oob += chip->ecc.postpad;
>   		}
> +
> +		if (stat < 0) {
> +			/* check for empty pages with bitflips */
> +			chip->cmdfunc(mtd, NAND_CMD_RNDOUT, i * eccstepsize, -1);
> +			chip->read_buf(mtd, p, chip->ecc.size);
> +			stat = nand_check_erased_ecc_chunk(p, chip->ecc.size,
> +							   oob - eccstepsize,
> +							   eccstepsize,
> +							   NULL, 0,
> +							   chip->ecc.strength);
> +		}
> +
> +		if (stat < 0) {
> +			mtd->ecc_stats.failed++;
> +		} else {
> +			mtd->ecc_stats.corrected += stat;
> +			max_bitflips = max_t(unsigned int, max_bitflips, stat);
> +		}
>   	}
>
>   	/* Calculate remaining oob bytes */
>