Skip to contentSkip to conversation

dfu: Handle large transfers correctly

PATCH New Pantelis Antoniou 3 participants 5 messages Nov 28, 2012 – Nov 29, 2012

lore
Share & tools
lore

Pantelis Antoniou

author
#0 lore raw

PATCH 1/1 dfu: Handle large transfers correctly

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 <[email protected]>
---
 drivers/dfu/dfu.c     | 28 +++++++++++++++++++++++-----
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  2 ++
 3 files changed, 28 insertions(+), 5 deletions(-)
drivers/dfu/dfu.c +23 -5
index 1260c55..2483018 100644
@@ -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);
+		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;
drivers/dfu/dfu_mmc.c +3 -0
index 4c8994b..906df55 100644
@@ -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;
 }
include/dfu.h +2 -0
index b662488..0ce91b7 100644
@@ -90,6 +90,8 @@ struct dfu_entity {
 	u8 *i_buf_end;
 	long r_left;
 	long b_left;
+
+	unsigned int do_init : 1
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
Signature · 1 line
1.7.12
#1 lore raw

PATCH 1/1 dfu: Handle large transfers correctly

Dear Pantelis Antoniou,

Quoted 16 lines

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 <[email protected]>
---
 drivers/dfu/dfu.c     | 28 +++++++++++++++++++++++-----
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  2 ++
 3 files changed, 28 insertions(+), 5 deletions(-)
drivers/dfu/dfu.c +0 -0
index 1260c55..2483018 100644

drivers/dfu/dfu.c int dfu_write(struct dfu_entity *dfu, void *buf, int · line 85

Quoted 5 lines
drivers/dfu/dfu.c +1 -1
@@ -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) {

If you were to change this to dfu->inited, I think it'd be a bit more intuitive ;)

Quoted 23 lines
drivers/dfu/dfu.c +4 -1
 		/* 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) {

drivers/dfu/dfu.c int dfu_write(struct dfu_entity *dfu, void *buf, int · line 112

Quoted 4 lines
drivers/dfu/dfu.c +1 -0
@@ -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) */

I can't make much sense from this comment :(

drivers/dfu/dfu.c int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) · line 114

Quoted 2 lines
drivers/dfu/dfu.c +2 -0
+	if ((dfu->i_buf + size) > dfu->i_buf_end) {
+		printf("%s: Wrong size! [%d] [%d] - \n",

And you should really make this output a bit more human readable please.

Quoted 4 lines
drivers/dfu/dfu.c +4 -0
+		       __func__, dfu->i_blk_seq_num, blk_seq_num, size);
+		return -1;
+	}
+

[...]

#2 lore raw

PATCH 1/1 dfu: Handle large transfers correctly

In reply to Pantelis Antoniou

Hi Pantelis,

Quoted 3 lines

The sequence number is a 16 bit counter; make sure we handle rollover correctly. This fixes the wrong transfers for large (> 256MB) images.

This is a bit misleading comment.
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."

Maybe above statement shall be put to the comment.

One more thing:

wTransferSize = 4096 and the DFU 1.1 standard says:

"The host sends between bMaxPacketSize0 and wTransferSize" and increments wBlockNum (which is 16 bits).

Maybe we shall increase the wTransferSize (defined at static const struct dfu_function_descriptor dfu_func) to e.g. 32 KiB and leave the code unchanged?

Can you test this option on your setup?

Quoted 137 lines
Signed-off-by Pantelis Antoniou <[email protected]>
---
 drivers/dfu/dfu.c     | 28 +++++++++++++++++++++++-----
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  2 ++
 3 files changed, 28 insertions(+), 5 deletions(-)
drivers/dfu/dfu.c +16 -3
index 1260c55..2483018 100644
@@ -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);
+		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);

drivers/dfu/dfu.c +1 -1
-	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;

drivers/dfu/dfu.c +6 -1
 		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;
drivers/dfu/dfu_mmc.c +3 -0
index 4c8994b..906df55 100644
@@ -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;
 }
