Messages in this thread | | | Date | Wed, 17 May 2017 15:00:59 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH] fsl_ifc_nand: fix handing of bit flips in erased nand |
| |
Hi!
> On Wed, 17 May 2017 14:22:24 +0200 > Pavel Machek <pavel@ucw.cz> wrote: > > > If we see unrecoverable ECC error, we need to count number of bitflips > > from all-ones and report correctable/uncorrectable according to > > that. Otherwise we report ECC failed on erased flash with single bit error. > > > > Signed-off-by: Pavel Machek <pavel@denx.de> > > > > @@ -678,6 +648,41 @@ static int fsl_ifc_wait(struct mtd_info *mtd, struct nand_chip *chip) > > return nand_fsr | NAND_STATUS_WP; > > } > > > > +/* > > + * The controller does not check for bitflips in erased pages, > > + * therefore software must check instead. > > + */ > > +static int check_erased_page(struct nand_chip *chip, u8 *buf) > > +{ > > + struct mtd_info *mtd = nand_to_mtd(chip); > > + u8 *ecc = chip->oob_poi; > > + const int ecc_size = chip->ecc.bytes; > > + const int pkt_size = chip->ecc.size; > > + int i, res, bitflips = 0; > > + struct mtd_oob_region oobregion = { }; > > + > > + mtd_ooblayout_ecc(mtd, 0, &oobregion); > > + ecc += oobregion.offset; > > + > > + for (i = 0; i < chip->ecc.steps; ++i) { > > + res = nand_check_erased_ecc_chunk(buf, pkt_size, ecc, ecc_size, > > + NULL, 0, > > + chip->ecc.strength); > > + if (res < 0) > > + mtd->ecc_stats.failed++; > > + else > > + mtd->ecc_stats.corrected += res; > > + > > + bitflips = max(res, bitflips); > > + buf += pkt_size; > > + ecc += ecc_size; > > + } > > + > > + mtd_ooblayout_ecc(mtd, 1, &oobregion); > > Why is this needed?
It is not, will remove.
> > @@ -904,6 +922,21 @@ static int fsl_ifc_chip_init(struct fsl_ifc_mtd *priv) > > chip->ecc.algo = NAND_ECC_HAMMING; > > } > > > > + { > > + struct mtd_oob_region oobregion = { }; > > + > > + mtd_ooblayout_ecc(mtd, 0, &oobregion); > > + if (!oobregion.length) { > > + dev_err(priv->dev, "No ECC in oobregion?\n"); > > + return -EINVAL; > > + } > > + mtd_ooblayout_ecc(mtd, 1, &oobregion); > > + if (oobregion.length) { > > + dev_err(priv->dev, "Extra data in oobregion?\n"); > > + return -EINVAL; > > + } > > + } > > This clearly doesn't belong in this patch. And if you really want to > check that, please create a separate function instead of defining a > non-conditional code block inside fsl_ifc_chip_init().
I am not sure I want to check that. check_erased_page() can only handle layout with just one oobregion. If you think check is not needed, I'll happily remove the checking.
Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [unhandled content-type:application/pgp-signature] | |