[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Pantelis Antoniou]

Introduce on-the fly DFU NAND support.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/dfu/Makefile   |   1 +
 drivers/dfu/dfu.c      |   7 ++
 drivers/dfu/dfu_nand.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/dfu.h          |  23 ++++++
 4 files changed, 225 insertions(+)
 create mode 100644 drivers/dfu/dfu_nand.c

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 fb9b417..1972b17 100644
--- a/drivers/dfu/dfu.c
+++ b/drivers/dfu/dfu.c
@@ -234,6 +234,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;
 	}
 
@@ -263,6 +265,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;
 	}
 
@@ -285,6 +289,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..9ea5f0c
--- /dev/null
+++ b/drivers/dfu/dfu_nand.c
@@ -0,0 +1,194 @@
+/*
+ * dfu_nand.c -- DFU for NAND routines.
+ *
+ * Copyright (C) 2012 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)
+{
+	char cmd_buf[DFU_CMD_BUF_SIZE];
+	u64 start, count;
+	int ret;
+	int dev;
+	loff_t actual;
+
+	/* 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;
+	}
+
+	sprintf(cmd_buf, "nand %s %p %llx %llx",
+		op == DFU_OP_READ ? "read" : "write",
+		 buf, start, count);
+
+	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
+	ret = run_command(cmd_buf, 0);
+
+	/* find out how much actual bytes have been written */
+	/* the difference is the amount of skip we must add from now on */
+	actual = nand_extent_skip_bad(&nand_info[dev], start, count);
+	if (actual == (loff_t)-1) {
+		printf("nand_extend_skip_bad: error!\n");
+		return ret;
+	}
+
+	if (actual > (start + count)) {
+		debug("%s: skipped %llx bad bytes at %llx\n", __func__,
+				actual - (start + count), start);
+		dfu->bad_skip += (u32)(actual - (start + count));
+	}
+
+	return ret;
+}
+
+static inline int nand_block_write(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
+{
+	return nand_block_op(DFU_OP_WRITE, dfu, offset, buf, len);
+}
+
+static inline int nand_block_read(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
+{
+	return nand_block_op(DFU_OP_READ, dfu, offset, buf, len);
+}
+
+static int dfu_write_medium_nand(struct dfu_entity *dfu,
+		u64 offset, void *buf, long *len)
+{
+	int ret = -1;
+
+	switch (dfu->layout) {
+	case DFU_RAW_ADDR:
+		ret = nand_block_write(dfu, offset, buf, len);
+		break;
+	default:
+		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
+		       dfu_get_layout(dfu->layout));
+	}
+
+	return ret;
+}
+
+static int dfu_read_medium_nand(struct dfu_entity *dfu, u64 offset, void *buf,
+		long *len)
+{
+	int ret = -1;
+
+	switch (dfu->layout) {
+	case DFU_RAW_ADDR:
+		ret = nand_block_read(dfu, offset, buf, len);
+		break;
+	default:
+		printf("%s: Layout (%s) not (yet) supported!\n", __func__,
+		       dfu_get_layout(dfu->layout));
+	}
+
+	return ret;
+}
+
+extern int mtdparts_init(void);
+extern struct part_info* mtd_part_info(struct mtd_device *dev, unsigned int part_num);
+extern int find_dev_and_part(const char *id, struct mtd_device **dev,
+		u8 *part_num, struct part_info **part);
+
+
+int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
+{
+	char *st;
+	int ret, dev, part;
+
+	dfu->dev_type = DFU_DEV_NAND;
+	st = strsep(&s, " ");
+	if (!strcmp(st, "raw")) {
+		dfu->layout = DFU_RAW_ADDR;
+		dfu->data.nand.start = simple_strtoul(s, &s, 16);
+		s++;
+		dfu->data.nand.size = simple_strtoul(s, &s, 16);
+	} else if (!strcmp(st, "part")) {
+		char mtd_id[32];
+		struct mtd_device *mtd_dev;
+		u8 part_num;
+		struct part_info *pi;
+
+		dfu->layout = DFU_RAW_ADDR;
+
+		dev = simple_strtoul(s, &s, 10);
+		s++;
+		part = simple_strtoul(s, &s, 10);
+
+		sprintf(mtd_id, "%s%d,%d", "nand", dev, part - 1);
+		printf("using id '%s'\n", mtd_id);
+
+		mtdparts_init();
+
+		ret = find_dev_and_part(mtd_id, &mtd_dev, &part_num, &pi);
+		if (ret != 0) {
+			printf("Could not locate '%s'\n", mtd_id);
+			return -1;
+		}
+
+		dfu->data.nand.start = pi->offset;
+		dfu->data.nand.size = pi->size;
+
+	} else {
+		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		return -1;
+	}
+
+	dfu->read_medium = dfu_read_medium_nand;
+	dfu->write_medium = dfu_write_medium_nand;
+
+	/* initial state */
+	dfu->inited = 0;
+
+	return 0;
+}
diff --git a/include/dfu.h b/include/dfu.h
index 2847ef8..0454762 100644
--- a/include/dfu.h
+++ b/include/dfu.h
@@ -52,6 +52,15 @@ struct mmc_internal_data {
 	unsigned int part;
 };
 
+struct nand_internal_data {
+	/* RAW programming */
+	u64 start;
+	u64 size;
+
+	unsigned int dev;
+	unsigned int part;
+};
+
 static inline unsigned int get_mmc_blk_size(int dev)
 {
 	return find_mmc_device(dev)->read_bl_len;
@@ -71,6 +80,7 @@ struct dfu_entity {
 
 	union {
 		struct mmc_internal_data mmc;
+		struct nand_internal_data nand;
 	} data;
 
 	int (*read_medium)(struct dfu_entity *dfu,
@@ -91,6 +101,8 @@ struct dfu_entity {
 	long r_left;
 	long b_left;
 
+	u32 bad_skip;	/* for nand use */
+
 	unsigned int inited : 1;
 };
 
@@ -115,4 +127,15 @@ static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 	return -1;
 }
 #endif
+
+#ifdef CONFIG_DFU_NAND
+extern int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s);
+#else
+static inline int dfu_fill_entity_nand(struct dfu_entity *dfu, char *s)
+{
+	puts("NAND support not available!\n");
+	return -1;
+}
+#endif
+
 #endif /* __DFU_ENTITY_H_ */
-- 
1.7.12

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Scott Wood]

On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> Introduce on-the fly DFU NAND support.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/dfu/Makefile   |   1 +
>  drivers/dfu/dfu.c      |   7 ++
>  drivers/dfu/dfu_nand.c | 194  
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/dfu.h          |  23 ++++++
>  4 files changed, 225 insertions(+)
>  create mode 100644 drivers/dfu/dfu_nand.c

What is DFU?  I don't see anything in README or doc/, despite there  
already being CONFIG symbols for it.

> +static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
> +			u64 offset, void *buf, long *len)
> +{
> +	char cmd_buf[DFU_CMD_BUF_SIZE];
> +	u64 start, count;
> +	int ret;
> +	int dev;
> +	loff_t actual;
> +
> +	/* 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;
> +	}
> +
> +	sprintf(cmd_buf, "nand %s %p %llx %llx",
> +		op == DFU_OP_READ ? "read" : "write",
> +		 buf, start, count);
> +
> +	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> +	ret = run_command(cmd_buf, 0);

Why not use the C interface to NAND?

> +	/* find out how much actual bytes have been written */
> +	/* the difference is the amount of skip we must add from now on  
> */
> +	actual = nand_extent_skip_bad(&nand_info[dev], start, count);

...especially since you already need to interact with it here?

-Scott

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Tom Rini]

On Mon, Dec 10, 2012 at 07:09:55PM -0600, Scott Wood wrote:
> On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> >Introduce on-the fly DFU NAND support.
> >
> >Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >---
> > drivers/dfu/Makefile   |   1 +
> > drivers/dfu/dfu.c      |   7 ++
> > drivers/dfu/dfu_nand.c | 194
> >+++++++++++++++++++++++++++++++++++++++++++++++++
> > include/dfu.h          |  23 ++++++
> > 4 files changed, 225 insertions(+)
> > create mode 100644 drivers/dfu/dfu_nand.c
> 
> What is DFU?  I don't see anything in README or doc/, despite there
> already being CONFIG symbols for it.

Pah... Yes, we let that one slip past.  DFU is USB Device Firmware
Upgrade, something from the openmoko folks (roughly) that various
devices support for sending payload via USB to be written to $flash.

> >+static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
> >+			u64 offset, void *buf, long *len)
> >+{
> >+	char cmd_buf[DFU_CMD_BUF_SIZE];
> >+	u64 start, count;
> >+	int ret;
> >+	int dev;
> >+	loff_t actual;
> >+
> >+	/* 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;
> >+	}
> >+
> >+	sprintf(cmd_buf, "nand %s %p %llx %llx",
> >+		op == DFU_OP_READ ? "read" : "write",
> >+		 buf, start, count);
> >+
> >+	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> >+	ret = run_command(cmd_buf, 0);
> 
> Why not use the C interface to NAND?
> 
> >+	/* find out how much actual bytes have been written */
> >+	/* the difference is the amount of skip we must add from now on
> >*/
> >+	actual = nand_extent_skip_bad(&nand_info[dev], start, count);
> 
> ...especially since you already need to interact with it here?

I've been talking with Pantelis about this as well and in short, this
series adds NAND support ala MMC (which is to say, (ab)using the command
interface).  I think he was thinking we need a bit more generic help to
avoid having to duplicate the code the command interface also uses
(state, sanity checking), iirc.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121210/d8edb0a1/attachment.pgp>

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Lukasz Majewski]

