Hi Tom,
>
> > From: Pantelis Antoniou <pa...@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 <pa...@antoniou-consulting.com>
> > Signed-off-by: Tom Rini <tr...@ti.com>
>
> Acked-by: Lukasz Majewski <l.majew...@samsung.com>
>
> Test-hw: Exynos 4210 (Trats)
>
> Tested-by: Lukasz Majewski <l.majew...@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.
--
Best regards,
Lukasz Majewski
Samsung R&D Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
