On Fri, 2015-03-13 at 09:04 +0100, Albert ARIBAUD (3ADEV) wrote: > + /* go through all four small pages */ > + for (i = 0; i < 4; i++) { > + /* start auto decode (reads 528 NAND bytes) */ > + writel(0, &lpc32xx_nand_mlc_registers->ecc_auto_dec_reg); > + /* wait for controller to return to ready state */ > + timeout = LPC32X_NAND_TIMEOUT; > + do { > + if (timeout-- == 0) > + return -1; > + status = readl(&lpc32xx_nand_mlc_registers->isr); > + } while (!(status & ISR_CONTROLLER_READY));
How much time does 10000 reads of this register equate to? Are you sure it's enough? Timeouts should generally be in terms of time, not loop iterations. > +static int read_single_page(uint8_t *dest, int page, > + struct lpc32xx_oob *oob) > +{ > + int status, i, timeout, err, max_bitflips = 0; > + > + /* enter read mode */ > + writel(NAND_CMD_READ0, &lpc32xx_nand_mlc_registers->cmd); > + /* send column (lsb then MSB) and page (lsb to MSB) */ > + writel(0, &lpc32xx_nand_mlc_registers->addr); > + writel(0, &lpc32xx_nand_mlc_registers->addr); > + writel(page & 0xff, &lpc32xx_nand_mlc_registers->addr); > + writel((page>>8) & 0xff, &lpc32xx_nand_mlc_registers->addr); > + writel((page>>16) & 0xff, &lpc32xx_nand_mlc_registers->addr); > + /* start reading */ > + writel(NAND_CMD_READSTART, &lpc32xx_nand_mlc_registers->cmd); > + > + /* large page auto decode read */ > + for (i = 0; i < 4; i++) { > + /* start auto decode (reads 528 NAND bytes) */ > + writel(0, &lpc32xx_nand_mlc_registers->ecc_auto_dec_reg); > + /* wait for controller to return to ready state */ > + timeout = LPC32X_NAND_TIMEOUT; > + do { > + if (timeout-- == 0) > + return -1; > + status = readl(&lpc32xx_nand_mlc_registers->isr); > + } while (!(status & ISR_CONTROLLER_READY)) > + ; > + /* return -1 if hard error */ > + if (status & ISR_DECODER_FAILURE) > + return -1; > + /* keep count of maximum bitflips performed */ > + if (status & ISR_DECODER_ERROR) { > + err = ISR_DECODER_ERRORS(status); > + if (err > max_bitflips) > + max_bitflips = err; > + } > + /* copy first 512 bytes into buffer */ > + memcpy(dest+i*512, lpc32xx_nand_mlc_registers->buff, 512); > + /* copy next 6 bytes bytes into OOB buffer */ > + memcpy(&oob->free[i], lpc32xx_nand_mlc_registers->buff, 6); > + } > + return max_bitflips; > +} > + Why keep track of max_bitflips if the caller doesn't use it? > +#define LARGE_PAGE_SIZE 2048 > + > +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst) > +{ > + struct lpc32xx_oob oob; > + unsigned int page = offs / LARGE_PAGE_SIZE; > + unsigned int left = DIV_ROUND_UP(size, LARGE_PAGE_SIZE); > + > + while (left) { > + int res = read_single_page(dst, page, &oob); > + page++; > + /* if read succeeded, even if fixed by ECC */ > + if (res >= 0) { > + /* skip bad block */ > + if (oob.free[0].free_oob_bytes[0] != 0xff) > + continue; > + if (oob.free[0].free_oob_bytes[1] != 0xff) > + continue; > + /* page is good, keep it */ > + dst += LARGE_PAGE_SIZE; > + left--; > + } You should be checking the designated page(s) of the block, rather than the current page, for the bad block markers -- and skipping the entire block if it's bad. Also, if you fail ECC, that should be a fatal error, not something to silently skip. -Scott _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot