[U-Boot] [PATCH v5 6/9] dfu: NAND specific routines for DFU operation

[Scott Wood <scottwood@freescale.com>]

On 03/08/2013 11:37:25 AM, Tom Rini wrote:
> From: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> Support for NAND storage devices to work with the DFU framework.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> Changes in v5:
> - Document CONFIG_DFU_NAND in README
> 
> Changes in v4: None
> Changes in v3:
> - Rework logic in nand_block_op for nand_(read|write)_skip_bad  
> returning
>   just a size for actual used length.
> - Remove unused externs from drivers/dfu/dfu_nand.c
> 
> Changes in v2:
> - nand_block_op calls nand_(read|write)_skip_bad directly.
> - Bugfix in dfu_nand to make sure we set dfu->skip_bad to 0 on each
>   iteration.
> 
>  README                 |    3 +
>  drivers/dfu/Makefile   |    1 +
>  drivers/dfu/dfu.c      |    8 ++
>  drivers/dfu/dfu_nand.c |  195  
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dfu.h          |   23 ++++++
>  5 files changed, 230 insertions(+)
>  create mode 100644 drivers/dfu/dfu_nand.c
> 
> diff --git a/README b/README
> index 154b82f..c70251e 100644
> --- a/README
> +++ b/README
> @@ -1338,6 +1338,9 @@ The following options need to be configured:
>  		CONFIG_DFU_MMC
>  		This enables support for exposing (e)MMC devices via  
> DFU.
> 
> +		CONFIG_DFU_NAND
> +		This enables support for exposing NAND devices via DFU.
> +
>  		CONFIG_SYS_DFU_MAX_FILE_SIZE
>  		When updating files rather than the raw storage device,
>  		we use a static buffer to copy the file into and then  
> write
> diff --git a/drivers/dfu/Makefile b/drivers/dfu/Makefile
> index 7b717bc..153095d 100644
> --- a/drivers/dfu/Makefile
> +++ b/drivers/dfu/Makefile
> @@ -27,6 +27,7 @@ LIB	= $(obj)libdfu.o
> 
>  COBJS-$(CONFIG_DFU_FUNCTION) += dfu.o
>  COBJS-$(CONFIG_DFU_MMC) += dfu_mmc.o
> +COBJS-$(CONFIG_DFU_NAND) += dfu_nand.o
> 
>  SRCS    := $(COBJS-y:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS-y))
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 2fecf77..84139bd 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -86,6 +86,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf,  
> int size, int blk_seq_num)
>  		/* initial state */
>  		dfu->crc = 0;
>  		dfu->offset = 0;
> +		dfu->bad_skip = 0;
>  		dfu->i_blk_seq_num = 0;
>  		dfu->i_buf_start = dfu_buf;
>  		dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
> @@ -235,6 +236,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf,  
> int size, int blk_seq_num)
>  		dfu->i_buf = dfu->i_buf_start;
>  		dfu->b_left = 0;
> 
> +		dfu->bad_skip = 0;
> +
>  		dfu->inited = 1;
>  	}
> 
> @@ -264,6 +267,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf,  
> int size, int blk_seq_num)
>  		dfu->i_buf = dfu->i_buf_start;
>  		dfu->b_left = 0;
> 
> +		dfu->bad_skip = 0;
> +
>  		dfu->inited = 0;
>  	}
> 
> @@ -286,6 +291,9 @@ static int dfu_fill_entity(struct dfu_entity  
> *dfu, char *s, int alt,
>  	if (strcmp(interface, "mmc") == 0) {
>  		if (dfu_fill_entity_mmc(dfu, s))
>  			return -1;
> +	} else if (strcmp(interface, "nand") == 0) {
> +		if (dfu_fill_entity_nand(dfu, s))
> +			return -1;
>  	} else {
>  		printf("%s: Device %s not (yet) supported!\n",
>  		       __func__,  interface);
> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c
> new file mode 100644
> index 0000000..b7f60dd
> --- /dev/null
> +++ b/drivers/dfu/dfu_nand.c
> @@ -0,0 +1,195 @@
> +/*
> + * dfu_nand.c -- DFU for NAND routines.
> + *
> + * Copyright (C) 2012-2013 Texas Instruments, Inc.
> + *
> + * Based on dfu_mmc.c which is:
> + * Copyright (C) 2012 Samsung Electronics
> + * author: Lukasz Majewski <l.majewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or  
> modify
> + * it under the terms of the GNU General Public License as published  
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA   
> 02111-1307  USA
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <errno.h>
> +#include <div64.h>
> +#include <dfu.h>
> +#include <linux/mtd/mtd.h>
> +#include <jffs2/load_kernel.h>
> +#include <nand.h>
> +
> +enum dfu_nand_op {
> +	DFU_OP_READ = 1,
> +	DFU_OP_WRITE,
> +};
> +
> +static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
> +			u64 offset, void *buf, long *len)
> +{
> +	loff_t start;
> +	size_t count, actual;
> +	int ret;
> +	int dev;
> +	nand_info_t *nand;
> +
> +	/* if buf == NULL return total size of the area */
> +	if (buf == NULL) {
> +		*len = dfu->data.nand.size;
> +		return 0;
> +	}
> +
> +	start = dfu->data.nand.start + offset + dfu->bad_skip;
> +	count = *len;
> +	if (start + count >
> +			dfu->data.nand.start + dfu->data.nand.size) {
> +		printf("%s: block_op out of bounds\n", __func__);
> +		return -1;
> +	}
> +
> +	dev = nand_curr_device;
> +	if (dev < 0 || dev >= CONFIG_SYS_MAX_NAND_DEVICE ||
> +		!nand_info[dev].name) {
> +		printf("%s: invalid nand device\n", __func__);
> +		return -1;
> +	}
> +
> +	nand = &nand_info[dev];
> +
> +	if (op == DFU_OP_READ)
> +		ret = nand_read_skip_bad(nand, start, &count, &actual,
> +				nand->size, buf);
> +	else
> +		ret = nand_write_skip_bad(nand, start, &count, &actual,
> +				nand->size, buf, 0);

Shouldn't "lim" be "dfu->data.nand.start + dfu->data.nand.size - start"?

This should let you get rid of the bounds check above, which doesn't  
factor in bad blocks that have yet to be encountered.

> +	if (ret != 0) {
> +		printf("%s: nand_%s_skip_bad call failed at %llx!\n",
> +				__func__, op == DFU_OP_READ ? "read" :  
> "write",
> +				start);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Find out where we stopped writing data.  This can be deeper  
> into
> +	 * the NAND than we expected due to having to skip bad blocks.   
> So
> +	 * we must take this into account for the next write, if any.
> +	 */
> +	if (actual > count) {
> +		printf("%s: skipped 0x%x bad bytes at 0x%llx\n",  
> __func__,
> +				actual - count, start);
> +		dfu->bad_skip += actual - count;
> +	}

Hmm, the message suggests the bad blocks are at "start" rather than  
somewhere between "start" and "start + actual".

nand_write_skip_bad() already prints a message when actually skipping  
the block, so you probably don't need any print here.

-Scott