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

Pantelis Antoniou
2014-03-31T11:19:13Z

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