Hi Scott,

> On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> > Introduce on-the fly DFU NAND support.
> > 
> > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> > ---
> >  drivers/dfu/Makefile   |   1 +
> >  drivers/dfu/dfu.c      |   7 ++
> >  drivers/dfu/dfu_nand.c | 194  
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/dfu.h          |  23 ++++++
> >  4 files changed, 225 insertions(+)
> >  create mode 100644 drivers/dfu/dfu_nand.c
> 
> What is DFU?  I don't see anything in README or doc/, despite there  
> already being CONFIG symbols for it.

DFU means Device Firmware Upgrade. It is a relatively simple protocol
to update firmware on the target (mainly with small files - e.g.
uImage).

For more information please skim:
http://www.usb.org/developers/devclass_docs/usbdfu10.pdf


> 
> > +static int nand_block_op(enum dfu_nand_op op, struct dfu_entity
> > *dfu,
> > +			u64 offset, void *buf, long *len)
> > +{
> > +	char cmd_buf[DFU_CMD_BUF_SIZE];
> > +	u64 start, count;
> > +	int ret;
> > +	int dev;
> > +	loff_t actual;
> > +
> > +	/* 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;
> > +	}
> > +
> > +	sprintf(cmd_buf, "nand %s %p %llx %llx",
> > +		op == DFU_OP_READ ? "read" : "write",
> > +		 buf, start, count);
> > +
> > +	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> > +	ret = run_command(cmd_buf, 0);
> 
> Why not use the C interface to NAND?

We also support there eMMC (both with raw and file systems). Moreover
we had this discussion some time ago (if we shall use "command" or
native C API). 


> 
> > +	/* find out how much actual bytes have been written */
> > +	/* the difference is the amount of skip we must add from
> > now on */
> > +	actual = nand_extent_skip_bad(&nand_info[dev], start,
> > count);
> 
> ...especially since you already need to interact with it here?
> 
> -Scott



-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Pantelis Antoniou]

Hi Scott,

On Dec 11, 2012, at 3:09 AM, Scott Wood wrote:

> On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
>> Introduce on-the fly DFU NAND support.
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/dfu/Makefile   |   1 +
>> drivers/dfu/dfu.c      |   7 ++
>> drivers/dfu/dfu_nand.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
>> include/dfu.h          |  23 ++++++
>> 4 files changed, 225 insertions(+)
>> create mode 100644 drivers/dfu/dfu_nand.c
> 
> What is DFU?  I don't see anything in README or doc/, despite there already being CONFIG symbols for it.
> 

This gets answered by a following email.

>> +static int nand_block_op(enum dfu_nand_op op, struct dfu_entity *dfu,
>> +			u64 offset, void *buf, long *len)
>> +{
>> +	char cmd_buf[DFU_CMD_BUF_SIZE];
>> +	u64 start, count;
>> +	int ret;
>> +	int dev;
>> +	loff_t actual;
>> +
>> +	/* 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;
>> +	}
>> +
>> +	sprintf(cmd_buf, "nand %s %p %llx %llx",
>> +		op == DFU_OP_READ ? "read" : "write",
>> +		 buf, start, count);
>> +
>> +	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
>> +	ret = run_command(cmd_buf, 0);
> 
> Why not use the C interface to NAND?

Sigh. That's what I'm working on right now. The original implementation for mmc uses the user
facing cmd api, so that's what I'm using for nand too.

The problem is that I see, is that by using this method (dfu_mmc|dfu_nand|dfu_foo) this piece of code 
gets duplicated for each different dfu target.

I was told that Device Model will fix everything, eventually, so I abandoned anything more generic than
that. I will follow up with a patch that uses the C interface, and hope that sometime in the
future we can consolidate. Maybe. 

> 
>> +	/* find out how much actual bytes have been written */
>> +	/* the difference is the amount of skip we must add from now on */
>> +	actual = nand_extent_skip_bad(&nand_info[dev], start, count);
> 
> ...especially since you already need to interact with it here?
> 
> -Scott

Regards

-- Pantelis

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Scott Wood]

On 12/11/2012 01:56:25 AM, Lukasz Majewski wrote:
> Hi Scott,
> 
> > On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> > > +	sprintf(cmd_buf, "nand %s %p %llx %llx",
> > > +		op == DFU_OP_READ ? "read" : "write",
> > > +		 buf, start, count);
> > > +
> > > +	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> > > +	ret = run_command(cmd_buf, 0);
> >
> > Why not use the C interface to NAND?
> 
> We also support there eMMC (both with raw and file systems). Moreover
> we had this discussion some time ago (if we shall use "command" or
> native C API).

