> > +static void nand_davinci_4bit_enable_hwecc(struct mtd_info *mtd, int > mode) > > +{ > > + u32 val; > > + > > + switch (mode) { > > + case NAND_ECC_WRITE: > > + case NAND_ECC_READ: > > + /* > > + * Start a new ECC calculation for reading or writing 512 > bytes > > + * of data. > > + */ > > + val = (emif_regs->NANDFCR & ~(3 << 4)) | (1 << 12); > > + emif_regs->NANDFCR = val; > > + break; > > + case NAND_ECC_READSYN: > > + val = emif_regs->NAND4BITECC1; > > Use I/O accessors. I could not understand this one. It is done similarly nand_davinci_enable_hwecc which is used for 1 BIT ECC. NANDFCR is a control register and we have to write the appropriate valvue to it. We similarly write to other register that are part of the IP in other sections of this driver.
> > > + for (i = 0; i < 100; i++) > > + udelay(this->chip_delay); > > No explanation for the delay? Is there any status register you can poll? > > Is it truly 100 times the chip delay (even if that changes), or is it a > fixed delay that just happens to work out to that? remnant of an old version of the driver which had bugs. I have got rid of this > > > +static int nand_davinci_4bit_correct_data(struct mtd_info *mtd, uint8_t > *dat, > > + uint8_t *read_ecc, uint8_t *calc_ecc) > > +{ > > + return nand_davinci_4bit_compare_ecc(mtd, read_ecc, dat); > > +} > > Why have a wrapper? This seems to be the only place where compare_ecc is > used. Yes its of no use, I'll remove it. > > -Scott I'll send in a patch with your comments addressed except the first one. I can resend the patch( again if required) once I know what you mean by your comment #1. Thanks, Sandeep _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot