13 Mar
2015
13 Mar
'15
10:57 p.m.
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