[U-Boot] [PATCH] NAND: add the ability to directly write yaffs image

[Scott Wood]

On Mon, Jan 03, 2011 at 09:09:20PM +0800, Lei Wen wrote:
> This patch add addition suffix to nand write to give the uboot
> the power to directly burn the yaffs image to nand.
> 
> Signed-off-by: Lei Wen <leiwen at marvell.com>
> ---
>  common/cmd_nand.c            |   12 ++++++++--
>  drivers/mtd/nand/nand_util.c |   43 +++++++++++++++++++++++++++++++++++------
>  include/nand.h               |    2 +-
>  3 files changed, 46 insertions(+), 11 deletions(-)
> 
> diff --git a/common/cmd_nand.c b/common/cmd_nand.c
> index c547a68..e113971 100644
> --- a/common/cmd_nand.c
> +++ b/common/cmd_nand.c
> @@ -574,7 +574,13 @@ int do_nand(cmd_tbl_t * cmdtp, int flag, int argc, char * const argv[])
>  							 (u_char *)addr);
>  			else
>  				ret = nand_write_skip_bad(nand, off, &rwsize,
> -							  (u_char *)addr);
> +							  (u_char *)addr, 0);
> +		} else if (!strcmp(s, ".yaffs")) {
> +			if (read) {
> +				printf("Unknown nand command suffix '%s'.\n", s);
> +				return 1;
> +			}
> +			ret = nand_write_skip_bad(nand, off, &size, (u_char *)addr, 1);

cmd_nand.c:583:4: warning: passing argument 3 of 'nand_write_skip_bad' from incompatible pointer type
include/nand.h:117:5: note: expected 'size_t *' but argument is of type 'loff_t *'

Use rwsize.

> diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c
> index 22c7411..e7a314a 100644
> --- a/drivers/mtd/nand/nand_util.c
> +++ b/drivers/mtd/nand/nand_util.c
> @@ -451,12 +451,24 @@ static int check_skip_len(nand_info_t *nand, loff_t offset, size_t length)
>   * @return		0 in case of success
>   */
>  int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
> -			u_char *buffer)
> +			u_char *buffer, int withoob)
>  {
> -	int rval;
> +	int rval = 0, blocksize, pages, page;
>  	size_t left_to_write = *length;
>  	u_char *p_buffer = buffer;
>  	int need_skip;
> +	struct mtd_oob_ops ops;
> +
> +	if (withoob) {
> +		pages = nand->erasesize / nand->writesize;
> +		blocksize = (pages * nand->oobsize) + nand->erasesize;
> +		if (*length % (nand->writesize + nand->oobsize)) {
> +			printf ("Attempt to write non yaffs page aligned data\n");
> +			return -EINVAL;
> +		}
> +	}
> +	else
> +		blocksize = nand->erasesize;

if (withoob) {
	...
} else {
	blocksize = nand->erasesize;
}

For the error message, how about, "Attempt to write incomplete page
in yaffs mode"?

>  	/*
>  	 * nand_write() handles unaligned, partial page writes.
> @@ -506,12 +518,26 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t offset, size_t *length,
>  			continue;
>  		}
>  
> -		if (left_to_write < (nand->erasesize - block_offset))
> +		if (left_to_write < (blocksize - block_offset))
>  			write_size = left_to_write;
>  		else
> -			write_size = nand->erasesize - block_offset;
> -
> -		rval = nand_write (nand, offset, &write_size, p_buffer);
> +			write_size = blocksize - block_offset;
> +
> +		if (withoob) {
> +			ops.len = nand->writesize;
> +			ops.ooblen = nand->oobsize;
> +			ops.mode = MTD_OOB_AUTO;
> +			ops.ooboffs = 0;
> +			pages = write_size / (nand->writesize + nand->oobsize);
> +			for (page = 0; page < pages && !rval; page ++) {
> +				ops.datbuf = (page * (nand->writesize + nand->oobsize))
> +					     + p_buffer;
> +				ops.oobbuf = ops.datbuf + nand->writesize;
> +				rval = nand->write_oob(nand, offset + page *nand->writesize, &ops);

This looks like you're dereferencing a pointer, rather than multiplying page
with nand->writesize.  Also, line length.

How about something like this (untested), IMHO a little clearer, and 60
bytes smaller (out of 481 bytes added by the patch as written) on powerpc
with GCC 4.5:

	while (left_to_write > 0) {
		...

		if (withoob) {
			size_t pagesize = nand->writesize;
			size_t pagesize_oob = pagesize + nand->oobsize;

			ops.len = pagesize;
			ops.ooblen = nand->oobsize;
			ops.mode = MTD_OOB_AUTO;
			ops.ooboffs = 0;

			pages = write_size / pagesize_oob;
			for (page = 0; page < pages; page++) {
				ops.databuf = p_buffer;
				ops.oobbuf = ops.databuf + pagesize;

				rval = nand->write_oob(nand, offset, &ops);
				if (!rval)
					break;

				offset += pagesize;
				p_buffer += pagesize_oob;
			}
		} else {
			rval = nand_write(nand, offset, &write_size, p_buffer);
			offset += write_size;
			p_buffer += write_size;
		}

		if (rval != 0) {
			...
		}

		left_to_write -= write_size;
	}

Should this be conditional on CONFIG_YAFFS2, or a new CONFIG_CMD_NAND_YAFFS? 
The NAND code is too big already...

> +			}
> +		}
> +		else
> +			rval = nand_write (nand, offset, &write_size, p_buffer);

	}
} else {
	rval = ...;
}

-Scott