On 18/11/09 20:22, Scott Wood wrote: > On Mon, Nov 16, 2009 at 05:49:55PM +0000, Nick Thompson wrote: >> static void nand_davinci_enable_hwecc(struct mtd_info *mtd, int mode) >> { >> - int dummy; >> + u_int32_t val; >> >> - dummy = emif_regs->NANDF1ECC; >> + val = readl(&emif_regs->NANDF1ECC); > > "val =" can be omitted, which would keep it clear that it is a dummy read.
Sounds good - I'll follow Wolfgang's idea and add a (void) as well. > >> - /* FIXME: only chipselect 0 is supported for now */ >> - emif_regs->NANDFCR |= 1 << 8; >> + val = readl(&emif_regs->NANDFCR); >> + val |= DAVINCI_NANDFCR_1BIT_ECC_START(CONFIG_SYS_NAND_CS); >> + writel(val, &emif_regs->NANDFCR); > > Do you need to clear the bit corresponding to the previous chipselect? No, the bits clear themselves when the ECC result is read. The device is actually capable of running multiple ECC1's at the same time. It can't do multiple ECC4's, but the select for that is coded to make this clear. > > Otherwise, ACK. I'm going to resubmit as your first comment revealed a latent bug. That dummy read is what ensures the ECC start bit is clear, but It only clear the CS2 ECC, rather than the config selected ECC. Thanks, Nick. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot