On Sat, 2015-03-14 at 15:27 +0100, Albert ARIBAUD wrote:
Bonjour Scott,
Le Fri, 13 Mar 2015 16:57:33 -0500, Scott Wood scottwood@freescale.com a écrit :
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.
I followed the examples in several drivers where timeouts are by iteration. Note that -- while this does not void your point -- I did not use 10000 but 100000, which at a CPU clock of 208 MHz, and assuming an optimistic one instruction per cycle and two instructions per loop, makes the loop last at least 960 us, well over the 600 us which the NAND takes for any page programming.
What if this driver ends up being used on hardware that runs significantly faster than 208 MHz? I could understand if it's hugely space-constrained SPL code (like the ones that have to fit in 4K), but otherwise why not make use of the timekeeping code that exists in U-Boot (either by reading the timer, or by putting udelay(1) in the loop body)?
+#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.
Will fix this -- is there any helper function in the bad block management code for this? I could not find one, but I'm no NAND expert.
I don't know of any such helper -- outside of SPL it's handled via the BBT. fsl_ifc_spl.c is an example that checks for bad block markers, but it's hardcoded to assume the first two pages of a block which is a bit simpler than checking at the end of the block.
-Scott