RFC fsl_elbc_nand.c does not update mtd->ecc_stats

RFC fsl_elbc_nand.c does not update mtd->ecc_stats

[Michael Hench]

when using the fsl_elbc_nand driver, mtd->ecc_stats does not appear to
get updated
UBI uses this to determine when to scrub blocks
the patch below appears to fix this (draft 0)
am I missing anything?
does this look reasonable?



diff -purN orig/drivers/mtd/nand/fsl_elbc_nand.c
linux-3.0-rc7/drivers/mtd/nand/fsl_elbc_nand.c
--- orig/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-22 07:02:37.908778052 -0500
+++ linux-3.0-rc7/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-22
06:56:09.655002047 -0500
@@ -748,12 +748,43 @@ static int fsl_elbc_read_page(struct mtd
 			      uint8_t *buf,
 			      int page)
 {
+	struct fsl_elbc_mtd *priv = chip->priv;
+	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
+	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
+	uint32_t lteccr;
+
 	fsl_elbc_read_buf(mtd, buf, mtd->writesize);
 	fsl_elbc_read_buf(mtd, chip->oob_poi, mtd->oobsize);

 	if (fsl_elbc_wait(mtd, chip) & NAND_STATUS_FAIL)
 		mtd->ecc_stats.failed++;

+	if(chip->ecc.mode != NAND_ECC_HW)
+		return(0);
+
+	/* get the hardware ECC results */
+	lteccr = in_be32(&lbc->lteccr);
+	if(lteccr & 0x000F000F) {
+		int i;
+
+		out_be32(&lbc->lteccr, -1); /* clear lteccr */
+		printk(KERN_ERR "ECC RESULT %x n=%d",
+			lteccr, mtd->ecc_stats.corrected);
+		/*
+		 * 4 bits, one for each 512 byte subpage
+		 * 12-15 (ppc order) are for successfully corrected
+		 * 28-31 are for failed
+		 * small page nand has 3 bits set to zero in each field
+		 */
+		for(i=0; i < 4; i++) {
+			if(lteccr & 0x10000)
+				mtd->ecc_stats.corrected++;
+			if(lteccr & 0x1)
+				mtd->ecc_stats.failed++;
+			lteccr >>= 1;
+		}
+	}
+
 	return 0;
 }

Re: RFC fsl_elbc_nand.c does not update mtd->ecc_stats

[Scott Wood]

On Fri, 22 Jul 2011 08:06:53 -0500
Michael Hench <michaelhench@gmail.com> wrote:

> when using the fsl_elbc_nand driver, mtd->ecc_stats does not appear to
> get updated

It gets updated for uncorrectable errors (so no need to use lteccr for
that), but not correctable.

> diff -purN orig/drivers/mtd/nand/fsl_elbc_nand.c
> linux-3.0-rc7/drivers/mtd/nand/fsl_elbc_nand.c
> --- orig/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-22 07:02:37.908778052 -0500
> +++ linux-3.0-rc7/drivers/mtd/nand/fsl_elbc_nand.c	2011-07-22
> 06:56:09.655002047 -0500
> @@ -748,12 +748,43 @@ static int fsl_elbc_read_page(struct mtd
>  			      uint8_t *buf,
>  			      int page)
>  {
> +	struct fsl_elbc_mtd *priv = chip->priv;
> +	struct fsl_lbc_ctrl *ctrl = priv->ctrl;
> +	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> +	uint32_t lteccr;
> +
>  	fsl_elbc_read_buf(mtd, buf, mtd->writesize);
>  	fsl_elbc_read_buf(mtd, chip->oob_poi, mtd->oobsize);
> 
>  	if (fsl_elbc_wait(mtd, chip) & NAND_STATUS_FAIL)
>  		mtd->ecc_stats.failed++;
> 
> +	if(chip->ecc.mode != NAND_ECC_HW)
> +		return(0);
> +
> +	/* get the hardware ECC results */
> +	lteccr = in_be32(&lbc->lteccr);

Older versions of eLBC have LTECCR, though on the ones that don't have it,
it appears to read as zero and not be modifyable, so it shouldn't make
anything worse.

> +	if(lteccr & 0x000F000F) {
> +		int i;
> +
> +		out_be32(&lbc->lteccr, -1); /* clear lteccr */

We should read and clear lteccr in fsl_elbc_run_command, and test the
stored value here.  That way we don't have leftover bits set in lteccr from
a read that didn't use read_page (such as MTD_OOB_RAW).

> +		printk(KERN_ERR "ECC RESULT %x n=%d",
> +			lteccr, mtd->ecc_stats.corrected);

I assume this is temporary?

-Scott