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

[Lukasz Majewski <l.majewski@samsung.com>]

Hi Pantelis,

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


Board Exynos4412 - Trats2. Cache L1 enabled, cache L2 disabled.

Crc32 is calculated for 32 MiB chunks of data in the received buffer.
I'm using the standard software crc32 u-boot's implementation. It is
the same as the one for perl-crc32 debian package.


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

It seems so. Moreover the Exynos4412 has HW crypto engine which
supports SHA1, MD5 and other algorithms. Unfortunately it doesn't
provide speedup for crc32.


> 
> Regards
> 
> -- Pantelis
> 
> 
> > Signed-off-by: Lukasz Majewski <l.majewski@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
> > 



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group