[U-Boot] [PATCH 3/3] dfu: Introduction of the "dfu_checksum_method" env variable for checksum method setting

[Pantelis Antoniou]

Hi Lukasz,

On Mar 31, 2014, at 11:48 AM, Lukasz Majewski wrote:

> Up till now the CRC32 of received data was calculated unconditionally.
> The standard crc32 implementation causes long delays when large images
> were uploaded.
> 
> The "dfu_checksum_method" environment variable gives the opportunity to
> enable on demand (when e.g. debugging) the crc32 calculation.
> It can be done without need to recompile the u-boot binary.
> 
> By default the crc32 is not calculated.
> 
> Tests results:
> 400 MiB ums.img file
> With 		crc32 calculation: 65 sec [avg 6.29 MB/s]
> Without 		crc32 calculation: 25 sec [avg 16.17 MB/s]
> 

That's interesting; I'm surprised that there's so much difference.
Can we get some info about the environment? I.e. board, whether cache
is enabled etc.

The crc table is per byte and I guess lookups maybe expensive.

Regards

-- Pantelis


> Signed-off-by: Lukasz Majewski <l.majewski at samsung.com>
> ---
> drivers/dfu/dfu.c |   34 ++++++++++++++++++++++++++++++----
> include/dfu.h     |    5 +++++
> 2 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c
> index e15f959..5d50b47 100644
> --- a/drivers/dfu/dfu.c
> +++ b/drivers/dfu/dfu.c
> @@ -20,6 +20,7 @@ static bool dfu_reset_request;
> static LIST_HEAD(dfu_list);
> static int dfu_alt_num;
> static int alt_num_count;
> +static int dfu_checksum_method;
> 
> bool dfu_reset(void)
> {
> @@ -99,6 +100,23 @@ unsigned char *dfu_get_buf(void)
> 	return dfu_buf;
> }
> 
> +static int dfu_get_checksum_method(void)
> +{
> +	char *s;
> +
> +	s = getenv("dfu_checksum_method");
> +	if (!s)
> +		return DFU_NO_CHECKSUM;
> +
> +	if (!strcmp(s, "crc32")) {
> +		debug("%s: DFU checksum method: %s\n", __func__, s);
> +		return DFU_CRC32;
> +	} else {
> +		error("DFU checksum method: %s not supported!\n", s);
> +		return -EINVAL;
> +	}
> +}
> +
> static int dfu_write_buffer_drain(struct dfu_entity *dfu)
> {
> 	long w_size;
> @@ -109,8 +127,8 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu)
> 	if (w_size == 0)
> 		return 0;
> 
> -	/* update CRC32 */
> -	dfu->crc = crc32(dfu->crc, dfu->i_buf_start, w_size);
> +	if (dfu_checksum_method == DFU_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)
> @@ -234,7 +252,8 @@ static int dfu_read_buffer_fill(struct dfu_entity *dfu, void *buf, int size)
> 		/* consume */
> 		if (chunk > 0) {
> 			memcpy(buf, dfu->i_buf, chunk);
> -			dfu->crc = crc32(dfu->crc, buf, chunk);
> +			if (dfu_checksum_method == DFU_CRC32)
> +				dfu->crc = crc32(dfu->crc, buf, chunk);
> 			dfu->i_buf += chunk;
> 			dfu->b_left -= chunk;
> 			dfu->r_left -= chunk;
> @@ -318,7 +337,9 @@ int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
> 	}
> 
> 	if (ret < size) {
> -		debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name, dfu->crc);
> +		if (dfu_checksum_method == DFU_CRC32)
> +			debug("%s: %s CRC32: 0x%x\n", __func__, dfu->name,
> +			      dfu->crc);
> 		puts("\nUPLOAD ... done\nCtrl+C to exit ...\n");
> 
> 		dfu_free_buf();
> @@ -393,6 +414,11 @@ int dfu_config_entities(char *env, char *interface, int num)
> 	dfu_alt_num = dfu_find_alt_num(env);
> 	debug("%s: dfu_alt_num=%d\n", __func__, dfu_alt_num);
> 
> +	ret = dfu_get_checksum_method();
> +	if (ret < 0)
> +		return ret;
> +	dfu_checksum_method = ret;
> +
> 	dfu = calloc(sizeof(*dfu), dfu_alt_num);
> 	if (!dfu)
> 		return -1;
> diff --git a/include/dfu.h b/include/dfu.h
> index 751f0fd..855d6dc 100644
> --- a/include/dfu.h
> +++ b/include/dfu.h
> @@ -37,6 +37,11 @@ enum dfu_op {
> 	DFU_OP_WRITE,
> };
> 
> +enum dfu_checksum {
> +	DFU_NO_CHECKSUM = 0,
> +	DFU_CRC32,
> +};
> +
> #define DFU_NOT_SUPPORTED -1
> 
> struct mmc_internal_data {
> -- 
> 1.7.10.4
>