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

Reply via email to