[U-Boot] [PATCH] mtd: nand: fix the written length when nand_write_skip_bad failed

[Scott Wood]

On 03/02/2013 03:01:10 AM, Tao Hou wrote:
> When the data has been partially written into the NAND Flash,
> returning the written length instead of 0. The written length
> may be useful when the upper level decides to continue the writing
> after skipping the block causing the write failure.

We already do that in some code paths.

> Signed-off-by: Tao Hou <hotforest at gmail.com>
> Cc: Scott Wood <scottwood at freescale.com>
> Cc: Ben Gardiner <bengardiner at nanometrics.ca>
> Cc: Lei Wen <leiwen at marvell.com>
> ---
>  drivers/mtd/nand/nand_util.c |   22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)

Could you rebase this on top of this patch:
http://patchwork.ozlabs.org/patch/224842/

BTW, are you actually using WITH_YAFFS_OOB?  I think there are some  
other things wrong with it at the moment, as mentioned here:
http://lists.denx.de/pipermail/u-boot/2013-March/148378.html

> diff --git a/drivers/mtd/nand/nand_util.c  
> b/drivers/mtd/nand/nand_util.c
> index de1d13e..f57d723 100644
> --- a/drivers/mtd/nand/nand_util.c
> +++ b/drivers/mtd/nand/nand_util.c
> @@ -496,8 +496,10 @@ int nand_write_skip_bad(nand_info_t *nand,  
> loff_t offset, size_t *length,
> 
>  #ifdef CONFIG_CMD_NAND_YAFFS
>  	if (flags & WITH_YAFFS_OOB) {
> -		if (flags & ~WITH_YAFFS_OOB)
> +		if (flags & ~WITH_YAFFS_OOB) {
> +			*length = 0;
>  			return -EINVAL;
> +		}
> 
>  		int pages;
>  		pages = nand->erasesize / nand->writesize;
> @@ -505,6 +507,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t  
> offset, size_t *length,
>  		if (*length % (nand->writesize + nand->oobsize)) {
>  			printf("Attempt to write incomplete page"
>  				" in yaffs mode\n");
> +			*length = 0;
>  			return -EINVAL;
>  		}
>  	} else
> @@ -542,7 +545,6 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t  
> offset, size_t *length,
>  		if (rval == 0)
>  			return 0;
> 
> -		*length = 0;
>  		printf("NAND write to offset %llx failed %d\n",
>  			offset, rval);
>  		return rval;

OK so far...

> @@ -550,7 +552,7 @@ int nand_write_skip_bad(nand_info_t *nand, loff_t  
> offset, size_t *length,
> 
>  	while (left_to_write > 0) {
>  		size_t block_offset = offset & (nand->erasesize - 1);
> -		size_t write_size, truncated_write_size;
> +		size_t write_size, truncated_write_size, written_size;
> 
>  		WATCHDOG_RESET();
> 
> @@ -586,8 +588,10 @@ int nand_write_skip_bad(nand_info_t *nand,  
> loff_t offset, size_t *length,
>  				ops.oobbuf = ops.datbuf + pagesize;
> 
>  				rval = nand->write_oob(nand, offset,  
> &ops);
> -				if (rval != 0)
> +				if (rval != 0) {
> +					written_size = pagesize_oob *  
> page;
>  					break;
> +				}
> 
>  				offset += pagesize;
>  				p_buffer += pagesize_oob;
> @@ -605,14 +609,18 @@ int nand_write_skip_bad(nand_info_t *nand,  
> loff_t offset, size_t *length,
> 
>  			rval = nand_write(nand, offset,  
> &truncated_write_size,
>  					p_buffer);
> -			offset += write_size;
> -			p_buffer += write_size;
> +			if (rval == 0) {
> +				offset += write_size;
> +				p_buffer += write_size;
> +			} else {
> +				written_size = truncated_write_size;
> +			}
>  		}
> 
>  		if (rval != 0) {
>  			printf("NAND write to offset %llx failed %d\n",
>  				offset, rval);
> -			*length -= left_to_write;
> +			*length -= left_to_write - written_size;
>  			return rval;
>  		}

...but I don't see why this part is needed (or correct).  Why doesn't  
"*length -= left_to_write" already get you what you want?

-Scott