include/dfu.h +2 -0
index b662488..0ce91b7 100644
@@ -90,6 +90,8 @@ struct dfu_entity {
 	u8 *i_buf_end;
 	long r_left;
 	long b_left;
+
+	unsigned int do_init : 1
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group
author
#3 lore raw

PATCH 1/1 dfu: Handle large transfers correctly

Hi Lukasz,

Quoted 6 lines · Lukasz Majewski

Hi Pantelis,

Quoted 3 lines

The sequence number is a 16 bit counter; make sure we handle rollover correctly. This fixes the wrong transfers for large (> 256MB) images.

Quoted 8 lines

This is a bit misleading comment.
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."

Maybe above statement shall be put to the comment.

No problem there.

Quoted 4 lines

One more thing:

wTransferSize = 4096 and the DFU 1.1 standard says:

Quoted 9 lines

"The host sends between bMaxPacketSize0 and wTransferSize" and increments wBlockNum (which is 16 bits).

Maybe we shall increase the wTransferSize (defined at static const struct dfu_function_descriptor dfu_func) to e.g. 32 KiB and leave the code unchanged?

Can you test this option on your setup?

No. The code as it stands, it's just broken. You will only delay the inevitable, i.e. the rollover that is not handled correctly.

This is just bandaid, and is not fixing the problem.

Regards

-- Pantelis

Quoted 146 lines
Quoted 137 lines
Signed-off-by Pantelis Antoniou <[email protected]>

---

drivers/dfu/dfu.c     | 28 +++++++++++++++++++++++-----
drivers/dfu/dfu_mmc.c |  3 +++
include/dfu.h         |  2 ++
3 files changed, 28 insertions(+), 5 deletions(-)
drivers/dfu/dfu.c +16 -3
index 1260c55..2483018 100644
@@ -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);
+		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);

drivers/dfu/dfu.c +1 -1
-	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;
drivers/dfu/dfu.c +2 -0
+
+		dfu->do_init = 0;

	}

	if (dfu->i_blk_seq_num != blk_seq_num) {
drivers/dfu/dfu.c +4 -1
@@ -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;
drivers/dfu/dfu_mmc.c +3 -0
index 4c8994b..906df55 100644
@@ -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;
 }
include/dfu.h +2 -0
index b662488..0ce91b7 100644
@@ -90,6 +90,8 @@ struct dfu_entity {
 	u8 *i_buf_end;
 	long r_left;
 	long b_left;
+
+	unsigned int do_init : 1
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group
#4 lore raw

PATCH 1/1 dfu: Handle large transfers correctly

In reply to Pantelis Antoniou

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.

Quoted 44 lines

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 <[email protected]>
---
 drivers/dfu/dfu.c     | 28 +++++++++++++++++++++++-----
 drivers/dfu/dfu_mmc.c |  3 +++
 include/dfu.h         |  2 ++
 3 files changed, 28 insertions(+), 5 deletions(-)
drivers/dfu/dfu.c +5 -2
index 1260c55..2483018 100644
@@ -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) {

drivers/dfu/dfu.c int dfu_write(struct dfu_entity *dfu, void *buf, · line 116

Quoted 8 lines
drivers/dfu/dfu.c +4 -0
@@ -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.

Quoted 80 lines
drivers/dfu/dfu.c +7 -1
+		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);

drivers/dfu/dfu.c +1 -1
-	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;

drivers/dfu/dfu.c +6 -1
 		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;
drivers/dfu/dfu_mmc.c +3 -0
index 4c8994b..906df55 100644
@@ -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;
 }
include/dfu.h +0 -0
index b662488..0ce91b7 100644

include/dfu.h struct dfu_entity { · line 94

Quoted 5 lines
include/dfu.h +2 -0
@@ -90,6 +90,8 @@ struct dfu_entity {
 	u8 *i_buf_end;
 	long r_left;
 	long b_left;
+
+	unsigned int do_init : 1

                                 ^^^Missing semicolon ....
Quoted 3 lines
include/dfu.h +0 -0
 };
 
 int dfu_config_entities(char *s, char *interface, int num);
Signature · 5 lines
Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group