[U-Boot] [PATCH v3 2/4] usb/gadget: fastboot: add eMMC support for flash command

Marek Vasut
2014-07-31T01:37:50Z

On Thursday, June 26, 2014 at 10:13:22 PM, Steve Rae wrote:
[...]
> +
> +#include <common.h>
> +#include <fb_mmc.h>
> +#include <part.h>
> +#include <sparse_format.h>
> +
> +/* The 64 defined bytes plus \0 */
> +#define RESPONSE_LEN (64 + 1)
> +
> +static char *response_str;

I'd suggest to pass this "response_str" around instead of making it global.

> +static void fastboot_resp(const char *s)
> +{
> + strncpy(response_str, s, RESPONSE_LEN);
> + response_str[RESPONSE_LEN - 1] = '\0';

This could be shrunk to a single snprintf(response_str, RESPONSE_LENGTH, s); I
think, but I'm not sure if the overhead won't grow.

> +}
> +
> +static int is_sparse_image(void *buf)
> +{
> + sparse_header_t *s_header = (sparse_header_t *)buf;
> +
> + if ((le32_to_cpu(s_header->magic) == SPARSE_HEADER_MAGIC) &&
> + (le16_to_cpu(s_header->major_version) == 1))
> + return 1;
> +
> + return 0;
> +}
> +
> +static void write_sparse_image(block_dev_desc_t *dev_desc,
> + disk_partition_t *info, const char *part_name,
> + void *buffer, unsigned int download_bytes)
> +{
> + lbaint_t blk;
> + lbaint_t blkcnt;
> + lbaint_t blks;
> + sparse_header_t *s_header = (sparse_header_t *)buffer;
> + chunk_header_t *c_header;
> + void *buf;
> + uint32_t blk_sz;
> + uint32_t remaining_chunks;
> + uint32_t bytes_written = 0;
> +
> + blk_sz = le32_to_cpu(s_header->blk_sz);
> +
> + /* verify s_header->blk_sz is exact multiple of info->blksz */
> + if (blk_sz != (blk_sz & ~(info->blksz - 1))) {
> + printf("%s: Sparse image block size issue [%u]\n",
> + __func__, blk_sz);
> + fastboot_resp("FAILsparse image block size issue");

Can't you just make the fastboot_resp() function a variadic one AND move the
printf() into the fastboot_resp() function? You could then even get consistent
output on both the device and in the response if you snprintf() into the
response_str first and then printf() the response_str .

> + return;
> + }

[...]

> +static void write_raw_image(block_dev_desc_t *dev_desc, disk_partition_t
> *info, + const char *part_name, void *buffer,
> + unsigned int download_bytes)
> +{
> + lbaint_t blkcnt;
> + lbaint_t blks;
> +
> + /* determine number of blocks to write */
> + blkcnt = ((download_bytes + (info->blksz - 1)) & ~(info->blksz - 1));
> + blkcnt = blkcnt / info->blksz;
> +
> + if (blkcnt > info->size) {
> + printf("%s: too large for partition: '%s'\n", __func__,
> + part_name);
> + fastboot_resp("FAILtoo large for partition");
> + return;
> + }
> +
> + printf("Flashing Raw Image\n");

Use puts() here and everywhere where printf() is not taking any args please.

> + blks = dev_desc->block_write(dev_desc->dev, info->start, blkcnt,
> + buffer);
> + if (blks != blkcnt) {
> + printf("%s: failed writing to device %d\n", __func__,
> + dev_desc->dev);
> + fastboot_resp("FAILfailed writing to device");
> + return;
> + }
> +
> + printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz,
> + part_name);
> + fastboot_resp("OKAY");
> +} …