| Message ID | 200907171232.53725.david-b@pacbell.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Dave, > -----Original Message----- > From: David Brownell [mailto:david-b@pacbell.net] > Sent: Friday, July 17, 2009 3:33 PM > To: davinci-linux-open-source@linux.davincidsp.com > Cc: Narnakaje, Snehaprabha; linux-mtd@lists.infradead.org; > dwmw2@infradead.org; tglx@linutronix.de; akpm@linux-foundation.org > Subject: Re: [PATCH v3 3/3] mtd-nand: DaVinci: Add 4-bit ECC support for > large page NAND chips > > On Thursday 16 July 2009, nsnehaprabha@ti.com wrote: > > It also fixes a bug in the ECC correction handler. > > When we introduce 5 bit-errors in chunk, error correction stops working. > When > > errors are detected the 4BITECC_START bit was left high, which should be > > cleared. > > Agreed that needs to be fixed, but there should be a comment > about this being an *undocumented* behavior in the hardware. > > The reason that the bug exists at all is because this step > has never been documented. So, please roll in this update. How about, we get the documents updated for this missing step. Thanks Sneha > > - Dave > > --- > drivers/mtd/nand/davinci_nand.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > --- a/drivers/mtd/nand/davinci_nand.c > +++ b/drivers/mtd/nand/davinci_nand.c > @@ -350,13 +350,16 @@ compare: > > /* > * Clear any previous address calculation by doing a dummy read of > an > - * error address register. > + * error address register. UNDOCUMENTED that the ECC engine won't > + * recover after 5-bit ECC errors without this step. > */ > davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET); > > /* Start address calculation, and wait for it to complete. > * We _could_ start reading more data while this is working, > - * to speed up the overall page read. > + * to speed up the overall page read. UNDOCUMENTED that > + * reading some ERRVAL register is needed in all cases, not > + * just when an error must be corrected. > */ > davinci_nand_writel(info, NANDFCR_OFFSET, > davinci_nand_readl(info, NANDFCR_OFFSET) | BIT(13));
On Friday 17 July 2009, Narnakaje, Snehaprabha wrote: > > > Agreed that needs to be fixed, but there should be a comment > > about this being an *undocumented* behavior in the hardware. > > > > The reason that the bug exists at all is because this step > > has never been documented. Â So, please roll in this update. > > How about, we get the documents updated for this missing step. I had hoped this was in the works, but I was told that for some reason being able to do that was a problem. And in the few months since this issue was identified, the docs haven't been updated. :( - Dve
--- a/drivers/mtd/nand/davinci_nand.c +++ b/drivers/mtd/nand/davinci_nand.c @@ -350,13 +350,16 @@ compare: /* * Clear any previous address calculation by doing a dummy read of an - * error address register. + * error address register. UNDOCUMENTED that the ECC engine won't + * recover after 5-bit ECC errors without this step. */ davinci_nand_readl(info, NAND_ERR_ADD1_OFFSET); /* Start address calculation, and wait for it to complete. * We _could_ start reading more data while this is working, - * to speed up the overall page read. + * to speed up the overall page read. UNDOCUMENTED that + * reading some ERRVAL register is needed in all cases, not + * just when an error must be corrected. */ davinci_nand_writel(info, NANDFCR_OFFSET, davinci_nand_readl(info, NANDFCR_OFFSET) | BIT(13));