Hi Lukasz, On Mar 31, 2014, at 3:04 PM, Lukasz Majewski wrote:
> 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. > Thanks for the report. Would it be too much to ask for the time it takes to do a crc32 of the same image on user-space after boot? I'm interested in the effect the disabling of L2 has. > >> >> 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. > > You could do a basic tradeoff of speed vs memory by creating larger tables but it might not work if we're cache trashing. Or even try using CRC with smaller tables (i.e. 4 bits) which would not affect the cache much. Regards -- Pantelis >> >> Regards >> >> -- Pantelis >> >> >>> Signed-off-by: Lukasz Majewski <l.majew...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot