Hi Pantelis,
Please see below comments.
Moreover this patch doesn't solve the problem of bricking trats board
after flashing it with dfu after applying:
[U-Boot] [PATCH] dfu: Handle large transfers correctly
on top of:
[U-Boot] [PATCH 00/10] USB: Gadget & DFU related fixes
Without your patches it works, so thorough examination is needed.
I will have time to do that after weekend.
> The sequence number is a 16 bit counter; make sure we
> handle rollover correctly. This fixes the wrong transfers for
> large (> 256MB) images.
>
> Signed-off-by: Pantelis Antoniou <pa...@antoniou-consulting.com>
> ---
> drivers/dfu/dfu.c | 28 +++++++++++++++++++++++-----
> drivers/dfu/dfu_mmc.c | 3 +++
> include/dfu.h | 2 ++
> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index 1260c55..2483018 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -82,7 +82,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) __func__, dfu->name, buf, size,
> blk_seq_num, dfu->offset, dfu->i_buf - dfu->i_buf_start);
>
> - if (blk_seq_num == 0) {
> + if (dfu->do_init) {
> /* initial state */
> dfu->crc = 0;
> dfu->offset = 0;
> @@ -90,6 +90,8 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->i_buf_start = dfu_buf;
> dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
> dfu->i_buf = dfu->i_buf_start;
> +
> + dfu->do_init = 0;
> }
>
> if (dfu->i_blk_seq_num != blk_seq_num) {
> @@ -97,7 +99,8 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) __func__, dfu->i_blk_seq_num, blk_seq_num);
> return -1;
> }
> - dfu->i_blk_seq_num++;
> + /* 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) {
> @@ -106,6 +109,13 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) ret = tret;
> }
>
> + /* 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] - \n",
> + __func__, dfu->i_blk_seq_num, blk_seq_num,
> size);
Missing %d for size parameter.
> + return -1;
> + }
> +
> memcpy(dfu->i_buf, buf, size);
> dfu->i_buf += size;
>
> @@ -120,7 +130,7 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) if (size == 0) {
> debug("%s: DFU complete CRC32: 0x%x\n", __func__,
> dfu->crc);
> - puts("\nDownload complete\n");
> + printf("\nDownload complete (CRC32 0x%04x)\n",
> dfu->crc);
> /* clear everything */
> dfu->crc = 0;
> @@ -129,6 +139,9 @@ int dfu_write(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->i_buf_start = dfu_buf;
> dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
> dfu->i_buf = dfu->i_buf_start;
> +
> + dfu->do_init = 1;
> +
> }
>
> return ret = 0 ? size : ret;
> @@ -190,7 +203,7 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) 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, dfu->i_buf);
> - if (blk_seq_num == 0) {
> + if (dfu->do_init) {
> ret = dfu->read_medium(dfu, 0, NULL, &dfu->r_left);
> if (ret != 0) {
> debug("%s: failed to get r_left\n",
> __func__); @@ -206,6 +219,8 @@ int dfu_read(struct dfu_entity *dfu,
> void *buf, int size, int blk_seq_num) dfu->i_buf_end = dfu_buf +
> sizeof(dfu_buf); dfu->i_buf = dfu->i_buf_start;
> dfu->b_left = 0;
> +
> + dfu->do_init = 0;
> }
>
> if (dfu->i_blk_seq_num != blk_seq_num) {
> @@ -213,7 +228,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) __func__, dfu->i_blk_seq_num, blk_seq_num);
> return -1;
> }
> - dfu->i_blk_seq_num++;
> + /* handle rollover */
> + dfu->i_blk_seq_num = (dfu->i_blk_seq_num + 1) & 0xffff;
>
> ret = dfu_read_buffer_fill(dfu, buf, size);
> if (ret < 0) {
> @@ -232,6 +248,8 @@ int dfu_read(struct dfu_entity *dfu, void *buf,
> int size, int blk_seq_num) dfu->i_buf_end = dfu_buf + sizeof(dfu_buf);
> dfu->i_buf = dfu->i_buf_start;
> dfu->b_left = 0;
> +
> + dfu->do_init = 1;
> }
>
> return ret;
> diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c
> index 4c8994b..906df55 100644
> --- a/drivers/dfu/dfu_mmc.c
> +++ b/drivers/dfu/dfu_mmc.c
> @@ -234,5 +234,8 @@ 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;
>
> + /* initial state */
> + dfu->do_init = 1;
> +
> return 0;
> }
> diff --git a/include/dfu.h b/include/dfu.h
> index b662488..0ce91b7 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -90,6 +90,8 @@ struct dfu_entity {
> u8 *i_buf_end;
> long r_left;
> long b_left;
> +
> + unsigned int do_init : 1
^^^Missing semicolon ....
> };
>
> int dfu_config_entities(char *s, char *interface, int num);
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
