On Mon, Aug 04, 2008 at 02:45:33PM +0200, Guennadi Liakhovetski wrote: > I _think_ this should work with all NAND chips. Otherwise we might have to > introduce a configuration variable.
Which small-page NAND chips can't handle READOOB? On large page devices, nand_command changes it to READ0. That said, doing it all at once could result in smaller, faster, and simpler code. > @@ -150,8 +135,6 @@ static int nand_read_page(struct mtd_info *mtd, int > block, int page, uchar *dst) > u_char *ecc_code; > u_char *oob_data; > int i; > - int eccsize = CFG_NAND_ECCSIZE; > - int eccbytes = CFG_NAND_ECCBYTES; Any particular reason for this change? It's more readable as is, IMHO. > @@ -195,6 +180,7 @@ static int nand_load(struct mtd_info *mtd, int offs, int > uboot_size, uchar *dst) > int block; > int blockcopy_count; > int page; > + unsigned read = 0; "unsigned int", please. > + int badblock = 0; > + for (page = 0; page < CFG_NAND_PAGE_COUNT; page++) { > + nand_read_page(mtd, block, page, dst); > + if ((!page > +#ifdef CFG_NAND_BBT_2NDPAGE > + || page == 1 > +#endif Please use page == 0 rather than !page when checking for an actual value of zero as opposed to a zero that means "not valid" or similar. > + ) && dst[CFG_NAND_PAGE_SIZE] != 0xff) { > + badblock = 1; > + break; > } > + /* Overwrite skipped pages */ > + if (read >= offs) > + dst += CFG_NAND_PAGE_SIZE; > + read += CFG_NAND_PAGE_SIZE; > + } I don't follow the logic here -- you're discarding a number of good blocks equal to the offset? That might make sense if we were starting at block zero, and defining the offset as not including any bad blocks before the image -- but the first block we read is at the start of the image, not the start of flash. > @@ -241,12 +239,18 @@ void nand_boot(void) > nand_chip.dev_ready = NULL; /* preset to NULL */ > board_nand_init(&nand_chip); > > + if (nand_chip.select_chip) > + nand_chip.select_chip(&nand_info, 0); > + > /* > * Load U-Boot image from NAND into RAM > */ > ret = nand_load(&nand_info, CFG_NAND_U_BOOT_OFFS, CFG_NAND_U_BOOT_SIZE, > (uchar *)CFG_NAND_U_BOOT_DST); > > + if (nand_chip.select_chip) > + nand_chip.select_chip(&nand_info, -1); > + This seems like an unrelated change, that wasn't described in the changelog. -Scott ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users