[U-Boot] dfu: Handle large transfers correctly

Submitted by Pantelis Antoniou on Nov. 28, 2012, 10:38 p.m.

Details

Message ID 1354142330-31130-1-git-send-email-panto@antoniou-consulting.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Pantelis Antoniou Nov. 28, 2012, 10:38 p.m.
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 <panto@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(-)

Comments

Marek Vasut Nov. 29, 2012, 6:26 a.m.
Dear Pantelis Antoniou,

> 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 <panto@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) {

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

>  		/* 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) */

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

> +	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.

> +		       __func__, dfu->i_blk_seq_num, blk_seq_num, size);
> +		return -1;
> +	}
> +
[...]
Łukasz Majewski Nov. 29, 2012, 9:06 a.m.
Hi Pantelis,

> 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?

> 
> Signed-off-by: Pantelis Antoniou <panto@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);
> +		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
>  };
>  
>  int dfu_config_entities(char *s, char *interface, int num);
Pantelis Antoniou Nov. 29, 2012, 9:35 a.m.
Hi Lukasz,

On Nov 29, 2012, at 11:06 AM, Lukasz Majewski wrote:

> Hi Pantelis,
> 
>> 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.
> 

No problem there.

> 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?
> 

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

>> 
>> Signed-off-by: Pantelis Antoniou <panto@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);
>> +		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
>> };
>> 
>> int dfu_config_entities(char *s, char *interface, int num);
> 
> 
> 
> -- 
> Best regards,
> 
> Lukasz Majewski
> 
> Samsung Poland R&D Center | Linux Platform Group
Łukasz Majewski Nov. 29, 2012, 10:25 a.m.
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 <panto@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);

Patch hide | download patch | download mbox

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);
+		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
 };
 
 int dfu_config_entities(char *s, char *interface, int num);