[U-Boot-Users] [PATCH 2/7 v3] NAND_CMD_READOOB is not supported by all chips, read OOB with the page instead

[Scott Wood]

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