[U-Boot] [PATCH 0/5] Update fastboot sparse image handling

While retaining the storage abstraction feature implemented in U-Boot, this series updates the fastboot sparse image handling by (1) fixing broken code, (2) resync'ing with the upstream code, and (3) improving performance when writing CHUNK_TYPE_FILL
Steve Rae (5): fastboot: sparse: remove session-id logic fastboot: sparse: resync common/image-sparse.c (part 1) fastboot: sparse: resync common/image-sparse.c (part 2) fastboot: sparse: implement reserve() fastboot: sparse: improve CHUNK_TYPE_FILL write performance
common/fb_mmc.c | 79 +++---- common/fb_nand.c | 100 +++++---- common/image-sparse.c | 478 +++++++++++++++------------------------- drivers/usb/gadget/f_fastboot.c | 47 ++-- include/fastboot.h | 4 +- include/fb_mmc.h | 7 +- include/fb_nand.h | 7 +- include/image-sparse.h | 29 ++- 8 files changed, 309 insertions(+), 442 deletions(-)

This "session-id" alogrithm is not required, and currently corrupts the stored image whenever more the one "session" is required.
Signed-off-by: Steve Rae srae@broadcom.com --- for more information, see the thread starting at [1] [1] http://lists.denx.de/pipermail/u-boot/2016-April/251889.html
common/fb_mmc.c | 8 +++----- common/fb_nand.c | 4 ++-- common/image-sparse.c | 21 ++++----------------- drivers/usb/gadget/f_fastboot.c | 16 ++-------------- include/fb_mmc.h | 5 ++--- include/fb_nand.h | 5 ++--- include/image-sparse.h | 2 +- 7 files changed, 16 insertions(+), 45 deletions(-)
diff --git a/common/fb_mmc.c b/common/fb_mmc.c index e3abcc8..9e53adb 100644 --- a/common/fb_mmc.c +++ b/common/fb_mmc.c @@ -97,9 +97,8 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info, fastboot_okay(response_str, ""); }
-void fb_mmc_flash_write(const char *cmd, unsigned int session_id, - void *download_buffer, unsigned int download_bytes, - char *response) +void fb_mmc_flash_write(const char *cmd, void *download_buffer, + unsigned int download_bytes, char *response) { struct blk_desc *dev_desc; disk_partition_t info; @@ -153,8 +152,7 @@ void fb_mmc_flash_write(const char *cmd, unsigned int session_id, printf("Flashing sparse image at offset " LBAFU "\n", info.start);
- store_sparse_image(&sparse, &sparse_priv, session_id, - download_buffer); + store_sparse_image(&sparse, &sparse_priv, download_buffer); } else { write_raw_image(dev_desc, &info, cmd, download_buffer, download_bytes); diff --git a/common/fb_nand.c b/common/fb_nand.c index 9ca8602..896ed6d 100644 --- a/common/fb_nand.c +++ b/common/fb_nand.c @@ -126,7 +126,7 @@ static int fb_nand_sparse_write(struct sparse_storage *storage, return written / storage->block_sz; }
-void fb_nand_flash_write(const char *partname, unsigned int session_id, +void fb_nand_flash_write(const char *partname, void *download_buffer, unsigned int download_bytes, char *response) { @@ -161,7 +161,7 @@ void fb_nand_flash_write(const char *partname, unsigned int session_id, sparse.name = part->name; sparse.write = fb_nand_sparse_write;
- ret = store_sparse_image(&sparse, &sparse_priv, session_id, + ret = store_sparse_image(&sparse, &sparse_priv, download_buffer); } else { printf("Flashing raw image at offset 0x%llx\n", diff --git a/common/image-sparse.c b/common/image-sparse.c index 2bf737b..893c68b 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -52,8 +52,6 @@ typedef struct sparse_buffer { u16 type; } sparse_buffer_t;
-static uint32_t last_offset; - static unsigned int sparse_get_chunk_data_size(sparse_header_t *sparse, chunk_header_t *chunk) { @@ -267,8 +265,8 @@ static void sparse_put_data_buffer(sparse_buffer_t *buffer) free(buffer); }
-int store_sparse_image(sparse_storage_t *storage, void *storage_priv, - unsigned int session_id, void *data) +int store_sparse_image(sparse_storage_t *storage, + void *storage_priv, void *data) { unsigned int chunk, offset; sparse_header_t *sparse_header; @@ -303,19 +301,10 @@ int store_sparse_image(sparse_storage_t *storage, void *storage_priv, return -EINVAL; }
- /* - * If it's a new flashing session, start at the beginning of - * the partition. If not, then simply resume where we were. - */ - if (session_id > 0) - start = last_offset; - else - start = storage->start; - - printf("Flashing sparse image on partition %s at offset 0x%x (ID: %d)\n", - storage->name, start * storage->block_sz, session_id); + puts("Flashing Sparse Image\n");
/* Start processing chunks */ + start = storage->start; for (chunk = 0; chunk < sparse_header->total_chunks; chunk++) { uint32_t blkcnt;
@@ -390,7 +379,5 @@ int store_sparse_image(sparse_storage_t *storage, void *storage_priv, return -EIO; }
- last_offset = start + total_blocks; - return 0; } diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index 28b244a..ddf989c 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -59,7 +59,6 @@ static inline struct f_fastboot *func_to_fastboot(struct usb_function *f) }
static struct f_fastboot *fastboot_func; -static unsigned int fastboot_flash_session_id; static unsigned int download_size; static unsigned int download_bytes;
@@ -424,15 +423,6 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req)
sprintf(str_num, "0x%08x", CONFIG_FASTBOOT_BUF_SIZE); strncat(response, str_num, chars_left); - - /* - * This also indicates the start of a new flashing - * "session", in which we could have 1-N buffers to - * write to a partition. - * - * Reset our session counter. - */ - fastboot_flash_session_id = 0; } else if (!strcmp_l1("serialno", cmd)) { s = getenv("serial#"); if (s) @@ -600,16 +590,14 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req)
strcpy(response, "FAILno flash device defined"); #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV - fb_mmc_flash_write(cmd, fastboot_flash_session_id, - (void *)CONFIG_FASTBOOT_BUF_ADDR, + fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR, download_bytes, response); #endif #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV - fb_nand_flash_write(cmd, fastboot_flash_session_id, + fb_nand_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR, download_bytes, response); #endif - fastboot_flash_session_id++; fastboot_tx_write_str(response); } #endif diff --git a/include/fb_mmc.h b/include/fb_mmc.h index 978a139..402ba9b 100644 --- a/include/fb_mmc.h +++ b/include/fb_mmc.h @@ -4,7 +4,6 @@ * SPDX-License-Identifier: GPL-2.0+ */
-void fb_mmc_flash_write(const char *cmd, unsigned int session_id, - void *download_buffer, unsigned int download_bytes, - char *response); +void fb_mmc_flash_write(const char *cmd, void *download_buffer, + unsigned int download_bytes, char *response); void fb_mmc_erase(const char *cmd, char *response); diff --git a/include/fb_nand.h b/include/fb_nand.h index 80ddef5..88bdf36 100644 --- a/include/fb_nand.h +++ b/include/fb_nand.h @@ -5,7 +5,6 @@ * SPDX-License-Identifier: GPL-2.0+ */
-void fb_nand_flash_write(const char *cmd, unsigned int session_id, - void *download_buffer, unsigned int download_bytes, - char *response); +void fb_nand_flash_write(const char *cmd, void *download_buffer, + unsigned int download_bytes, char *response); void fb_nand_erase(const char *cmd, char *response); diff --git a/include/image-sparse.h b/include/image-sparse.h index 0382f5b..a2b0694 100644 --- a/include/image-sparse.h +++ b/include/image-sparse.h @@ -32,4 +32,4 @@ static inline int is_sparse_image(void *buf) }
int store_sparse_image(sparse_storage_t *storage, void *storage_priv, - unsigned int session_id, void *data); + void *data);

This file originally came from upstream code.
While retaining the storage abstraction feature, this is the first set of the changes required to resync with the cmd_flash_mmc_sparse_img() in the file aboot.c from https://us.codeaurora.org/cgit/quic/la/kernel/lk/plain/app/aboot/aboot.c?h=L...
Signed-off-by: Steve Rae srae@broadcom.com ---
common/fb_mmc.c | 32 ++-- common/fb_nand.c | 52 +++--- common/image-sparse.c | 449 +++++++++++++++++-------------------------------- include/image-sparse.h | 25 +-- 4 files changed, 208 insertions(+), 350 deletions(-)
diff --git a/common/fb_mmc.c b/common/fb_mmc.c index 9e53adb..3dad0ea 100644 --- a/common/fb_mmc.c +++ b/common/fb_mmc.c @@ -7,12 +7,10 @@ #include <config.h> #include <common.h> #include <blk.h> -#include <errno.h> #include <fastboot.h> #include <fb_mmc.h> #include <image-sparse.h> #include <part.h> -#include <sparse_format.h> #include <mmc.h> #include <div64.h>
@@ -48,22 +46,13 @@ static int part_get_info_efi_by_name_or_alias(struct blk_desc *dev_desc, return ret; }
- -static int fb_mmc_sparse_write(struct sparse_storage *storage, - void *priv, - unsigned int offset, - unsigned int size, - char *data) +static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt, const void *buffer) { - struct fb_mmc_sparse *sparse = priv; + struct fb_mmc_sparse *sparse = info->priv; struct blk_desc *dev_desc = sparse->dev_desc; - int ret; - - ret = blk_dwrite(dev_desc, offset, size, data); - if (!ret) - return -EIO;
- return ret; + return blk_dwrite(dev_desc, blk, blkcnt, buffer); }
static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info, @@ -139,26 +128,25 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
if (is_sparse_image(download_buffer)) { struct fb_mmc_sparse sparse_priv; - sparse_storage_t sparse; + struct sparse_storage sparse;
sparse_priv.dev_desc = dev_desc;
- sparse.block_sz = info.blksz; + sparse.blksz = info.blksz; sparse.start = info.start; sparse.size = info.size; - sparse.name = cmd; sparse.write = fb_mmc_sparse_write;
printf("Flashing sparse image at offset " LBAFU "\n", - info.start); + sparse.start);
- store_sparse_image(&sparse, &sparse_priv, download_buffer); + sparse.priv = &sparse_priv; + write_sparse_image(&sparse, cmd, download_buffer, + download_bytes, response_str); } else { write_raw_image(dev_desc, &info, cmd, download_buffer, download_bytes); } - - fastboot_okay(response_str, ""); }
void fb_mmc_erase(const char *cmd, char *response) diff --git a/common/fb_nand.c b/common/fb_nand.c index 896ed6d..6307bfc 100644 --- a/common/fb_nand.c +++ b/common/fb_nand.c @@ -10,7 +10,6 @@
#include <fastboot.h> #include <image-sparse.h> -#include <sparse_format.h>
#include <linux/mtd/mtd.h> #include <jffs2/jffs2.h> @@ -105,30 +104,32 @@ static int _fb_nand_write(nand_info_t *nand, struct part_info *part, buffer, flags); }
-static int fb_nand_sparse_write(struct sparse_storage *storage, - void *priv, - unsigned int offset, - unsigned int size, - char *data) +static lbaint_t fb_nand_sparse_write(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt, const void *buffer) { - struct fb_nand_sparse *sparse = priv; + struct fb_nand_sparse *sparse = info->priv; size_t written; int ret;
- ret = _fb_nand_write(sparse->nand, sparse->part, data, - offset * storage->block_sz, - size * storage->block_sz, &written); + ret = _fb_nand_write(sparse->nand, sparse->part, (void *)buffer, + blk * info->blksz, + blkcnt * info->blksz, &written); if (ret < 0) { printf("Failed to write sparse chunk\n"); return ret; }
- return written / storage->block_sz; +/* TODO - verify that the value "written" includes the "bad-blocks" ... */ + + /* + * the return value must be 'blkcnt' ("good-blocks") plus the + * number of "bad-blocks" encountered within this space... + */ + return written / info->blksz; }
-void fb_nand_flash_write(const char *partname, - void *download_buffer, unsigned int download_bytes, - char *response) +void fb_nand_flash_write(const char *cmd, void *download_buffer, + unsigned int download_bytes, char *response) { struct part_info *part; nand_info_t *nand = NULL; @@ -137,7 +138,7 @@ void fb_nand_flash_write(const char *partname, /* initialize the response buffer */ response_str = response;
- ret = fb_nand_lookup(partname, response, &nand, &part); + ret = fb_nand_lookup(cmd, response, &nand, &part); if (ret) { error("invalid NAND device"); fastboot_fail(response_str, "invalid NAND device"); @@ -150,19 +151,22 @@ void fb_nand_flash_write(const char *partname,
if (is_sparse_image(download_buffer)) { struct fb_nand_sparse sparse_priv; - sparse_storage_t sparse; + struct sparse_storage sparse;
sparse_priv.nand = nand; sparse_priv.part = part;
- sparse.block_sz = nand->writesize; - sparse.start = part->offset / sparse.block_sz; - sparse.size = part->size / sparse.block_sz; - sparse.name = part->name; + sparse.blksz = nand->writesize; + sparse.start = part->offset / sparse.blksz; + sparse.size = part->size / sparse.blksz; sparse.write = fb_nand_sparse_write;
- ret = store_sparse_image(&sparse, &sparse_priv, - download_buffer); + printf("Flashing sparse image at offset " LBAFU "\n", + sparse.start); + + sparse.priv = &sparse_priv; + write_sparse_image(&sparse, cmd, download_buffer, + download_bytes, response_str); } else { printf("Flashing raw image at offset 0x%llx\n", part->offset); @@ -182,7 +186,7 @@ void fb_nand_flash_write(const char *partname, fastboot_okay(response_str, ""); }
-void fb_nand_erase(const char *partname, char *response) +void fb_nand_erase(const char *cmd, char *response) { struct part_info *part; nand_info_t *nand = NULL; @@ -191,7 +195,7 @@ void fb_nand_erase(const char *partname, char *response) /* initialize the response buffer */ response_str = response;
- ret = fb_nand_lookup(partname, response, &nand, &part); + ret = fb_nand_lookup(cmd, response, &nand, &part); if (ret) { error("invalid NAND device"); fastboot_fail(response_str, "invalid NAND device"); diff --git a/common/image-sparse.c b/common/image-sparse.c index 893c68b..924cc63 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -36,54 +36,44 @@
#include <config.h> #include <common.h> -#include <div64.h> -#include <errno.h> #include <image-sparse.h> +#include <div64.h> #include <malloc.h> #include <part.h> #include <sparse_format.h> +#include <fastboot.h>
#include <linux/math64.h>
-typedef struct sparse_buffer { - void *data; - u32 length; - u32 repeat; - u16 type; -} sparse_buffer_t; - -static unsigned int sparse_get_chunk_data_size(sparse_header_t *sparse, - chunk_header_t *chunk) +void write_sparse_image( + struct sparse_storage *info, const char *part_name, + void *data, unsigned sz, char *response_str) { - return chunk->total_sz - sparse->chunk_hdr_sz; -} - -static unsigned int sparse_block_size_to_storage(unsigned int size, - sparse_storage_t *storage, - sparse_header_t *sparse) -{ - return (unsigned int)lldiv((uint64_t)size * sparse->blk_sz, - storage->block_sz); -} - -static bool sparse_chunk_has_buffer(chunk_header_t *chunk) -{ - switch (chunk->chunk_type) { - case CHUNK_TYPE_RAW: - case CHUNK_TYPE_FILL: - return true; - - default: - return false; - } -} + lbaint_t blk; + lbaint_t blkcnt; + lbaint_t blks; + uint32_t bytes_written = 0; + unsigned int chunk; + unsigned int offset; + unsigned int chunk_data_sz; + uint32_t *fill_buf = NULL; + uint32_t fill_val; + sparse_header_t *sparse_header; + chunk_header_t *chunk_header; + uint32_t total_blocks = 0; + int i;
-static sparse_header_t *sparse_parse_header(void **data) -{ /* Read and skip over sparse image header */ - sparse_header_t *sparse_header = (sparse_header_t *) *data; + sparse_header = (sparse_header_t *)data;
- *data += sparse_header->file_hdr_sz; + data += sparse_header->file_hdr_sz; + if (sparse_header->file_hdr_sz > sizeof(sparse_header_t)) { + /* + * Skip the remaining bytes in a header that is longer than + * we expected. + */ + data += (sparse_header->file_hdr_sz - sizeof(sparse_header_t)); + }
debug("=== Sparse Image Header ===\n"); debug("magic: 0x%x\n", sparse_header->magic); @@ -95,289 +85,164 @@ static sparse_header_t *sparse_parse_header(void **data) debug("total_blks: %d\n", sparse_header->total_blks); debug("total_chunks: %d\n", sparse_header->total_chunks);
- return sparse_header; -} - -static int sparse_parse_fill_chunk(sparse_header_t *sparse, - chunk_header_t *chunk) -{ - unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk); - - if (chunk_data_sz != sizeof(uint32_t)) - return -EINVAL; - - return 0; -} - -static int sparse_parse_raw_chunk(sparse_header_t *sparse, - chunk_header_t *chunk) -{ - unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk); - - /* Check if the data size is a multiple of the main block size */ - if (chunk_data_sz % sparse->blk_sz) - return -EINVAL; - - /* Check that the chunk size is consistent */ - if ((chunk_data_sz / sparse->blk_sz) != chunk->chunk_sz) - return -EINVAL; - - return 0; -} - -static chunk_header_t *sparse_parse_chunk(sparse_header_t *sparse, - void **image) -{ - chunk_header_t *chunk = (chunk_header_t *) *image; - int ret; - - debug("=== Chunk Header ===\n"); - debug("chunk_type: 0x%x\n", chunk->chunk_type); - debug("chunk_data_sz: 0x%x\n", chunk->chunk_sz); - debug("total_size: 0x%x\n", chunk->total_sz); - - switch (chunk->chunk_type) { - case CHUNK_TYPE_RAW: - ret = sparse_parse_raw_chunk(sparse, chunk); - if (ret) - return NULL; - break; - - case CHUNK_TYPE_FILL: - ret = sparse_parse_fill_chunk(sparse, chunk); - if (ret) - return NULL; - break; - - case CHUNK_TYPE_DONT_CARE: - case CHUNK_TYPE_CRC32: - debug("Ignoring chunk\n"); - break; - - default: - printf("%s: Unknown chunk type: %x\n", __func__, - chunk->chunk_type); - return NULL; - } - - *image += sparse->chunk_hdr_sz; - - return chunk; -} - -static int sparse_get_fill_buffer(sparse_header_t *sparse, - chunk_header_t *chunk, - sparse_buffer_t *buffer, - unsigned int blk_sz, - void *data) -{ - int i; - - buffer->type = CHUNK_TYPE_FILL; - - /* - * We create a buffer of one block, and ask it to be - * repeated as many times as needed. - */ - buffer->length = blk_sz; - buffer->repeat = (chunk->chunk_sz * sparse->blk_sz) / blk_sz; - - buffer->data = memalign(ARCH_DMA_MINALIGN, - ROUNDUP(blk_sz, - ARCH_DMA_MINALIGN)); - if (!buffer->data) - return -ENOMEM; - - for (i = 0; i < (buffer->length / sizeof(uint32_t)); i++) - ((uint32_t *)buffer->data)[i] = *(uint32_t *)(data); - - return 0; -} - -static int sparse_get_raw_buffer(sparse_header_t *sparse, - chunk_header_t *chunk, - sparse_buffer_t *buffer, - unsigned int blk_sz, - void *data) -{ - unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk); - - buffer->type = CHUNK_TYPE_RAW; - buffer->length = chunk_data_sz; - buffer->data = data; - buffer->repeat = 1; - - return 0; -} - -static sparse_buffer_t *sparse_get_data_buffer(sparse_header_t *sparse, - chunk_header_t *chunk, - unsigned int blk_sz, - void **image) -{ - unsigned int chunk_data_sz = sparse_get_chunk_data_size(sparse, chunk); - sparse_buffer_t *buffer; - void *data = *image; - int ret; - - *image += chunk_data_sz; - - if (!sparse_chunk_has_buffer(chunk)) - return NULL; - - buffer = calloc(sizeof(sparse_buffer_t), 1); - if (!buffer) - return NULL; - - switch (chunk->chunk_type) { - case CHUNK_TYPE_RAW: - ret = sparse_get_raw_buffer(sparse, chunk, buffer, blk_sz, - data); - if (ret) - return NULL; - break; - - case CHUNK_TYPE_FILL: - ret = sparse_get_fill_buffer(sparse, chunk, buffer, blk_sz, - data); - if (ret) - return NULL; - break; - - default: - return NULL; - } - - debug("=== Buffer ===\n"); - debug("length: 0x%x\n", buffer->length); - debug("repeat: 0x%x\n", buffer->repeat); - debug("type: 0x%x\n", buffer->type); - debug("data: 0x%p\n", buffer->data); - - return buffer; -} - -static void sparse_put_data_buffer(sparse_buffer_t *buffer) -{ - if (buffer->type == CHUNK_TYPE_FILL) - free(buffer->data); - - free(buffer); -} - -int store_sparse_image(sparse_storage_t *storage, - void *storage_priv, void *data) -{ - unsigned int chunk, offset; - sparse_header_t *sparse_header; - chunk_header_t *chunk_header; - sparse_buffer_t *buffer; - uint32_t start; - uint32_t total_blocks = 0; - int i; - - debug("=== Storage ===\n"); - debug("name: %s\n", storage->name); - debug("block_size: 0x%x\n", storage->block_sz); - debug("start: 0x%x\n", storage->start); - debug("size: 0x%x\n", storage->size); - debug("write: 0x%p\n", storage->write); - debug("priv: 0x%p\n", storage_priv); - - sparse_header = sparse_parse_header(&data); - if (!sparse_header) { - printf("sparse header issue\n"); - return -EINVAL; - } - /* * Verify that the sparse block size is a multiple of our * storage backend block size */ - div_u64_rem(sparse_header->blk_sz, storage->block_sz, &offset); + div_u64_rem(sparse_header->blk_sz, info->blksz, &offset); if (offset) { printf("%s: Sparse image block size issue [%u]\n", __func__, sparse_header->blk_sz); - return -EINVAL; + fastboot_fail(response_str, "sparse image block size issue"); + return; }
puts("Flashing Sparse Image\n");
/* Start processing chunks */ - start = storage->start; + blk = info->start; for (chunk = 0; chunk < sparse_header->total_chunks; chunk++) { - uint32_t blkcnt; - - chunk_header = sparse_parse_chunk(sparse_header, &data); - if (!chunk_header) { - printf("Unknown chunk type"); - return -EINVAL; + /* Read and skip over chunk header */ + chunk_header = (chunk_header_t *)data; + data += sizeof(chunk_header_t); + + if (chunk_header->chunk_type != CHUNK_TYPE_RAW) { + debug("=== Chunk Header ===\n"); + debug("chunk_type: 0x%x\n", chunk_header->chunk_type); + debug("chunk_data_sz: 0x%x\n", chunk_header->chunk_sz); + debug("total_size: 0x%x\n", chunk_header->total_sz); }
- /* - * If we have a DONT_CARE type, just skip the blocks - * and go on parsing the rest of the chunks - */ - if (chunk_header->chunk_type == CHUNK_TYPE_DONT_CARE) { - blkcnt = sparse_block_size_to_storage(chunk_header->chunk_sz, - storage, - sparse_header); -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV - total_blocks += blkcnt; -#endif - continue; + if (sparse_header->chunk_hdr_sz > sizeof(chunk_header_t)) { + /* + * Skip the remaining bytes in a header that is longer + * than we expected. + */ + data += (sparse_header->chunk_hdr_sz - + sizeof(chunk_header_t)); }
- /* Retrieve the buffer we're going to write */ - buffer = sparse_get_data_buffer(sparse_header, chunk_header, - storage->block_sz, &data); - if (!buffer) - continue; + chunk_data_sz = sparse_header->blk_sz * chunk_header->chunk_sz; + blkcnt = chunk_data_sz / info->blksz; + switch (chunk_header->chunk_type) { + case CHUNK_TYPE_RAW: + if (chunk_header->total_sz != + (sparse_header->chunk_hdr_sz + chunk_data_sz)) { + fastboot_fail(response_str, + "Bogus chunk size for chunk type Raw"); + return; + } + + if (blk + blkcnt > info->start + info->size) { + printf( + "%s: Request would exceed partition size!\n", + __func__); + fastboot_fail(response_str, + "Request would exceed partition size!"); + return; + }
- blkcnt = (buffer->length / storage->block_sz) * buffer->repeat; + blks = info->write(info, blk, blkcnt, data); + /* blks might be > blkcnt (eg. NAND bad-blocks) */ + if (blks < blkcnt) { + printf("%s: %s" LBAFU " [" LBAFU "]\n", + __func__, "Write failed, block #", + blk, blks); + fastboot_fail(response_str, + "flash write failure"); + return; + } + blk += blks; + bytes_written += blkcnt * info->blksz; + total_blocks += chunk_header->chunk_sz; + data += chunk_data_sz; + break; + + case CHUNK_TYPE_FILL: + if (chunk_header->total_sz != + (sparse_header->chunk_hdr_sz + sizeof(uint32_t))) { + fastboot_fail(response_str, + "Bogus chunk size for chunk type FILL"); + return; + }
- if ((start + total_blocks + blkcnt) > - (storage->start + storage->size)) { - printf("%s: Request would exceed partition size!\n", - __func__); - return -EINVAL; - } + fill_buf = (uint32_t *) + memalign(ARCH_DMA_MINALIGN, + ROUNDUP(info->blksz, + ARCH_DMA_MINALIGN)); + if (!fill_buf) { + fastboot_fail(response_str, + "Malloc failed for: CHUNK_TYPE_FILL"); + return; + }
- for (i = 0; i < buffer->repeat; i++) { - unsigned long buffer_blk_cnt; - int ret; + fill_val = *(uint32_t *)data; + data = (char *)data + sizeof(uint32_t);
- buffer_blk_cnt = buffer->length / storage->block_sz; + for (i = 0; i < (info->blksz / sizeof(fill_val)); i++) + fill_buf[i] = fill_val;
- ret = storage->write(storage, storage_priv, - start + total_blocks, - buffer_blk_cnt, - buffer->data); - if (ret < 0) { - printf("%s: Write %d failed %d\n", - __func__, i, ret); - return ret; + if (blk + blkcnt > info->start + info->size) { + printf( + "%s: Request would exceed partition size!\n", + __func__); + fastboot_fail(response_str, + "Request would exceed partition size!"); + return; }
- total_blocks += ret; - } + for (i = 0; i < blkcnt; i++) { + blks = info->write(info, blk, 1, fill_buf); + /* blks might be > 1 (eg. NAND bad-blocks) */ + if (blks < 1) { + printf("%s: %s, block # " LBAFU "\n", + __func__, "Write failed", blk); + fastboot_fail(response_str, + "flash write failure"); + free(fill_buf); + return; + } + blk += blks; + } + bytes_written += blkcnt * info->blksz; + total_blocks += chunk_data_sz / sparse_header->blk_sz; + free(fill_buf); + break;
- sparse_put_data_buffer(buffer); + case CHUNK_TYPE_DONT_CARE: +#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV + blk += blkcnt; + total_blocks += chunk_header->chunk_sz; +#endif + break; + + case CHUNK_TYPE_CRC32: + if (chunk_header->total_sz != + sparse_header->chunk_hdr_sz) { + fastboot_fail(response_str, + "Bogus chunk size for chunk type Dont Care"); + return; + } + total_blocks += chunk_header->chunk_sz; + data += chunk_data_sz; + break; + + default: + printf("%s: Unknown chunk type: %x\n", __func__, + chunk_header->chunk_type); + fastboot_fail(response_str, "Unknown chunk type"); + return; + } }
debug("Wrote %d blocks, expected to write %d blocks\n", - total_blocks, - sparse_block_size_to_storage(sparse_header->total_blks, - storage, sparse_header)); - printf("........ wrote %d blocks to '%s'\n", total_blocks, - storage->name); + total_blocks, sparse_header->total_blks); + printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
- if (total_blocks != - sparse_block_size_to_storage(sparse_header->total_blks, - storage, sparse_header)) { - printf("sparse image write failure\n"); - return -EIO; - } + if (total_blocks != sparse_header->total_blks) + fastboot_fail(response_str, "sparse image write failure"); + else + fastboot_okay(response_str, "");
- return 0; + return; } diff --git a/include/image-sparse.h b/include/image-sparse.h index a2b0694..4e9e784 100644 --- a/include/image-sparse.h +++ b/include/image-sparse.h @@ -9,16 +9,17 @@
#define ROUNDUP(x, y) (((x) + ((y) - 1)) & ~((y) - 1))
-typedef struct sparse_storage { - unsigned int block_sz; - unsigned int start; - unsigned int size; - const char *name; - - int (*write)(struct sparse_storage *storage, void *priv, - unsigned int offset, unsigned int size, - char *data); -} sparse_storage_t; +struct sparse_storage { + lbaint_t blksz; + lbaint_t start; + lbaint_t size; + void *priv; + + lbaint_t (*write)(struct sparse_storage *info, + lbaint_t blk, + lbaint_t blkcnt, + const void *buffer); +};
static inline int is_sparse_image(void *buf) { @@ -31,5 +32,5 @@ static inline int is_sparse_image(void *buf) return 0; }
-int store_sparse_image(sparse_storage_t *storage, void *storage_priv, - void *data); +void write_sparse_image(struct sparse_storage *info, const char *part_name, + void *data, unsigned sz, char *response_str);

Hi Steve,
On Fri, May 20, 2016 at 06:05:55PM -0700, Steve Rae wrote:
This file originally came from upstream code.
Which upstream?
While retaining the storage abstraction feature, this is the first set of the changes required to resync with the cmd_flash_mmc_sparse_img() in the file aboot.c from https://us.codeaurora.org/cgit/quic/la/kernel/lk/plain/app/aboot/aboot.c?h=L...
Having a list of what the changes are (and if possible split them in several patches).
It looks like there's a lot of very different things done in this patch.
Maxime

- update fastboot_okay() and fastboot_fail()
This file originally came from upstream code.
While retaining the storage abstraction feature, this is the second set of the changes required to resync with the cmd_flash_mmc_sparse_img() in the file aboot.c from https://us.codeaurora.org/cgit/quic/la/kernel/lk/plain/app/aboot/aboot.c?h=L...
Signed-off-by: Steve Rae srae@broadcom.com ---
common/fb_mmc.c | 40 ++++++++++++++++------------------------ common/fb_nand.c | 38 +++++++++++++++----------------------- common/image-sparse.c | 26 +++++++++++++------------- drivers/usb/gadget/f_fastboot.c | 31 +++++++++++++++++++------------ include/fastboot.h | 4 ++-- include/fb_mmc.h | 4 ++-- include/fb_nand.h | 4 ++-- include/image-sparse.h | 2 +- 8 files changed, 70 insertions(+), 79 deletions(-)
diff --git a/common/fb_mmc.c b/common/fb_mmc.c index 3dad0ea..6bafbc6 100644 --- a/common/fb_mmc.c +++ b/common/fb_mmc.c @@ -18,8 +18,6 @@ #define CONFIG_FASTBOOT_GPT_NAME GPT_ENTRY_NAME #endif
-static char *response_str; - struct fb_mmc_sparse { struct blk_desc *dev_desc; }; @@ -68,7 +66,7 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
if (blkcnt > info->size) { error("too large for partition: '%s'\n", part_name); - fastboot_fail(response_str, "too large for partition"); + fastboot_fail("too large for partition"); return; }
@@ -77,28 +75,25 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info, blks = blk_dwrite(dev_desc, info->start, blkcnt, buffer); if (blks != blkcnt) { error("failed writing to device %d\n", dev_desc->devnum); - fastboot_fail(response_str, "failed writing to device"); + fastboot_fail("failed writing to device"); return; }
printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz, part_name); - fastboot_okay(response_str, ""); + fastboot_okay(""); }
void fb_mmc_flash_write(const char *cmd, void *download_buffer, - unsigned int download_bytes, char *response) + unsigned int download_bytes) { struct blk_desc *dev_desc; disk_partition_t info;
- /* initialize the response buffer */ - response_str = response; - dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) { error("invalid mmc device\n"); - fastboot_fail(response_str, "invalid mmc device"); + fastboot_fail("invalid mmc device"); return; }
@@ -108,21 +103,21 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer, if (is_valid_gpt_buf(dev_desc, download_buffer)) { printf("%s: invalid GPT - refusing to write to flash\n", __func__); - fastboot_fail(response_str, "invalid GPT partition"); + fastboot_fail("invalid GPT partition"); return; } if (write_mbr_and_gpt_partitions(dev_desc, download_buffer)) { printf("%s: writing GPT partitions failed\n", __func__); - fastboot_fail(response_str, + fastboot_fail( "writing GPT partitions failed"); return; } printf("........ success\n"); - fastboot_okay(response_str, ""); + fastboot_okay(""); return; } else if (part_get_info_efi_by_name_or_alias(dev_desc, cmd, &info)) { error("cannot find partition: '%s'\n", cmd); - fastboot_fail(response_str, "cannot find partition"); + fastboot_fail("cannot find partition"); return; }
@@ -142,14 +137,14 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer,
sparse.priv = &sparse_priv; write_sparse_image(&sparse, cmd, download_buffer, - download_bytes, response_str); + download_bytes); } else { write_raw_image(dev_desc, &info, cmd, download_buffer, download_bytes); } }
-void fb_mmc_erase(const char *cmd, char *response) +void fb_mmc_erase(const char *cmd) { int ret; struct blk_desc *dev_desc; @@ -159,24 +154,21 @@ void fb_mmc_erase(const char *cmd, char *response)
if (mmc == NULL) { error("invalid mmc device"); - fastboot_fail(response_str, "invalid mmc device"); + fastboot_fail("invalid mmc device"); return; }
- /* initialize the response buffer */ - response_str = response; - dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) { error("invalid mmc device"); - fastboot_fail(response_str, "invalid mmc device"); + fastboot_fail("invalid mmc device"); return; }
ret = part_get_info_efi_by_name_or_alias(dev_desc, cmd, &info); if (ret) { error("cannot find partition: '%s'", cmd); - fastboot_fail(response_str, "cannot find partition"); + fastboot_fail("cannot find partition"); return; }
@@ -195,11 +187,11 @@ void fb_mmc_erase(const char *cmd, char *response) blks = dev_desc->block_erase(dev_desc, blks_start, blks_size); if (blks != blks_size) { error("failed erasing from device %d", dev_desc->devnum); - fastboot_fail(response_str, "failed erasing from device"); + fastboot_fail("failed erasing from device"); return; }
printf("........ erased " LBAFU " bytes from '%s'\n", blks_size * info.blksz, cmd); - fastboot_okay(response_str, ""); + fastboot_okay(""); } diff --git a/common/fb_nand.c b/common/fb_nand.c index 6307bfc..b5a3820 100644 --- a/common/fb_nand.c +++ b/common/fb_nand.c @@ -15,8 +15,6 @@ #include <jffs2/jffs2.h> #include <nand.h>
-static char *response_str; - struct fb_nand_sparse { nand_info_t *nand; struct part_info *part; @@ -32,7 +30,7 @@ __weak int board_fastboot_write_partition_setup(char *name) return 0; }
-static int fb_nand_lookup(const char *partname, char *response, +static int fb_nand_lookup(const char *partname, nand_info_t **nand, struct part_info **part) { @@ -43,21 +41,21 @@ static int fb_nand_lookup(const char *partname, char *response, ret = mtdparts_init(); if (ret) { error("Cannot initialize MTD partitions\n"); - fastboot_fail(response_str, "cannot init mtdparts"); + fastboot_fail("cannot init mtdparts"); return ret; }
ret = find_dev_and_part(partname, &dev, &pnum, part); if (ret) { error("cannot find partition: '%s'", partname); - fastboot_fail(response_str, "cannot find partition"); + fastboot_fail("cannot find partition"); return ret; }
if (dev->id->type != MTD_DEV_TYPE_NAND) { error("partition '%s' is not stored on a NAND device", partname); - fastboot_fail(response_str, "not a NAND device"); + fastboot_fail("not a NAND device"); return -EINVAL; }
@@ -129,19 +127,16 @@ static lbaint_t fb_nand_sparse_write(struct sparse_storage *info, }
void fb_nand_flash_write(const char *cmd, void *download_buffer, - unsigned int download_bytes, char *response) + unsigned int download_bytes) { struct part_info *part; nand_info_t *nand = NULL; int ret;
- /* initialize the response buffer */ - response_str = response; - - ret = fb_nand_lookup(cmd, response, &nand, &part); + ret = fb_nand_lookup(cmd, &nand, &part); if (ret) { error("invalid NAND device"); - fastboot_fail(response_str, "invalid NAND device"); + fastboot_fail("invalid NAND device"); return; }
@@ -166,7 +161,7 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer,
sparse.priv = &sparse_priv; write_sparse_image(&sparse, cmd, download_buffer, - download_bytes, response_str); + download_bytes); } else { printf("Flashing raw image at offset 0x%llx\n", part->offset); @@ -179,26 +174,23 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer, }
if (ret) { - fastboot_fail(response_str, "error writing the image"); + fastboot_fail("error writing the image"); return; }
- fastboot_okay(response_str, ""); + fastboot_okay(""); }
-void fb_nand_erase(const char *cmd, char *response) +void fb_nand_erase(const char *cmd) { struct part_info *part; nand_info_t *nand = NULL; int ret;
- /* initialize the response buffer */ - response_str = response; - - ret = fb_nand_lookup(cmd, response, &nand, &part); + ret = fb_nand_lookup(cmd, &nand, &part); if (ret) { error("invalid NAND device"); - fastboot_fail(response_str, "invalid NAND device"); + fastboot_fail("invalid NAND device"); return; }
@@ -209,9 +201,9 @@ void fb_nand_erase(const char *cmd, char *response) ret = _fb_nand_erase(nand, part); if (ret) { error("failed erasing from device %s", nand->name); - fastboot_fail(response_str, "failed erasing from device"); + fastboot_fail("failed erasing from device"); return; }
- fastboot_okay(response_str, ""); + fastboot_okay(""); } diff --git a/common/image-sparse.c b/common/image-sparse.c index 924cc63..b36703b 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -47,7 +47,7 @@
void write_sparse_image( struct sparse_storage *info, const char *part_name, - void *data, unsigned sz, char *response_str) + void *data, unsigned sz) { lbaint_t blk; lbaint_t blkcnt; @@ -93,7 +93,7 @@ void write_sparse_image( if (offset) { printf("%s: Sparse image block size issue [%u]\n", __func__, sparse_header->blk_sz); - fastboot_fail(response_str, "sparse image block size issue"); + fastboot_fail("sparse image block size issue"); return; }
@@ -128,7 +128,7 @@ void write_sparse_image( case CHUNK_TYPE_RAW: if (chunk_header->total_sz != (sparse_header->chunk_hdr_sz + chunk_data_sz)) { - fastboot_fail(response_str, + fastboot_fail( "Bogus chunk size for chunk type Raw"); return; } @@ -137,7 +137,7 @@ void write_sparse_image( printf( "%s: Request would exceed partition size!\n", __func__); - fastboot_fail(response_str, + fastboot_fail( "Request would exceed partition size!"); return; } @@ -148,7 +148,7 @@ void write_sparse_image( printf("%s: %s" LBAFU " [" LBAFU "]\n", __func__, "Write failed, block #", blk, blks); - fastboot_fail(response_str, + fastboot_fail( "flash write failure"); return; } @@ -161,7 +161,7 @@ void write_sparse_image( case CHUNK_TYPE_FILL: if (chunk_header->total_sz != (sparse_header->chunk_hdr_sz + sizeof(uint32_t))) { - fastboot_fail(response_str, + fastboot_fail( "Bogus chunk size for chunk type FILL"); return; } @@ -171,7 +171,7 @@ void write_sparse_image( ROUNDUP(info->blksz, ARCH_DMA_MINALIGN)); if (!fill_buf) { - fastboot_fail(response_str, + fastboot_fail( "Malloc failed for: CHUNK_TYPE_FILL"); return; } @@ -186,7 +186,7 @@ void write_sparse_image( printf( "%s: Request would exceed partition size!\n", __func__); - fastboot_fail(response_str, + fastboot_fail( "Request would exceed partition size!"); return; } @@ -197,7 +197,7 @@ void write_sparse_image( if (blks < 1) { printf("%s: %s, block # " LBAFU "\n", __func__, "Write failed", blk); - fastboot_fail(response_str, + fastboot_fail( "flash write failure"); free(fill_buf); return; @@ -219,7 +219,7 @@ void write_sparse_image( case CHUNK_TYPE_CRC32: if (chunk_header->total_sz != sparse_header->chunk_hdr_sz) { - fastboot_fail(response_str, + fastboot_fail( "Bogus chunk size for chunk type Dont Care"); return; } @@ -230,7 +230,7 @@ void write_sparse_image( default: printf("%s: Unknown chunk type: %x\n", __func__, chunk_header->chunk_type); - fastboot_fail(response_str, "Unknown chunk type"); + fastboot_fail("Unknown chunk type"); return; } } @@ -240,9 +240,9 @@ void write_sparse_image( printf("........ wrote %u bytes to '%s'\n", bytes_written, part_name);
if (total_blocks != sparse_header->total_blks) - fastboot_fail(response_str, "sparse image write failure"); + fastboot_fail("sparse image write failure"); else - fastboot_okay(response_str, ""); + fastboot_okay("");
return; } diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index ddf989c..2160b1c 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -151,16 +151,18 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req); static int strcmp_l1(const char *s1, const char *s2);
-void fastboot_fail(char *response, const char *reason) +static char *fb_response_str; + +void fastboot_fail(const char *reason) { - strncpy(response, "FAIL\0", 5); - strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1); + strncpy(fb_response_str, "FAIL\0", 5); + strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 - 1); }
-void fastboot_okay(char *response, const char *reason) +void fastboot_okay(const char *reason) { - strncpy(response, "OKAY\0", 5); - strncat(response, reason, FASTBOOT_RESPONSE_LEN - 4 - 1); + strncpy(fb_response_str, "OKAY\0", 5); + strncat(fb_response_str, reason, FASTBOOT_RESPONSE_LEN - 4 - 1); }
static void fastboot_complete(struct usb_ep *ep, struct usb_request *req) @@ -588,15 +590,18 @@ static void cb_flash(struct usb_ep *ep, struct usb_request *req) return; }
- strcpy(response, "FAILno flash device defined"); + /* initialize the response buffer */ + fb_response_str = response; + + fastboot_fail("no flash device defined"); #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV fb_mmc_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR, - download_bytes, response); + download_bytes); #endif #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV fb_nand_flash_write(cmd, (void *)CONFIG_FASTBOOT_BUF_ADDR, - download_bytes, response); + download_bytes); #endif fastboot_tx_write_str(response); } @@ -637,13 +642,15 @@ static void cb_erase(struct usb_ep *ep, struct usb_request *req) return; }
- strcpy(response, "FAILno flash device defined"); + /* initialize the response buffer */ + fb_response_str = response;
+ fastboot_fail("no flash device defined"); #ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV - fb_mmc_erase(cmd, response); + fb_mmc_erase(cmd); #endif #ifdef CONFIG_FASTBOOT_FLASH_NAND_DEV - fb_nand_erase(cmd, response); + fb_nand_erase(cmd); #endif fastboot_tx_write_str(response); } diff --git a/include/fastboot.h b/include/fastboot.h index db826d2..616631e 100644 --- a/include/fastboot.h +++ b/include/fastboot.h @@ -16,7 +16,7 @@ /* The 64 defined bytes plus \0 */ #define FASTBOOT_RESPONSE_LEN (64 + 1)
-void fastboot_fail(char *response, const char *reason); -void fastboot_okay(char *response, const char *reason); +void fastboot_fail(const char *reason); +void fastboot_okay(const char *reason);
#endif /* _FASTBOOT_H_ */ diff --git a/include/fb_mmc.h b/include/fb_mmc.h index 402ba9b..12b99cb 100644 --- a/include/fb_mmc.h +++ b/include/fb_mmc.h @@ -5,5 +5,5 @@ */
void fb_mmc_flash_write(const char *cmd, void *download_buffer, - unsigned int download_bytes, char *response); -void fb_mmc_erase(const char *cmd, char *response); + unsigned int download_bytes); +void fb_mmc_erase(const char *cmd); diff --git a/include/fb_nand.h b/include/fb_nand.h index 88bdf36..aaf7cf7 100644 --- a/include/fb_nand.h +++ b/include/fb_nand.h @@ -6,5 +6,5 @@ */
void fb_nand_flash_write(const char *cmd, void *download_buffer, - unsigned int download_bytes, char *response); -void fb_nand_erase(const char *cmd, char *response); + unsigned int download_bytes); +void fb_nand_erase(const char *cmd); diff --git a/include/image-sparse.h b/include/image-sparse.h index 4e9e784..f6869d6 100644 --- a/include/image-sparse.h +++ b/include/image-sparse.h @@ -33,4 +33,4 @@ static inline int is_sparse_image(void *buf) }
void write_sparse_image(struct sparse_storage *info, const char *part_name, - void *data, unsigned sz, char *response_str); + void *data, unsigned sz);

On Fri, May 20, 2016 at 06:05:56PM -0700, Steve Rae wrote:
- update fastboot_okay() and fastboot_fail()
This file originally came from upstream code.
While retaining the storage abstraction feature, this is the second set of the changes required to resync with the cmd_flash_mmc_sparse_img() in the file aboot.c from https://us.codeaurora.org/cgit/quic/la/kernel/lk/plain/app/aboot/aboot.c?h=L...
Why is this needed?
Maxime

In order to process the CHUNK_TYPE_DONT_CARE properly, there is a requirement to be able to 'reserve' a specified number of blocks in the storage media. Because of the special handling of "bad blocks" in NAND devices, this is implemented in a storage abstraction function.
Signed-off-by: Steve Rae srae@broadcom.com ---
common/fb_mmc.c | 7 +++++++ common/fb_nand.c | 20 ++++++++++++++++++++ common/image-sparse.c | 5 ++--- include/image-sparse.h | 4 ++++ 4 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/common/fb_mmc.c b/common/fb_mmc.c index 6bafbc6..c739651 100644 --- a/common/fb_mmc.c +++ b/common/fb_mmc.c @@ -53,6 +53,12 @@ static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info, return blk_dwrite(dev_desc, blk, blkcnt, buffer); }
+static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt) +{ + return blkcnt; +} + static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info, const char *part_name, void *buffer, unsigned int download_bytes) @@ -131,6 +137,7 @@ void fb_mmc_flash_write(const char *cmd, void *download_buffer, sparse.start = info.start; sparse.size = info.size; sparse.write = fb_mmc_sparse_write; + sparse.reserve = fb_mmc_sparse_reserve;
printf("Flashing sparse image at offset " LBAFU "\n", sparse.start); diff --git a/common/fb_nand.c b/common/fb_nand.c index b5a3820..ce63006 100644 --- a/common/fb_nand.c +++ b/common/fb_nand.c @@ -126,6 +126,25 @@ static lbaint_t fb_nand_sparse_write(struct sparse_storage *info, return written / info->blksz; }
+static lbaint_t fb_nand_sparse_reserve(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt) +{ + int bad_blocks = 0; + +/* + * TODO - implement a function to determine the total number + * of blocks which must be used in order to reserve the specified + * number ("blkcnt") of "good-blocks", starting at "blk"... + * ( possibly something like the "check_skip_len()" function ) + */ + + /* + * the return value must be 'blkcnt' ("good-blocks") plus the + * number of "bad-blocks" encountered within this space... + */ + return blkcnt + bad_blocks; +} + void fb_nand_flash_write(const char *cmd, void *download_buffer, unsigned int download_bytes) { @@ -155,6 +174,7 @@ void fb_nand_flash_write(const char *cmd, void *download_buffer, sparse.start = part->offset / sparse.blksz; sparse.size = part->size / sparse.blksz; sparse.write = fb_nand_sparse_write; + sparse.reserve = fb_nand_sparse_reserve;
printf("Flashing sparse image at offset " LBAFU "\n", sparse.start); diff --git a/common/image-sparse.c b/common/image-sparse.c index b36703b..9632c6f 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -1,3 +1,4 @@ + /* * Copyright (c) 2009, Google Inc. * All rights reserved. @@ -210,10 +211,8 @@ void write_sparse_image( break;
case CHUNK_TYPE_DONT_CARE: -#ifdef CONFIG_FASTBOOT_FLASH_MMC_DEV - blk += blkcnt; + blk += info->reserve(info, blk, blkcnt); total_blocks += chunk_header->chunk_sz; -#endif break;
case CHUNK_TYPE_CRC32: diff --git a/include/image-sparse.h b/include/image-sparse.h index f6869d6..b0cc500 100644 --- a/include/image-sparse.h +++ b/include/image-sparse.h @@ -19,6 +19,10 @@ struct sparse_storage { lbaint_t blk, lbaint_t blkcnt, const void *buffer); + + lbaint_t (*reserve)(struct sparse_storage *info, + lbaint_t blk, + lbaint_t blkcnt); };
static inline int is_sparse_image(void *buf)

- increase the size of the fill buffer - testing has shown a 10x improvement when the sparse image has large CHUNK_TYPE_FILL chunks
Signed-off-by: Steve Rae srae@broadcom.com ---
common/image-sparse.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/common/image-sparse.c b/common/image-sparse.c index 9632c6f..ddf5772 100644 --- a/common/image-sparse.c +++ b/common/image-sparse.c @@ -1,4 +1,3 @@ - /* * Copyright (c) 2009, Google Inc. * All rights reserved. @@ -46,6 +45,10 @@
#include <linux/math64.h>
+#ifndef CONFIG_FASTBOOT_FLASH_FILLBUF_SIZE +#define CONFIG_FASTBOOT_FLASH_FILLBUF_SIZE (1024 * 512) +#endif + void write_sparse_image( struct sparse_storage *info, const char *part_name, void *data, unsigned sz) @@ -62,7 +65,11 @@ void write_sparse_image( sparse_header_t *sparse_header; chunk_header_t *chunk_header; uint32_t total_blocks = 0; + int fill_buf_num_blks; int i; + int j; + + fill_buf_num_blks = CONFIG_FASTBOOT_FLASH_FILLBUF_SIZE / info->blksz;
/* Read and skip over sparse image header */ sparse_header = (sparse_header_t *)data; @@ -169,8 +176,9 @@ void write_sparse_image(
fill_buf = (uint32_t *) memalign(ARCH_DMA_MINALIGN, - ROUNDUP(info->blksz, - ARCH_DMA_MINALIGN)); + ROUNDUP( + info->blksz * fill_buf_num_blks, + ARCH_DMA_MINALIGN)); if (!fill_buf) { fastboot_fail( "Malloc failed for: CHUNK_TYPE_FILL"); @@ -180,7 +188,10 @@ void write_sparse_image( fill_val = *(uint32_t *)data; data = (char *)data + sizeof(uint32_t);
- for (i = 0; i < (info->blksz / sizeof(fill_val)); i++) + for (i = 0; + i < (info->blksz * fill_buf_num_blks / + sizeof(fill_val)); + i++) fill_buf[i] = fill_val;
if (blk + blkcnt > info->start + info->size) { @@ -192,18 +203,24 @@ void write_sparse_image( return; }
- for (i = 0; i < blkcnt; i++) { - blks = info->write(info, blk, 1, fill_buf); - /* blks might be > 1 (eg. NAND bad-blocks) */ - if (blks < 1) { - printf("%s: %s, block # " LBAFU "\n", - __func__, "Write failed", blk); + for (i = 0; i < blkcnt;) { + j = blkcnt - i; + if (j > fill_buf_num_blks) + j = fill_buf_num_blks; + blks = info->write(info, blk, j, fill_buf); + /* blks might be > j (eg. NAND bad-blocks) */ + if (blks < j) { + printf("%s: %s " LBAFU " [%d]\n", + __func__, + "Write failed, block #", + blk, j); fastboot_fail( "flash write failure"); free(fill_buf); return; } blk += blks; + i += j; } bytes_written += blkcnt * info->blksz; total_blocks += chunk_data_sz / sparse_header->blk_sz;

On Fri, May 20, 2016 at 6:05 PM, Steve Rae srae@broadcom.com wrote:
While retaining the storage abstraction feature implemented in U-Boot, this series updates the fastboot sparse image handling by (1) fixing broken code, (2) resync'ing with the upstream code, and (3) improving performance when writing CHUNK_TYPE_FILL
Steve Rae (5): fastboot: sparse: remove session-id logic fastboot: sparse: resync common/image-sparse.c (part 1) fastboot: sparse: resync common/image-sparse.c (part 2) fastboot: sparse: implement reserve() fastboot: sparse: improve CHUNK_TYPE_FILL write performance
common/fb_mmc.c | 79 +++---- common/fb_nand.c | 100 +++++---- common/image-sparse.c | 478 +++++++++++++++------------------------- drivers/usb/gadget/f_fastboot.c | 47 ++-- include/fastboot.h | 4 +- include/fb_mmc.h | 7 +- include/fb_nand.h | 7 +- include/image-sparse.h | 29 ++- 8 files changed, 309 insertions(+), 442 deletions(-)
-- 1.8.5
Tom, please pull this series. Thanks, Steve
participants (3)
-
Maxime Ripard
-
Steve Rae
-
Steve Rae