| Message ID | 1362764249-15547-3-git-send-email-trini@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
Hi Tom, > From: Pantelis Antoniou <panto@antoniou-consulting.com> > > Previously we didn't support upload/download larger than available > memory. This is pretty bad when you have to update your root > filesystem for example. > > This patch removes that limitation (and the crashes when you > transfered any file larger than 4MB) by making raw image writes be > done in chunks and making file maximum size be configurable. > > The sequence number is a 16 bit counter; make sure we handle rollover > correctly. This fixes the wrong transfers for large (> 256MB) images. > > Also utilize a variable to handle initialization, so that we don't > rely on just the counter sent by the host. > > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> > Signed-off-by: Tom Rini <trini@ti.com> Acked-by: Lukasz Majewski <l.majewski@samsung.com> Test-hw: Exynos 4210 (Trats) Tested-by: Lukasz Majewski <l.majewski@samsung.com> > --- > Changes in v5: > - Rework Pantelis' "dfu: Support larger than memory transfers" to not > break filesystem writing > > Changes in v4: None > Changes in v3: None > Changes in v2: None > > README | 7 ++ > drivers/dfu/dfu.c | 245 > ++++++++++++++++++++++++++++++++++++++----------- > drivers/dfu/dfu_mmc.c | 116 +++++++++++++++++------ > include/dfu.h | 26 +++++- 4 files changed, 310 > insertions(+), 84 deletions(-) > > diff --git a/README b/README > index 900ae5f..154b82f 100644 > --- a/README > +++ b/README > @@ -1338,6 +1338,13 @@ The following options need to be configured: > CONFIG_DFU_MMC > This enables support for exposing (e)MMC 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 > + the buffer once we've been given the whole file. > Define > + this to the maximum filesize (in bytes) for the > buffer. > + Default is 4 MiB if undefined. > + > - Journaling Flash filesystem support: > CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, > CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index e8477fb..2fecf77 100644 > --- a/drivers/dfu/dfu.c > +++ b/drivers/dfu/dfu.c > @@ -44,90 +44,229 @@ static int dfu_find_alt_num(const char *s) > static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) > dfu_buf[DFU_DATA_BUF_SIZE]; > > +static int dfu_write_buffer_drain(struct dfu_entity *dfu) > +{ > + long w_size; > + int ret; > + > + /* flush size? */ > + w_size = dfu->i_buf - dfu->i_buf_start; > + if (w_size == 0) > + return 0; > + > + /* update CRC32 */ > + dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size); > + > + ret = dfu->write_medium(dfu, dfu->offset, dfu->i_buf_start, > &w_size); > + if (ret) > + debug("%s: Write error!\n", __func__); > + > + /* point back */ > + dfu->i_buf = dfu->i_buf_start; > + > + /* update offset */ > + dfu->offset += w_size; > + > + puts("#"); > + > + return ret; > +} > + > int dfu_write(struct dfu_entity *dfu, void *buf, int size, int > blk_seq_num) { > - static unsigned char *i_buf; > - static int i_blk_seq_num; > - long w_size = 0; > int ret = 0; > + int tret; > + > + debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x " > + "offset: 0x%llx bufoffset: 0x%x\n", > + __func__, dfu->name, buf, size, blk_seq_num, > dfu->offset, > + dfu->i_buf - dfu->i_buf_start); > + > + if (!dfu->inited) { > + /* initial state */ > + dfu->crc = 0; > + dfu->offset = 0; > + dfu->i_blk_seq_num = 0; > + dfu->i_buf_start = dfu_buf; > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > + dfu->i_buf = dfu->i_buf_start; > + > + dfu->inited = 1; > + } > > - debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: > 0x%p\n", > - __func__, dfu->name, buf, size, blk_seq_num, i_buf); > + if (dfu->i_blk_seq_num != blk_seq_num) { > + printf("%s: Wrong sequence number! [%d] [%d]\n", > + __func__, dfu->i_blk_seq_num, blk_seq_num); > + return -1; > + } > > - if (blk_seq_num == 0) { > - i_buf = dfu_buf; > - i_blk_seq_num = 0; > + /* DFU 1.1 standard says: > + * The wBlockNum field is a block sequence number. It > increments each > + * time a block is transferred, wrapping to zero from > 65,535. It is used > + * to provide useful context to the DFU loader in the > device." > + * > + * This means that it's a 16 bit counter that roll-overs at > + * 0xffff -> 0x0000. By having a typical 4K transfer block > + * we roll-over at exactly 256MB. Not very fun to debug. > + * > + * Handling rollover, and having an inited variable, > + * makes things work. > + */ > + > + /* handle rollover */ > + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff; > + > + /* flush buffer if overflow */ > + if ((dfu->i_buf + size) > dfu->i_buf_end) { > + tret = dfu_write_buffer_drain(dfu); > + if (ret == 0) > + ret = tret; > } > > - if (i_blk_seq_num++ != blk_seq_num) { > - printf("%s: Wrong sequence number! [%d] [%d]\n", > - __func__, i_blk_seq_num, blk_seq_num); > + /* we should be in buffer now (if not then size too large) */ > + if ((dfu->i_buf + size) > dfu->i_buf_end) { > + printf("%s: Wrong size! [%d] [%d] - %d\n", > + __func__, dfu->i_blk_seq_num, blk_seq_num, > size); return -1; > } > > - memcpy(i_buf, buf, size); > - i_buf += size; > + memcpy(dfu->i_buf, buf, size); > + dfu->i_buf += size; > > + /* if end or if buffer full flush */ > + if (size == 0 || (dfu->i_buf + size) > dfu->i_buf_end) { > + tret = dfu_write_buffer_drain(dfu); > + if (ret == 0) > + ret = tret; > + } > + > + /* end? */ > if (size == 0) { > - /* Integrity check (if needed) */ > - debug("%s: %s %d [B] CRC32: 0x%x\n", __func__, > dfu->name, > - i_buf - dfu_buf, crc32(0, dfu_buf, i_buf - > dfu_buf)); > + /* Now try and flush to the medium if needed. */ > + if (dfu->flush_medium) > + ret = dfu->flush_medium(dfu); > + printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); > > - w_size = i_buf - dfu_buf; > - ret = dfu->write_medium(dfu, dfu_buf, &w_size); > - if (ret) > - debug("%s: Write error!\n", __func__); > + /* clear everything */ > + dfu->crc = 0; > + dfu->offset = 0; > + dfu->i_blk_seq_num = 0; > + dfu->i_buf_start = dfu_buf; > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > + dfu->i_buf = dfu->i_buf_start; > + > + dfu->inited = 0; > > - i_blk_seq_num = 0; > - i_buf = NULL; > - return ret; > } > > - return ret; > + return ret = 0 ? size : ret; > +} > + > +static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, > int size) +{ > + long chunk; > + int ret, readn; > + > + readn = 0; > + while (size > 0) { > + > + /* get chunk that can be read */ > + chunk = min(size, dfu->b_left); > + /* consume */ > + if (chunk > 0) { > + memcpy(buf, dfu->i_buf, chunk); > + dfu->crc = crc32(dfu->crc, buf, chunk); > + dfu->i_buf += chunk; > + dfu->b_left -= chunk; > + size -= chunk; > + buf += chunk; > + readn += chunk; > + } > + > + /* all done */ > + if (size > 0) { > + /* no more to read */ > + if (dfu->r_left == 0) > + break; > + > + dfu->i_buf = dfu->i_buf_start; > + dfu->b_left = dfu->i_buf_end - > dfu->i_buf_start; + > + /* got to read, but buffer is empty */ > + if (dfu->b_left > dfu->r_left) > + dfu->b_left = dfu->r_left; > + ret = dfu->read_medium(dfu, dfu->offset, > dfu->i_buf, > + &dfu->b_left); > + if (ret != 0) { > + debug("%s: Read error!\n", __func__); > + return ret; > + } > + dfu->offset += dfu->b_left; > + dfu->r_left -= dfu->b_left; > + > + puts("#"); > + } > + } > + > + return readn; > } > > int dfu_read(struct dfu_entity *dfu, void *buf, int size, int > blk_seq_num) { > - static unsigned char *i_buf; > - static int i_blk_seq_num; > - static long r_size; > - static u32 crc; > int ret = 0; > > debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: > 0x%p\n", > - __func__, dfu->name, buf, size, blk_seq_num, i_buf); > - > - if (blk_seq_num == 0) { > - i_buf = dfu_buf; > - ret = dfu->read_medium(dfu, i_buf, &r_size); > - debug("%s: %s %ld [B]\n", __func__, dfu->name, > r_size); > - i_blk_seq_num = 0; > - /* Integrity check (if needed) */ > - crc = crc32(0, dfu_buf, r_size); > + __func__, dfu->name, buf, size, blk_seq_num, > dfu->i_buf); + > + if (!dfu->inited) { > + ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left); > + if (ret != 0) { > + debug("%s: failed to get r_left\n", > __func__); > + return ret; > + } > + > + debug("%s: %s %ld [B]\n", __func__, dfu->name, > dfu->r_left); + > + dfu->i_blk_seq_num = 0; > + dfu->crc = 0; > + dfu->offset = 0; > + dfu->i_buf_start = dfu_buf; > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > + dfu->i_buf = dfu->i_buf_start; > + dfu->b_left = 0; > + > + dfu->inited = 1; > } > > - if (i_blk_seq_num++ != blk_seq_num) { > + if (dfu->i_blk_seq_num != blk_seq_num) { > printf("%s: Wrong sequence number! [%d] [%d]\n", > - __func__, i_blk_seq_num, blk_seq_num); > + __func__, dfu->i_blk_seq_num, blk_seq_num); > return -1; > } > + /* handle rollover */ > + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff; > > - if (r_size >= size) { > - memcpy(buf, i_buf, size); > - i_buf += size; > - r_size -= size; > - return size; > - } else { > - memcpy(buf, i_buf, r_size); > - i_buf += r_size; > - debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, > crc); > - puts("UPLOAD ... done\nCtrl+C to exit ...\n"); > + ret = dfu_read_buffer_fill(dfu, buf, size); > + if (ret < 0) { > + printf("%s: Failed to fill buffer\n", __func__); > + return -1; > + } > + > + if (ret < size) { > + debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, > dfu->crc); > + puts("\nUPLOAD ... done\nCtrl+C to exit ...\n"); > > - i_buf = NULL; > - i_blk_seq_num = 0; > - crc = 0; > - return r_size; > + dfu->i_blk_seq_num = 0; > + dfu->crc = 0; > + dfu->offset = 0; > + dfu->i_buf_start = dfu_buf; > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > + dfu->i_buf = dfu->i_buf_start; > + dfu->b_left = 0; > + > + dfu->inited = 0; > } > + > return ret; > } > > diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c > index 083d745..2d5ffd8 100644 > --- a/drivers/dfu/dfu_mmc.c > +++ b/drivers/dfu/dfu_mmc.c > @@ -22,6 +22,7 @@ > #include <common.h> > #include <malloc.h> > #include <errno.h> > +#include <div64.h> > #include <dfu.h> > > enum dfu_mmc_op { > @@ -29,36 +30,67 @@ enum dfu_mmc_op { > DFU_OP_WRITE, > }; > > +static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) > + > dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE]; +static long > dfu_file_buf_len; + > static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu, > - void *buf, long *len) > + u64 offset, void *buf, long *len) > { > char cmd_buf[DFU_CMD_BUF_SIZE]; > + u32 blk_start, blk_count; > + > + /* > + * We must ensure that we work in lba_blk_size chunks, so > ALIGN > + * this value. > + */ > + *len = ALIGN(*len, dfu->data.mmc.lba_blk_size); > + > + blk_start = dfu->data.mmc.lba_start + > + (u32)lldiv(offset, > dfu->data.mmc.lba_blk_size); > + blk_count = *len / dfu->data.mmc.lba_blk_size; > + if (*len + blk_start > > + dfu->data.mmc.lba_size * > dfu->data.mmc.lba_size) { > + puts("Request would exceed designated area!\n"); > + return -EINVAL; > + } > > - sprintf(cmd_buf, "mmc %s 0x%x %x %x", > + sprintf(cmd_buf, "mmc %s %p %x %x", > op == DFU_OP_READ ? "read" : "write", > - (unsigned int) buf, > - dfu->data.mmc.lba_start, > - dfu->data.mmc.lba_size); > - > - if (op == DFU_OP_READ) > - *len = dfu->data.mmc.lba_blk_size * > dfu->data.mmc.lba_size; > + buf, blk_start, blk_count); > > debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); > return run_command(cmd_buf, 0); > } > > -static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, > long *len) +static inline int mmc_block_write(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len) > +{ > + return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len); > +} > + > +static inline int mmc_block_read(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len) > { > - return mmc_block_op(DFU_OP_WRITE, dfu, buf, len); > + return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len); > } > > -static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, > long *len) +static int mmc_file_buffer(struct dfu_entity *dfu, void > *buf, long *len) { > - return mmc_block_op(DFU_OP_READ, dfu, buf, len); > + if (dfu_file_buf_len + *len > CONFIG_SYS_DFU_MAX_FILE_SIZE) { > + dfu_file_buf_len = 0; > + return -EINVAL; > + } > + > + /* Add to the current buffer. */ > + memcpy(dfu_file_buf + dfu_file_buf_len, buf, *len); > + dfu_file_buf_len += *len; > + > + return 0; > } > > static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, > - void *buf, long *len) > + u64 offset, void *buf, long *len) > { > char cmd_buf[DFU_CMD_BUF_SIZE]; > char *str_env; > @@ -70,8 +102,15 @@ static int mmc_file_op(enum dfu_mmc_op op, struct > dfu_entity *dfu, op == DFU_OP_READ ? "load" : "write", > dfu->data.mmc.dev, dfu->data.mmc.part, > (unsigned int) buf, dfu->name, *len); > + if (op == DFU_OP_READ && offset != 0) > + sprintf(cmd_buf + strlen(cmd_buf), " %llx", > offset); break; > case DFU_FS_EXT4: > + if (offset != 0) { > + debug("%s: Offset value %llx != 0 not > supported!\n", > + __func__, offset); > + return -1; > + } > sprintf(cmd_buf, "ext4%s mmc %d:%d /%s 0x%x %ld", > op == DFU_OP_READ ? "load" : "write", > dfu->data.mmc.dev, dfu->data.mmc.part, > @@ -80,6 +119,7 @@ static int mmc_file_op(enum dfu_mmc_op op, struct > dfu_entity *dfu, default: > printf("%s: Layout (%s) not (yet) supported!\n", > __func__, dfu_get_layout(dfu->layout)); > + return -1; > } > > debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); > @@ -102,27 +142,24 @@ static int mmc_file_op(enum dfu_mmc_op op, > struct dfu_entity *dfu, return ret; > } > > -static inline int mmc_file_write(struct dfu_entity *dfu, void *buf, > long *len) +static inline int mmc_file_read(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len) > { > - return mmc_file_op(DFU_OP_WRITE, dfu, buf, len); > + return mmc_file_op(DFU_OP_READ, dfu, offset, buf, len); > } > > -static inline int mmc_file_read(struct dfu_entity *dfu, void *buf, > long *len) -{ > - return mmc_file_op(DFU_OP_READ, dfu, buf, len); > -} > - > -int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long > *len) +int dfu_write_medium_mmc(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len) > { > int ret = -1; > > switch (dfu->layout) { > case DFU_RAW_ADDR: > - ret = mmc_block_write(dfu, buf, len); > + ret = mmc_block_write(dfu, offset, buf, len); > break; > case DFU_FS_FAT: > case DFU_FS_EXT4: > - ret = mmc_file_write(dfu, buf, len); > + ret = mmc_file_buffer(dfu, buf, len); > break; > default: > printf("%s: Layout (%s) not (yet) supported!\n", > __func__, @@ -132,17 +169,34 @@ int dfu_write_medium_mmc(struct > dfu_entity *dfu, void *buf, long *len) return ret; > } > > -int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) > +int dfu_flush_medium_mmc(struct dfu_entity *dfu) > +{ > + int ret = 0; > + > + if (dfu->layout != DFU_RAW_ADDR) { > + /* Do stuff here. */ > + ret = mmc_file_op(DFU_OP_WRITE, dfu, 0, > &dfu_file_buf, > + &dfu_file_buf_len); > + > + /* Now that we're done */ > + dfu_file_buf_len = 0; > + } > + > + return ret; > +} > + > +int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void > *buf, > + long *len) > { > int ret = -1; > > switch (dfu->layout) { > case DFU_RAW_ADDR: > - ret = mmc_block_read(dfu, buf, len); > + ret = mmc_block_read(dfu, offset, buf, len); > break; > case DFU_FS_FAT: > case DFU_FS_EXT4: > - ret = mmc_file_read(dfu, buf, len); > + ret = mmc_file_read(dfu, offset, buf, len); > break; > default: > printf("%s: Layout (%s) not (yet) supported!\n", > __func__, @@ -181,13 +235,15 @@ int dfu_fill_entity_mmc(struct > dfu_entity *dfu, char *s) > mmc = find_mmc_device(dev); > if (mmc == NULL || mmc_init(mmc)) { > - printf("%s: could not find mmc device > #%d!\n", __func__, dev); > + printf("%s: could not find mmc device > #%d!\n", > + __func__, dev); > return -ENODEV; > } > > blk_dev = &mmc->block_dev; > if (get_partition_info(blk_dev, part, &partinfo) != > 0) { > - printf("%s: could not find partition #%d on > mmc device #%d!\n", > + printf("%s: could not find partition #%d " > + "on mmc device #%d!\n", > __func__, part, dev); > return -ENODEV; > } > @@ -208,6 +264,10 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, > char *s) > dfu->read_medium = dfu_read_medium_mmc; > dfu->write_medium = dfu_write_medium_mmc; > + dfu->flush_medium = dfu_flush_medium_mmc; > + > + /* initial state */ > + dfu->inited = 0; > > return 0; > } > diff --git a/include/dfu.h b/include/dfu.h > index 5350d79..5182c6c 100644 > --- a/include/dfu.h > +++ b/include/dfu.h > @@ -59,7 +59,10 @@ static inline unsigned int get_mmc_blk_size(int > dev) > #define DFU_NAME_SIZE 32 > #define DFU_CMD_BUF_SIZE 128 > -#define DFU_DATA_BUF_SIZE (1024*1024*4) /* 4 MiB */ > +#define DFU_DATA_BUF_SIZE (64 << 10) /* 64 KiB > */ +#ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE > +#define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20) /* 4 > MiB */ +#endif > > struct dfu_entity { > char name[DFU_NAME_SIZE]; > @@ -73,10 +76,27 @@ struct dfu_entity { > struct mmc_internal_data mmc; > } data; > > - int (*read_medium)(struct dfu_entity *dfu, void *buf, long > *len); > - int (*write_medium)(struct dfu_entity *dfu, void *buf, long > *len); > + int (*read_medium)(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len); > + > + int (*write_medium)(struct dfu_entity *dfu, > + u64 offset, void *buf, long *len); > + > + int (*flush_medium)(struct dfu_entity *dfu); > > struct list_head list; > + > + /* on the fly state */ > + u32 crc; > + u64 offset; > + int i_blk_seq_num; > + u8 *i_buf; > + u8 *i_buf_start; > + u8 *i_buf_end; > + long r_left; > + long b_left; > + > + unsigned int inited:1; > }; > > int dfu_config_entities(char *s, char *interface, int num);
Hi Tom, > > > From: Pantelis Antoniou <panto@antoniou-consulting.com> > > > > Previously we didn't support upload/download larger than available > > memory. This is pretty bad when you have to update your root > > filesystem for example. > > > > This patch removes that limitation (and the crashes when you > > transfered any file larger than 4MB) by making raw image writes be > > done in chunks and making file maximum size be configurable. > > > > The sequence number is a 16 bit counter; make sure we handle > > rollover correctly. This fixes the wrong transfers for large (> > > 256MB) images. > > > > Also utilize a variable to handle initialization, so that we don't > > rely on just the counter sent by the host. > > > > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> > > Signed-off-by: Tom Rini <trini@ti.com> > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > Test-hw: Exynos 4210 (Trats) > > Tested-by: Lukasz Majewski <l.majewski@samsung.com> Sorry but, I've found a regression for reading image from a file system. It happens with EXT4 mmc read (like uImage). mmc_file_op: ext4load mmc 0:2 /uImage 0x0 0 0x7f95dc98 ** File not found 0x0 ** dfu: Read error! ext4load params: ext4load - load binary file from a Ext4 filesystem Usage: ext4load <interface> <dev[:part]> [addr] [filename] [bytes] - load binary file 'filename' from 'dev' on 'interface' to address 'addr' from ext4 filesystem. All numeric parameters are assumed to be hex. Some parameters are wrong (buffer - 0x0) and some are switched (address and filename). Please find more details below: > > > --- > > Changes in v5: > > - Rework Pantelis' "dfu: Support larger than memory transfers" to > > not break filesystem writing > > > > Changes in v4: None > > Changes in v3: None > > Changes in v2: None > > > > README | 7 ++ > > drivers/dfu/dfu.c | 245 > > ++++++++++++++++++++++++++++++++++++++----------- > > drivers/dfu/dfu_mmc.c | 116 +++++++++++++++++------ > > include/dfu.h | 26 +++++- 4 files changed, 310 > > insertions(+), 84 deletions(-) > > > > +static int dfu_write_buffer_drain(struct dfu_entity *dfu) > > +{ > > + long w_size; > > + int ret; > > + > > + /* flush size? */ > > + w_size = dfu->i_buf - dfu->i_buf_start; > > + if (w_size == 0) > > + return 0; > > + > > + /* update CRC32 */ > > + dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size); > > + > > + ret = dfu->write_medium(dfu, dfu->offset, dfu->i_buf_start, > > &w_size); > > + if (ret) > > + debug("%s: Write error!\n", __func__); > > + > > + /* point back */ > > + dfu->i_buf = dfu->i_buf_start; > > + > > + /* update offset */ > > + dfu->offset += w_size; > > + > > + puts("#"); > > + > > + return ret; > > +} > > + > > int dfu_write(struct dfu_entity *dfu, void *buf, int size, int > > blk_seq_num) { > > - static unsigned char *i_buf; > > - static int i_blk_seq_num; > > - long w_size = 0; > > int ret = 0; > > + int tret; > > + > > + debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x " > > + "offset: 0x%llx bufoffset: 0x%x\n", > > + __func__, dfu->name, buf, size, blk_seq_num, > > dfu->offset, > > + dfu->i_buf - dfu->i_buf_start); > > + > > + if (!dfu->inited) { > > + /* initial state */ > > + dfu->crc = 0; > > + dfu->offset = 0; > > + dfu->i_blk_seq_num = 0; > > + dfu->i_buf_start = dfu_buf; > > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > > + dfu->i_buf = dfu->i_buf_start; > > + > > + dfu->inited = 1; > > + } > > > > - debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: > > 0x%p\n", > > - __func__, dfu->name, buf, size, blk_seq_num, i_buf); > > + if (dfu->i_blk_seq_num != blk_seq_num) { > > + printf("%s: Wrong sequence number! [%d] [%d]\n", > > + __func__, dfu->i_blk_seq_num, blk_seq_num); > > + return -1; > > + } > > > > - if (blk_seq_num == 0) { > > - i_buf = dfu_buf; > > - i_blk_seq_num = 0; > > + /* DFU 1.1 standard says: > > + * The wBlockNum field is a block sequence number. It > > increments each > > + * time a block is transferred, wrapping to zero from > > 65,535. It is used > > + * to provide useful context to the DFU loader in the > > device." > > + * > > + * This means that it's a 16 bit counter that roll-overs at > > + * 0xffff -> 0x0000. By having a typical 4K transfer block > > + * we roll-over at exactly 256MB. Not very fun to debug. > > + * > > + * Handling rollover, and having an inited variable, > > + * makes things work. > > + */ > > + > > + /* handle rollover */ > > + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff; > > + > > + /* flush buffer if overflow */ > > + if ((dfu->i_buf + size) > dfu->i_buf_end) { > > + tret = dfu_write_buffer_drain(dfu); > > + if (ret == 0) > > + ret = tret; > > } > > > > - if (i_blk_seq_num++ != blk_seq_num) { > > - printf("%s: Wrong sequence number! [%d] [%d]\n", > > - __func__, i_blk_seq_num, blk_seq_num); > > + /* we should be in buffer now (if not then size too large) > > */ > > + if ((dfu->i_buf + size) > dfu->i_buf_end) { > > + printf("%s: Wrong size! [%d] [%d] - %d\n", > > + __func__, dfu->i_blk_seq_num, blk_seq_num, > > size); return -1; > > } > > > > - memcpy(i_buf, buf, size); > > - i_buf += size; > > + memcpy(dfu->i_buf, buf, size); > > + dfu->i_buf += size; > > > > + /* if end or if buffer full flush */ > > + if (size == 0 || (dfu->i_buf + size) > dfu->i_buf_end) { > > + tret = dfu_write_buffer_drain(dfu); > > + if (ret == 0) > > + ret = tret; > > + } > > + > > + /* end? */ > > if (size == 0) { > > - /* Integrity check (if needed) */ > > - debug("%s: %s %d [B] CRC32: 0x%x\n", __func__, > > dfu->name, > > - i_buf - dfu_buf, crc32(0, dfu_buf, i_buf - > > dfu_buf)); > > + /* Now try and flush to the medium if needed. */ > > + if (dfu->flush_medium) > > + ret = dfu->flush_medium(dfu); > > + printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); > > > > - w_size = i_buf - dfu_buf; > > - ret = dfu->write_medium(dfu, dfu_buf, &w_size); > > - if (ret) > > - debug("%s: Write error!\n", __func__); > > + /* clear everything */ > > + dfu->crc = 0; > > + dfu->offset = 0; > > + dfu->i_blk_seq_num = 0; > > + dfu->i_buf_start = dfu_buf; > > + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); > > + dfu->i_buf = dfu->i_buf_start; > > + > > + dfu->inited = 0; > > > > - i_blk_seq_num = 0; > > - i_buf = NULL; > > - return ret; > > } > > > > - return ret; > > + return ret = 0 ? size : ret; > > +} > > + > > +static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, > > int size) +{ > > + long chunk; > > + int ret, readn; > > + > > + readn = 0; > > + while (size > 0) { > > + > > + /* get chunk that can be read */ > > + chunk = min(size, dfu->b_left); > > + /* consume */ > > + if (chunk > 0) { > > + memcpy(buf, dfu->i_buf, chunk); > > + dfu->crc = crc32(dfu->crc, buf, chunk); > > + dfu->i_buf += chunk; > > + dfu->b_left -= chunk; > > + size -= chunk; > > + buf += chunk; > > + readn += chunk; > > + } > > + > > + /* all done */ > > + if (size > 0) { > > + /* no more to read */ > > + if (dfu->r_left == 0) > > + break; > > + > > + dfu->i_buf = dfu->i_buf_start; > > + dfu->b_left = dfu->i_buf_end - > > dfu->i_buf_start; + > > + /* got to read, but buffer is empty */ > > + if (dfu->b_left > dfu->r_left) > > + dfu->b_left = dfu->r_left; > > + ret = dfu->read_medium(dfu, dfu->offset, > > dfu->i_buf, > > + &dfu->b_left); > > + if (ret != 0) { > > + debug("%s: Read error!\n", > > __func__); > > + return ret; > > + } > > + dfu->offset += dfu->b_left; > > + dfu->r_left -= dfu->b_left; > > + > > + puts("#"); > > + } > > + } > > + > > + return readn; > > } > > > > int dfu_read(struct dfu_entity *dfu, void *buf, int size, int > > blk_seq_num) { > > - static unsigned char *i_buf; > > - static int i_blk_seq_num; > > - static long r_size; > > - static u32 crc; > > int ret = 0; > > > > debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: > > 0x%p\n", > > - __func__, dfu->name, buf, size, blk_seq_num, i_buf); > > - > > - if (blk_seq_num == 0) { > > - i_buf = dfu_buf; > > - ret = dfu->read_medium(dfu, i_buf, &r_size); > > - debug("%s: %s %ld [B]\n", __func__, dfu->name, > > r_size); > > - i_blk_seq_num = 0; > > - /* Integrity check (if needed) */ > > - crc = crc32(0, dfu_buf, r_size); > > + __func__, dfu->name, buf, size, blk_seq_num, > > dfu->i_buf); + > > + if (!dfu->inited) { > > + ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left); ^^^^^^^^^^^^ this call causes read error. I suppose, that it is an initial "read". Does it read the whole file at once? The problem is that the command is fromatted in a wrong way. On the other hand write operations works on Trats.
Hi, experts: I want to know which specific nand chip supports ONFI interface spec? I have googled , but not find any product manual. Best wishes,
Hi, On Mon, Mar 11, 2013 at 5:06 PM, <TigerLiu@viatech.com.cn> wrote: > Hi, experts: > I want to know which specific nand chip supports ONFI interface spec? > I have googled , but not find any product manual. Spansion ML series and Micron MT29F series flashes have onfi read parameter page support. Thanks, Jagan. > > Best wishes, > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/11/2013 06:03 AM, Lukasz Majewski wrote: > Hi Tom, > > >> >>> From: Pantelis Antoniou <panto@antoniou-consulting.com> >>> >>> Previously we didn't support upload/download larger than >>> available memory. This is pretty bad when you have to update >>> your root filesystem for example. >>> >>> This patch removes that limitation (and the crashes when you >>> transfered any file larger than 4MB) by making raw image writes >>> be done in chunks and making file maximum size be >>> configurable. >>> >>> The sequence number is a 16 bit counter; make sure we handle >>> rollover correctly. This fixes the wrong transfers for large >>> (> 256MB) images. >>> >>> Also utilize a variable to handle initialization, so that we >>> don't rely on just the counter sent by the host. >>> >>> Signed-off-by: Pantelis Antoniou >>> <panto@antoniou-consulting.com> Signed-off-by: Tom Rini >>> <trini@ti.com> >> >> Acked-by: Lukasz Majewski <l.majewski@samsung.com> >> >> Test-hw: Exynos 4210 (Trats) >> >> Tested-by: Lukasz Majewski <l.majewski@samsung.com> > > Sorry but, I've found a regression for reading image from a file > system. It happens with EXT4 mmc read (like uImage). Can you please confirm read works as-is today? It's possible that when I tried I was using (what I've now called) a marginal SD card I've finally written too much to the front sectors on, but it wasn't working for me at all. I'll try and reproduce as well. That said, there is a problem: > mmc_file_op: ext4load mmc 0:2 /uImage 0x0 0 0x7f95dc98 ** File not > found 0x0 ** dfu: Read error! > > ext4load params: ext4load - load binary file from a Ext4 > filesystem > > Usage: ext4load <interface> <dev[:part]> [addr] [filename] [bytes] > - load binary file 'filename' from 'dev' on 'interface' to address > 'addr' from ext4 filesystem. All numeric parameters are assumed to > be hex. > > Some parameters are wrong (buffer - 0x0) and some are switched > (address and filename). I just noticed last week that ext4load has the syntax backwards from fatload for address/filename :( And in this case it's passing in too many params, so I will fix that. >>> + __func__, dfu->name, buf, size, blk_seq_num, >>> dfu->i_buf); + + if (!dfu->inited) { + ret = >>> dfu->read_medium(dfu, 0, NULL, &dfu->r_left); > ^^^^^^^^^^^^ this call causes read error. I suppose, that it is an > initial "read". Does it read the whole file at once? I'll double check what's going on over here. - -- Tom -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRPdQ+AAoJENk4IS6UOR1WYzgP/RwNBOTJ7XPM9mrFFU+3UtyA Z/tUwuyghitC/rBBPuUyoml6Oq+9oVtCd89uZyjYjFUh68HKGSZ6a+XaLl1KC9Yr n/3m91CXgnJAdh3TcrGoX6Gg9dVPb0vomLZpyZg/UK71PNnXcFI9BAvkXlfwUi1r KbPDF10WzNq4qACELdwHKP8gumhBvgB+qyCHjFRalD/jk9HS1Nv7rIpgng2eQpbv kGyRzZRJNmJ5s0/XSlg6G+XP9seE23oAw1eLZRO3bF33PcynC1L2R96iBLU4dIcQ BHQp3jC+3m7v543eXqNvBaBJDIuABkqm8Or0xdw18T8d0unci+VxRXNvVlI2VrdL fhhj47cSEKzAEJ6KTzwwvZl4CUnqfExPds/yTWl5CF+PNukw91b7kZca2uOuEtkM htbQthvt7jzaD5Ji0U8R3FF3cJ39g1XVOARpY1hzPeHMrX83b5dHFPg0LDEj31qK FWkNElx4MHgu2Cir8cBWGaFwAHxXqyG9VfPX2LwHD6zCLHmKryq+UlH+KfZgi3UE u0vDxH24jeGccDc6BIXgJdDvt8AN8SQgXtpV6xrCWEPYR90GBpM8/yOIeTbxzuky hTi2Ke8VxIGYmVWBDQ/YtKoANL1peiZuFB/DmoMA9d6WbQDkZbmgI0lQ+3x6kZxs WRBsQxgaW4Yf6nIUv5X3 =t1hs -----END PGP SIGNATURE-----
Hi, Jagan: Got it! Thank you! I have downloaded a MT29F16G08A data sheet. Best wishes, -----邮件原件----- 发件人: Jagan Teki [mailto:jagannadh.teki@gmail.com] 发送时间: 2013年3月11日 19:56 收件人: Tiger Liu 抄送: u-boot@lists.denx.de 主题: Re: [U-Boot] Nand flash (supports ONFI) Hi, On Mon, Mar 11, 2013 at 5:06 PM, <TigerLiu@viatech.com.cn> wrote: > Hi, experts: > I want to know which specific nand chip supports ONFI interface spec? > I have googled , but not find any product manual. Spansion ML series and Micron MT29F series flashes have onfi read parameter page support. Thanks, Jagan. > > Best wishes, > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
On Mon, Mar 11, 2013 at 11:03:41AM +0100, Lukasz Majewski wrote: > Hi Tom, > > > > > > > From: Pantelis Antoniou <panto@antoniou-consulting.com> > > > > > > Previously we didn't support upload/download larger than available > > > memory. This is pretty bad when you have to update your root > > > filesystem for example. > > > > > > This patch removes that limitation (and the crashes when you > > > transfered any file larger than 4MB) by making raw image writes be > > > done in chunks and making file maximum size be configurable. > > > > > > The sequence number is a 16 bit counter; make sure we handle > > > rollover correctly. This fixes the wrong transfers for large (> > > > 256MB) images. > > > > > > Also utilize a variable to handle initialization, so that we don't > > > rely on just the counter sent by the host. > > > > > > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com> > > > Signed-off-by: Tom Rini <trini@ti.com> > > > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > > > Test-hw: Exynos 4210 (Trats) > > > > Tested-by: Lukasz Majewski <l.majewski@samsung.com> > > Sorry but, I've found a regression for reading image from a file > system. It happens with EXT4 mmc read (like uImage). > > mmc_file_op: ext4load mmc 0:2 /uImage 0x0 0 0x7f95dc98 > ** File not found 0x0 ** > dfu: Read error! > > ext4load params: > ext4load - load binary file from a Ext4 filesystem > > Usage: > ext4load <interface> <dev[:part]> [addr] [filename] [bytes] This is a bug, but this is not a regression. As I sent in another email, the issue is that ext4write takes arguments backwards from fatwrite/fatload/ext4load so the line here has always been wrong.
On Mon, Mar 11, 2013 at 11:03:41AM +0100, Lukasz Majewski wrote: [snip] > > > - if (blk_seq_num == 0) { > > > - i_buf = dfu_buf; > > > - ret = dfu->read_medium(dfu, i_buf, &r_size); > > > - debug("%s: %s %ld [B]\n", __func__, dfu->name, > > > r_size); > > > - i_blk_seq_num = 0; > > > - /* Integrity check (if needed) */ > > > - crc = crc32(0, dfu_buf, r_size); > > > + __func__, dfu->name, buf, size, blk_seq_num, > > > dfu->i_buf); + > > > + if (!dfu->inited) { > > > + ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left); > ^^^^^^^^^^^^ this call causes read error. I > suppose, that it is an initial "read". Does > it read the whole file at once? > > The problem is that the command is fromatted in a > wrong way. And we're also passing NULL as the buffer to read into? That also can't be good. I'll spend a little time here and see what's going on.
On Wed, Mar 13, 2013 at 11:25:42AM -0400, Tom Rini wrote: > On Mon, Mar 11, 2013 at 11:03:41AM +0100, Lukasz Majewski wrote: > > [snip] > > > > - if (blk_seq_num == 0) { > > > > - i_buf = dfu_buf; > > > > - ret = dfu->read_medium(dfu, i_buf, &r_size); > > > > - debug("%s: %s %ld [B]\n", __func__, dfu->name, > > > > r_size); > > > > - i_blk_seq_num = 0; > > > > - /* Integrity check (if needed) */ > > > > - crc = crc32(0, dfu_buf, r_size); > > > > + __func__, dfu->name, buf, size, blk_seq_num, > > > > dfu->i_buf); + > > > > + if (!dfu->inited) { > > > > + ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left); > > ^^^^^^^^^^^^ this call causes read error. I > > suppose, that it is an initial "read". Does > > it read the whole file at once? > > > > The problem is that the command is fromatted in a > > wrong way. > > And we're also passing NULL as the buffer to read into? That also can't > be good. I'll spend a little time here and see what's going on. OK, I see and have fixed some minor issues here, but without any of these patches, I'm not seeing DFU read work at all for raw. I keep getting "Wrong sequence number!" errors on the U-Boot side. Using dfu-util 0.5 from Ubuntu 12.04 still. I'm inclined to push back and say, after fixing a few calls now that I re-read my code, DFU read is broken and set that aside as another problem to get fixed.
Hi Tom, > On Wed, Mar 13, 2013 at 11:25:42AM -0400, Tom Rini wrote: > > > On Mon, Mar 11, 2013 at 11:03:41AM +0100, Lukasz Majewski wrote: > > > > [snip] > > > > > - if (blk_seq_num == 0) { > > > > > - i_buf = dfu_buf; > > > > > - ret = dfu->read_medium(dfu, i_buf, &r_size); > > > > > - debug("%s: %s %ld [B]\n", __func__, > > > > > dfu->name, r_size); > > > > > - i_blk_seq_num = 0; > > > > > - /* Integrity check (if needed) */ > > > > > - crc = crc32(0, dfu_buf, r_size); > > > > > + __func__, dfu->name, buf, size, blk_seq_num, > > > > > dfu->i_buf); + > > > > > + if (!dfu->inited) { > > > > > + ret = dfu->read_medium(dfu, 0, NULL, > > > > > &dfu->r_left); > > > ^^^^^^^^^^^^ this call causes read > > > error. I suppose, that it is an initial "read". Does > > > it read the whole file at once? > > > > > > The problem is that the command is fromatted in a > > > wrong way. > > > > And we're also passing NULL as the buffer to read into? That also > > can't be good. I'll spend a little time here and see what's going > > on. > > OK, I see and have fixed some minor issues here, but without any of > these patches, I'm not seeing DFU read work at all for raw. I keep > getting "Wrong sequence number!" errors on the U-Boot side. Using > dfu-util 0.5 from Ubuntu 12.04 still. I'm inclined to push back and > say, after fixing a few calls now that I re-read my code, DFU read is > broken and set that aside as another problem to get fixed. > Ok, lets first fix the download (with yours patch series). The DFU read can be fixed later, for example after Marek sends pull request for u-boot-usb branch. Anyway thanks for looking into the problem.
diff --git a/README b/README index 900ae5f..154b82f 100644 --- a/README +++ b/README @@ -1338,6 +1338,13 @@ The following options need to be configured: CONFIG_DFU_MMC This enables support for exposing (e)MMC 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 + the buffer once we've been given the whole file. Define + this to the maximum filesize (in bytes) for the buffer. + Default is 4 MiB if undefined. + - Journaling Flash filesystem support: CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index e8477fb..2fecf77 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -44,90 +44,229 @@ static int dfu_find_alt_num(const char *s) static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) dfu_buf[DFU_DATA_BUF_SIZE]; +static int dfu_write_buffer_drain(struct dfu_entity *dfu) +{ + long w_size; + int ret; + + /* flush size? */ + w_size = dfu->i_buf - dfu->i_buf_start; + if (w_size == 0) + return 0; + + /* update CRC32 */ + dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size); + + ret = dfu->write_medium(dfu, dfu->offset, dfu->i_buf_start, &w_size); + if (ret) + debug("%s: Write error!\n", __func__); + + /* point back */ + dfu->i_buf = dfu->i_buf_start; + + /* update offset */ + dfu->offset += w_size; + + puts("#"); + + return ret; +} + int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) { - static unsigned char *i_buf; - static int i_blk_seq_num; - long w_size = 0; int ret = 0; + int tret; + + debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x " + "offset: 0x%llx bufoffset: 0x%x\n", + __func__, dfu->name, buf, size, blk_seq_num, dfu->offset, + dfu->i_buf - dfu->i_buf_start); + + if (!dfu->inited) { + /* initial state */ + dfu->crc = 0; + dfu->offset = 0; + dfu->i_blk_seq_num = 0; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); + dfu->i_buf = dfu->i_buf_start; + + dfu->inited = 1; + } - debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n", - __func__, dfu->name, buf, size, blk_seq_num, i_buf); + if (dfu->i_blk_seq_num != blk_seq_num) { + printf("%s: Wrong sequence number! [%d] [%d]\n", + __func__, dfu->i_blk_seq_num, blk_seq_num); + return -1; + } - if (blk_seq_num == 0) { - i_buf = dfu_buf; - i_blk_seq_num = 0; + /* DFU 1.1 standard says: + * The wBlockNum field is a block sequence number. It increments each + * time a block is transferred, wrapping to zero from 65,535. It is used + * to provide useful context to the DFU loader in the device." + * + * This means that it's a 16 bit counter that roll-overs at + * 0xffff -> 0x0000. By having a typical 4K transfer block + * we roll-over at exactly 256MB. Not very fun to debug. + * + * Handling rollover, and having an inited variable, + * makes things work. + */ + + /* handle rollover */ + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff; + + /* flush buffer if overflow */ + if ((dfu->i_buf + size) > dfu->i_buf_end) { + tret = dfu_write_buffer_drain(dfu); + if (ret == 0) + ret = tret; } - if (i_blk_seq_num++ != blk_seq_num) { - printf("%s: Wrong sequence number! [%d] [%d]\n", - __func__, i_blk_seq_num, blk_seq_num); + /* we should be in buffer now (if not then size too large) */ + if ((dfu->i_buf + size) > dfu->i_buf_end) { + printf("%s: Wrong size! [%d] [%d] - %d\n", + __func__, dfu->i_blk_seq_num, blk_seq_num, size); return -1; } - memcpy(i_buf, buf, size); - i_buf += size; + memcpy(dfu->i_buf, buf, size); + dfu->i_buf += size; + /* if end or if buffer full flush */ + if (size == 0 || (dfu->i_buf + size) > dfu->i_buf_end) { + tret = dfu_write_buffer_drain(dfu); + if (ret == 0) + ret = tret; + } + + /* end? */ if (size == 0) { - /* Integrity check (if needed) */ - debug("%s: %s %d [B] CRC32: 0x%x\n", __func__, dfu->name, - i_buf - dfu_buf, crc32(0, dfu_buf, i_buf - dfu_buf)); + /* Now try and flush to the medium if needed. */ + if (dfu->flush_medium) + ret = dfu->flush_medium(dfu); + printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); - w_size = i_buf - dfu_buf; - ret = dfu->write_medium(dfu, dfu_buf, &w_size); - if (ret) - debug("%s: Write error!\n", __func__); + /* clear everything */ + dfu->crc = 0; + dfu->offset = 0; + dfu->i_blk_seq_num = 0; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); + dfu->i_buf = dfu->i_buf_start; + + dfu->inited = 0; - i_blk_seq_num = 0; - i_buf = NULL; - return ret; } - return ret; + return ret = 0 ? size : ret; +} + +static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size) +{ + long chunk; + int ret, readn; + + readn = 0; + while (size > 0) { + + /* get chunk that can be read */ + chunk = min(size, dfu->b_left); + /* consume */ + if (chunk > 0) { + memcpy(buf, dfu->i_buf, chunk); + dfu->crc = crc32(dfu->crc, buf, chunk); + dfu->i_buf += chunk; + dfu->b_left -= chunk; + size -= chunk; + buf += chunk; + readn += chunk; + } + + /* all done */ + if (size > 0) { + /* no more to read */ + if (dfu->r_left == 0) + break; + + dfu->i_buf = dfu->i_buf_start; + dfu->b_left = dfu->i_buf_end - dfu->i_buf_start; + + /* got to read, but buffer is empty */ + if (dfu->b_left > dfu->r_left) + dfu->b_left = dfu->r_left; + ret = dfu->read_medium(dfu, dfu->offset, dfu->i_buf, + &dfu->b_left); + if (ret != 0) { + debug("%s: Read error!\n", __func__); + return ret; + } + dfu->offset += dfu->b_left; + dfu->r_left -= dfu->b_left; + + puts("#"); + } + } + + return readn; } int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) { - static unsigned char *i_buf; - static int i_blk_seq_num; - static long r_size; - static u32 crc; int ret = 0; debug("%s: name: %s buf: 0x%p size: 0x%x p_num: 0x%x i_buf: 0x%p\n", - __func__, dfu->name, buf, size, blk_seq_num, i_buf); - - if (blk_seq_num == 0) { - i_buf = dfu_buf; - ret = dfu->read_medium(dfu, i_buf, &r_size); - debug("%s: %s %ld [B]\n", __func__, dfu->name, r_size); - i_blk_seq_num = 0; - /* Integrity check (if needed) */ - crc = crc32(0, dfu_buf, r_size); + __func__, dfu->name, buf, size, blk_seq_num, dfu->i_buf); + + if (!dfu->inited) { + ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left); + if (ret != 0) { + debug("%s: failed to get r_left\n", __func__); + return ret; + } + + debug("%s: %s %ld [B]\n", __func__, dfu->name, dfu->r_left); + + dfu->i_blk_seq_num = 0; + dfu->crc = 0; + dfu->offset = 0; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); + dfu->i_buf = dfu->i_buf_start; + dfu->b_left = 0; + + dfu->inited = 1; } - if (i_blk_seq_num++ != blk_seq_num) { + if (dfu->i_blk_seq_num != blk_seq_num) { printf("%s: Wrong sequence number! [%d] [%d]\n", - __func__, i_blk_seq_num, blk_seq_num); + __func__, dfu->i_blk_seq_num, blk_seq_num); return -1; } + /* handle rollover */ + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff; - if (r_size >= size) { - memcpy(buf, i_buf, size); - i_buf += size; - r_size -= size; - return size; - } else { - memcpy(buf, i_buf, r_size); - i_buf += r_size; - debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, crc); - puts("UPLOAD ... done\nCtrl+C to exit ...\n"); + ret = dfu_read_buffer_fill(dfu, buf, size); + if (ret < 0) { + printf("%s: Failed to fill buffer\n", __func__); + return -1; + } + + if (ret < size) { + debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, dfu->crc); + puts("\nUPLOAD ... done\nCtrl+C to exit ...\n"); - i_buf = NULL; - i_blk_seq_num = 0; - crc = 0; - return r_size; + dfu->i_blk_seq_num = 0; + dfu->crc = 0; + dfu->offset = 0; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf + sizeof(dfu_buf); + dfu->i_buf = dfu->i_buf_start; + dfu->b_left = 0; + + dfu->inited = 0; } + return ret; } diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 083d745..2d5ffd8 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -22,6 +22,7 @@ #include <common.h> #include <malloc.h> #include <errno.h> +#include <div64.h> #include <dfu.h> enum dfu_mmc_op { @@ -29,36 +30,67 @@ enum dfu_mmc_op { DFU_OP_WRITE, }; +static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) + dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE]; +static long dfu_file_buf_len; + static int mmc_block_op(enum dfu_mmc_op op, struct dfu_entity *dfu, - void *buf, long *len) + u64 offset, void *buf, long *len) { char cmd_buf[DFU_CMD_BUF_SIZE]; + u32 blk_start, blk_count; + + /* + * We must ensure that we work in lba_blk_size chunks, so ALIGN + * this value. + */ + *len = ALIGN(*len, dfu->data.mmc.lba_blk_size); + + blk_start = dfu->data.mmc.lba_start + + (u32)lldiv(offset, dfu->data.mmc.lba_blk_size); + blk_count = *len / dfu->data.mmc.lba_blk_size; + if (*len + blk_start > + dfu->data.mmc.lba_size * dfu->data.mmc.lba_size) { + puts("Request would exceed designated area!\n"); + return -EINVAL; + } - sprintf(cmd_buf, "mmc %s 0x%x %x %x", + sprintf(cmd_buf, "mmc %s %p %x %x", op == DFU_OP_READ ? "read" : "write", - (unsigned int) buf, - dfu->data.mmc.lba_start, - dfu->data.mmc.lba_size); - - if (op == DFU_OP_READ) - *len = dfu->data.mmc.lba_blk_size * dfu->data.mmc.lba_size; + buf, blk_start, blk_count); debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); return run_command(cmd_buf, 0); } -static inline int mmc_block_write(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_block_write(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) +{ + return mmc_block_op(DFU_OP_WRITE, dfu, offset, buf, len); +} + +static inline int mmc_block_read(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_block_op(DFU_OP_WRITE, dfu, buf, len); + return mmc_block_op(DFU_OP_READ, dfu, offset, buf, len); } -static inline int mmc_block_read(struct dfu_entity *dfu, void *buf, long *len) +static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len) { - return mmc_block_op(DFU_OP_READ, dfu, buf, len); + if (dfu_file_buf_len + *len > CONFIG_SYS_DFU_MAX_FILE_SIZE) { + dfu_file_buf_len = 0; + return -EINVAL; + } + + /* Add to the current buffer. */ + memcpy(dfu_file_buf + dfu_file_buf_len, buf, *len); + dfu_file_buf_len += *len; + + return 0; } static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, - void *buf, long *len) + u64 offset, void *buf, long *len) { char cmd_buf[DFU_CMD_BUF_SIZE]; char *str_env; @@ -70,8 +102,15 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, op == DFU_OP_READ ? "load" : "write", dfu->data.mmc.dev, dfu->data.mmc.part, (unsigned int) buf, dfu->name, *len); + if (op == DFU_OP_READ && offset != 0) + sprintf(cmd_buf + strlen(cmd_buf), " %llx", offset); break; case DFU_FS_EXT4: + if (offset != 0) { + debug("%s: Offset value %llx != 0 not supported!\n", + __func__, offset); + return -1; + } sprintf(cmd_buf, "ext4%s mmc %d:%d /%s 0x%x %ld", op == DFU_OP_READ ? "load" : "write", dfu->data.mmc.dev, dfu->data.mmc.part, @@ -80,6 +119,7 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, dfu_get_layout(dfu->layout)); + return -1; } debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); @@ -102,27 +142,24 @@ static int mmc_file_op(enum dfu_mmc_op op, struct dfu_entity *dfu, return ret; } -static inline int mmc_file_write(struct dfu_entity *dfu, void *buf, long *len) +static inline int mmc_file_read(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { - return mmc_file_op(DFU_OP_WRITE, dfu, buf, len); + return mmc_file_op(DFU_OP_READ, dfu, offset, buf, len); } -static inline int mmc_file_read(struct dfu_entity *dfu, void *buf, long *len) -{ - return mmc_file_op(DFU_OP_READ, dfu, buf, len); -} - -int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) +int dfu_write_medium_mmc(struct dfu_entity *dfu, + u64 offset, void *buf, long *len) { int ret = -1; switch (dfu->layout) { case DFU_RAW_ADDR: - ret = mmc_block_write(dfu, buf, len); + ret = mmc_block_write(dfu, offset, buf, len); break; case DFU_FS_FAT: case DFU_FS_EXT4: - ret = mmc_file_write(dfu, buf, len); + ret = mmc_file_buffer(dfu, buf, len); break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -132,17 +169,34 @@ int dfu_write_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) return ret; } -int dfu_read_medium_mmc(struct dfu_entity *dfu, void *buf, long *len) +int dfu_flush_medium_mmc(struct dfu_entity *dfu) +{ + int ret = 0; + + if (dfu->layout != DFU_RAW_ADDR) { + /* Do stuff here. */ + ret = mmc_file_op(DFU_OP_WRITE, dfu, 0, &dfu_file_buf, + &dfu_file_buf_len); + + /* Now that we're done */ + dfu_file_buf_len = 0; + } + + return ret; +} + +int dfu_read_medium_mmc(struct dfu_entity *dfu, u64 offset, void *buf, + long *len) { int ret = -1; switch (dfu->layout) { case DFU_RAW_ADDR: - ret = mmc_block_read(dfu, buf, len); + ret = mmc_block_read(dfu, offset, buf, len); break; case DFU_FS_FAT: case DFU_FS_EXT4: - ret = mmc_file_read(dfu, buf, len); + ret = mmc_file_read(dfu, offset, buf, len); break; default: printf("%s: Layout (%s) not (yet) supported!\n", __func__, @@ -181,13 +235,15 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s) mmc = find_mmc_device(dev); if (mmc == NULL || mmc_init(mmc)) { - printf("%s: could not find mmc device #%d!\n", __func__, dev); + printf("%s: could not find mmc device #%d!\n", + __func__, dev); return -ENODEV; } blk_dev = &mmc->block_dev; if (get_partition_info(blk_dev, part, &partinfo) != 0) { - printf("%s: could not find partition #%d on mmc device #%d!\n", + printf("%s: could not find partition #%d " + "on mmc device #%d!\n", __func__, part, dev); return -ENODEV; } @@ -208,6 +264,10 @@ int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s) dfu->read_medium = dfu_read_medium_mmc; dfu->write_medium = dfu_write_medium_mmc; + dfu->flush_medium = dfu_flush_medium_mmc; + + /* initial state */ + dfu->inited = 0; return 0; } diff --git a/include/dfu.h b/include/dfu.h index 5350d79..5182c6c 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -59,7 +59,10 @@ static inline unsigned int get_mmc_blk_size(int dev) #define DFU_NAME_SIZE 32 #define DFU_CMD_BUF_SIZE 128 -#define DFU_DATA_BUF_SIZE (1024*1024*4) /* 4 MiB */ +#define DFU_DATA_BUF_SIZE (64 << 10) /* 64 KiB */ +#ifndef CONFIG_SYS_DFU_MAX_FILE_SIZE +#define CONFIG_SYS_DFU_MAX_FILE_SIZE (4 << 20) /* 4 MiB */ +#endif struct dfu_entity { char name[DFU_NAME_SIZE]; @@ -73,10 +76,27 @@ struct dfu_entity { struct mmc_internal_data mmc; } data; - int (*read_medium)(struct dfu_entity *dfu, void *buf, long *len); - int (*write_medium)(struct dfu_entity *dfu, void *buf, long *len); + int (*read_medium)(struct dfu_entity *dfu, + u64 offset, void *buf, long *len); + + int (*write_medium)(struct dfu_entity *dfu, + u64 offset, void *buf, long *len); + + int (*flush_medium)(struct dfu_entity *dfu); struct list_head list; + + /* on the fly state */ + u32 crc; + u64 offset; + int i_blk_seq_num; + u8 *i_buf; + u8 *i_buf_start; + u8 *i_buf_end; + long r_left; + long b_left; + + unsigned int inited:1; }; int dfu_config_entities(char *s, char *interface, int num);