| Message ID | 200911172245.nAHMjk8X001820@imap1.linux-foundation.org |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2009-11-17 at 14:45 -0800, akpm@linux-foundation.org wrote: > + /* > + * ECC_STATE field reads 0x3 (Error correction complete) immediately > + * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately > + * begin trying to poll for the state, you may fall right out of your > + * loop without any of the correction calculations having taken place. > + * The recommendation from the hardware team is to wait till ECC_STATE > + * reads less than 4, which means ECC HW has entered correction state. > + */ > + do { > + ecc_state = (davinci_nand_readl(info, > + NANDFSR_OFFSET) >> 8) & 0x0f; > + cpu_relax(); > + } while (ecc_state < 4); Please can this have a timeout. Infinite loops waiting for hardware are generally a bad idea.
On Mon, Nov 30, 2009 at 17:08:04, David Woodhouse wrote: > On Tue, 2009-11-17 at 14:45 -0800, akpm@linux-foundation.org wrote: > > + /* > > + * ECC_STATE field reads 0x3 (Error correction complete) immediately > > + * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately > > + * begin trying to poll for the state, you may fall right out of your > > + * loop without any of the correction calculations having taken place. > > + * The recommendation from the hardware team is to wait till ECC_STATE > > + * reads less than 4, which means ECC HW has entered correction state. > > + */ > > + do { > > + ecc_state = (davinci_nand_readl(info, > > + NANDFSR_OFFSET) >> 8) & 0x0f; > > + cpu_relax(); > > + } while (ecc_state < 4); > > Please can this have a timeout. Infinite loops waiting for hardware are > generally a bad idea. > David, I'll add the timeout and re-submit this patch. Thanks, Sudhakar
diff -puN drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-correct-4-bit-error-correction drivers/mtd/nand/davinci_nand.c --- a/drivers/mtd/nand/davinci_nand.c~mtd-nand-davinci-correct-4-bit-error-correction +++ a/drivers/mtd/nand/davinci_nand.c @@ -310,6 +310,7 @@ static int nand_davinci_correct_4bit(str unsigned short ecc10[8]; unsigned short *ecc16; u32 syndrome[4]; + u32 ecc_state; unsigned num_errors, corrected; /* All bytes 0xff? It's an erased page; ignore its ECC. */ @@ -360,6 +361,21 @@ compare: */ davinci_nand_writel(info, NANDFCR_OFFSET, davinci_nand_readl(info, NANDFCR_OFFSET) | BIT(13)); + + /* + * ECC_STATE field reads 0x3 (Error correction complete) immediately + * after setting the 4BITECC_ADD_CALC_START bit. So if you immediately + * begin trying to poll for the state, you may fall right out of your + * loop without any of the correction calculations having taken place. + * The recommendation from the hardware team is to wait till ECC_STATE + * reads less than 4, which means ECC HW has entered correction state. + */ + do { + ecc_state = (davinci_nand_readl(info, + NANDFSR_OFFSET) >> 8) & 0x0f; + cpu_relax(); + } while (ecc_state < 4); + for (;;) { u32 fsr = davinci_nand_readl(info, NANDFSR_OFFSET);