| Message ID | 1355153072-6047-1-git-send-email-panto@antoniou-consulting.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Scott Wood |
| Headers | show |
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.
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
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
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
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.
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
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_ */
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