I don't see how "nand %s %p %llx %llx" supports anything that the NAND  
C API doesn't support.

I can't follow every discussion that happens on this list, on the off  
chance it might eventually become relevant to NAND.  "Some time ago" is  
not an easily followed reference to the archives.

-Scott

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Scott Wood]

On 12/10/2012 07:16:50 PM, Tom Rini wrote:
> On Mon, Dec 10, 2012 at 07:09:55PM -0600, Scott Wood wrote:
> > On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> > >+	sprintf(cmd_buf, "nand %s %p %llx %llx",
> > >+		op == DFU_OP_READ ? "read" : "write",
> > >+		 buf, start, count);
> > >+
> > >+	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> > >+	ret = run_command(cmd_buf, 0);
> >
> > Why not use the C interface to NAND?
> >
> > >+	/* find out how much actual bytes have been written */
> > >+	/* the difference is the amount of skip we must add from now on
> > >*/
> > >+	actual = nand_extent_skip_bad(&nand_info[dev], start, count);
> >
> > ...especially since you already need to interact with it here?
> 
> I've been talking with Pantelis about this as well and in short, this
> series adds NAND support ala MMC (which is to say, (ab)using the  
> command
> interface).  I think he was thinking we need a bit more generic help  
> to
> avoid having to duplicate the code the command interface also uses
> (state, sanity checking), iirc.

Some elaboration on what exactly he's relying on from the command line  
interface would be nice.

-Scott

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Tom Rini]

On Tue, Dec 11, 2012 at 04:24:37PM -0600, Scott Wood wrote:
> On 12/10/2012 07:16:50 PM, Tom Rini wrote:
> >On Mon, Dec 10, 2012 at 07:09:55PM -0600, Scott Wood wrote:
> >> On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> >> >+	sprintf(cmd_buf, "nand %s %p %llx %llx",
> >> >+		op == DFU_OP_READ ? "read" : "write",
> >> >+		 buf, start, count);
> >> >+
> >> >+	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> >> >+	ret = run_command(cmd_buf, 0);
> >>
> >> Why not use the C interface to NAND?
> >>
> >> >+	/* find out how much actual bytes have been written */
> >> >+	/* the difference is the amount of skip we must add from now on
> >> >*/
> >> >+	actual = nand_extent_skip_bad(&nand_info[dev], start, count);
> >>
> >> ...especially since you already need to interact with it here?
> >
> >I've been talking with Pantelis about this as well and in short, this
> >series adds NAND support ala MMC (which is to say, (ab)using the
> >command
> >interface).  I think he was thinking we need a bit more generic
> >help to
> >avoid having to duplicate the code the command interface also uses
> >(state, sanity checking), iirc.
> 
> Some elaboration on what exactly he's relying on from the command
> line interface would be nice.

My gmane-fu is failing me, but the actual 4/7 parts of
http://search.gmane.org/?query=dfu+mmc+%224%2F7%22&author=&group=gmane.comp.boot-loaders.u-boot&sort=revdate&DEFAULTOP=and&%3E=Next&xP=Zdfu%09Zmmc%094%097&xFILTERS=Gcomp.boot-loaders.u-boot---A
show the discussion.  In short, for MMC writing it's more complex
because we have a number of layouts we need to deal with (raw, FAT,
other), but yes, we should in the end migrate things to using the API
rather than abusing the command infrastructure.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121211/74ba1782/attachment.pgp>

[U-Boot] [PATCH] [DFU] Implement NAND dfu support

[Lukasz Majewski]

Hi Scott,

> On 12/11/2012 01:56:25 AM, Lukasz Majewski wrote:
> > Hi Scott,
> > 
> > > On 12/10/2012 09:24:32 AM, Pantelis Antoniou wrote:
> > > > +	sprintf(cmd_buf, "nand %s %p %llx %llx",
> > > > +		op == DFU_OP_READ ? "read" : "write",
> > > > +		 buf, start, count);
> > > > +
> > > > +	debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf);
> > > > +	ret = run_command(cmd_buf, 0);
> > >
> > > Why not use the C interface to NAND?
> > 
> > We also support there eMMC (both with raw and file systems).
> > Moreover we had this discussion some time ago (if we shall use
> > "command" or native C API).
> 
> I don't see how "nand %s %p %llx %llx" supports anything that the
> NAND C API doesn't support.
> 
> I can't follow every discussion that happens on this list, on the
> off chance it might eventually become relevant to NAND.  "Some time
> ago" is not an easily followed reference to the archives.

I understand. Please consult following thread:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/134397


> 
> -Scott



-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group