Re: [PATCH 3/4] fastboot: blk: introduce fastboot block flashing support

Hi Dmitrii,

Thank you for the patch and sorry for the review delay.

On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev dimorinny@google.com wrote:

Reuse common logic between existing mmc and new introduced block implementation

Signed-off-by: Dmitrii Merkurev dimorinny@google.com Cc: Alex Kiernan alex.kiernan@gmail.com Cc: Patrick Delaunay patrick.delaunay@foss.st.com Cc: Simon Glass sjg@chromium.org Cc: Mattijs Korpershoek mkorpershoek@baylibre.com Cc: Ying-Chun Liu (PaulLiu) paul.liu@linaro.org

This change (when applied along with 1/4 and 2/4) introduces a build for sandbox:

$ make sandbox_defconfig $ make

[...]

drivers/fastboot/fb_mmc.c: In function 'fastboot_mmc_erase': drivers/fastboot/fb_mmc.c:596:16: warning: implicit declaration of function 'fb_block_write'; did you mean 'blk_write'? [-Wimplicit-function-declaration] 596 | blks = fb_block_write(dev_desc, blks_start, blks_size, NULL); | ^~~~~~~~~~~~~~ | blk_write

[...]

LTO u-boot /usr/bin/ld: /tmp/cczd5cS1.ltrans18.ltrans.o: in function `erase.lto_priv.0': /mnt/work/upstream/u-boot/drivers/fastboot/fb_mmc.c:596: undefined reference to `fb_block_write' collect2: error: ld returned 1 exit status make: *** [Makefile:1798: u-boot] Error 1

---

drivers/fastboot/Makefile | 4 +- drivers/fastboot/fb_block.c | 200 ++++++++++++++++++++++++++++++++++++ drivers/fastboot/fb_mmc.c | 120 ++-------------------- include/fb_block.h | 70 +++++++++++++ 4 files changed, 281 insertions(+), 113 deletions(-) create mode 100644 drivers/fastboot/fb_block.c create mode 100644 include/fb_block.h

diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile index 048af5aa82..91e98763e8 100644 --- a/drivers/fastboot/Makefile +++ b/drivers/fastboot/Makefile @@ -3,5 +3,7 @@ obj-y += fb_common.o obj-y += fb_getvar.o obj-y += fb_command.o -obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o +obj-$(CONFIG_FASTBOOT_FLASH_BLOCK) += fb_block.o +# MMC reuses block implementation +obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_block.o fb_mmc.o obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o diff --git a/drivers/fastboot/fb_block.c b/drivers/fastboot/fb_block.c new file mode 100644 index 0000000000..908decd544 --- /dev/null +++ b/drivers/fastboot/fb_block.c @@ -0,0 +1,200 @@ +// SPDX-License-Identifier: BSD-2-Clause +/*

• • Copyright (C) 2024 The Android Open Source Project
• */

+#include <blk.h> +#include <div64.h> +#include <fastboot-internal.h> +#include <fastboot.h> +#include <fb_block.h> +#include <image-sparse.h> +#include <part.h>

+/**

• • FASTBOOT_MAX_BLOCKS_ERASE - maximum blocks to erase per derase call
• • in the ERASE case we can have much larger buffer size since
• • we're not transferring an actual buffer
• */

+#define FASTBOOT_MAX_BLOCKS_ERASE 1048576 +/**

• • FASTBOOT_MAX_BLOCKS_WRITE - maximum blocks to write per dwrite call
• */

+#define FASTBOOT_MAX_BLOCKS_WRITE 65536

+struct fb_block_sparse {

• struct blk_desc *dev_desc;

+};

+static lbaint_t fb_block_write(struct blk_desc *block_dev, lbaint_t start,

• 	       lbaint_t blkcnt, const void *buffer)
  

+{

• lbaint_t blk = start;
• lbaint_t blks_written = 0;
• lbaint_t cur_blkcnt = 0;
• lbaint_t blks = 0;
• int step = buffer ? FASTBOOT_MAX_BLOCKS_WRITE : FASTBOOT_MAX_BLOCKS_ERASE;
• int i;
• for (i = 0; i < blkcnt; i += step) {

• cur_blkcnt = min((int)blkcnt - i, step);
  

• if (buffer) {
  

• 	if (fastboot_progress_callback)
  

• 		fastboot_progress_callback("writing");
  

• 	blks_written = blk_dwrite(block_dev, blk, cur_blkcnt,
  

• 				  buffer + (i * block_dev->blksz));
  

• } else {
  

• 	if (fastboot_progress_callback)
  

• 		fastboot_progress_callback("erasing");
  

• 	blks_written = blk_derase(block_dev, blk, cur_blkcnt);
  

• }
  

• blk += blks_written;
  

• blks += blks_written;
  

• }
• return blks;

+}

+static lbaint_t fb_block_sparse_write(struct sparse_storage *info,

• 		      lbaint_t blk, lbaint_t blkcnt,
  

• 		      const void *buffer)
  

+{

• struct fb_block_sparse *sparse = info->priv;
• struct blk_desc *dev_desc = sparse->dev_desc;
• return fb_block_write(dev_desc, blk, blkcnt, buffer);

+}

+static lbaint_t fb_block_sparse_reserve(struct sparse_storage *info,

• 			lbaint_t blk, lbaint_t blkcnt)
  

+{

• return blkcnt;

+}

+int fastboot_block_get_part_info(const char *part_name,

• 		 struct blk_desc **dev_desc,
  

• 		 struct disk_partition *part_info,
  

• 		 char *response)
  

+{

• int ret;
• const char *interface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,

• 				   CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME,
  

• 				   NULL);
  

• const int device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,

• 			      CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID, -1);
  

• if (!part_name || !strcmp(part_name, "")) {

• fastboot_fail("partition not given", response);
  

• return -ENOENT;
  

• }
• if (!interface || !strcmp(interface, "")) {

• fastboot_fail("block interface isn't provided", response);
  

• return -EINVAL;
  

• }
• *dev_desc = blk_get_dev(interface, device);
• if (!dev_desc) {

• fastboot_fail("no such device", response);
  

• return -ENODEV;
  

• }
• ret = part_get_info_by_name(*dev_desc, part_name, part_info);
• if (ret < 0)

• fastboot_fail("failed to get partition info", response);
  

• return ret;

+}

+void fastboot_block_erase(const char *part_name, char *response) +{

• struct blk_desc *dev_desc;
• struct disk_partition part_info;
• lbaint_t written;
• if (fastboot_block_get_part_info(part_name, &dev_desc, &part_info, response) < 0)

• return;
  

• written = fb_block_write(dev_desc, part_info.start, part_info.size, NULL);
• if (written != part_info.size) {

• fastboot_fail("failed to erase partition", response);
  

• return;
  

• }
• fastboot_okay(NULL, response);

+}

+void fastboot_block_write_raw_image(struct blk_desc *dev_desc,

• 		    struct disk_partition *info, const char *part_name,
  

• 		    void *buffer, u32 download_bytes, char *response)
  

+{

• lbaint_t blkcnt;
• lbaint_t blks;
• /* determine number of blocks to write */
• blkcnt = ((download_bytes + (info->blksz - 1)) & ~(info->blksz - 1));
• blkcnt = lldiv(blkcnt, info->blksz);
• if (blkcnt > info->size) {

• pr_err("too large for partition: '%s'\n", part_name);
  

• fastboot_fail("too large for partition", response);
  

• return;
  

• }
• puts("Flashing Raw Image\n");
• blks = fb_block_write(dev_desc, info->start, blkcnt, buffer);
• if (blks != blkcnt) {

• pr_err("failed writing to device %d\n", dev_desc->devnum);
  

• fastboot_fail("failed writing to device", response);
  

• return;
  

• }
• printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz,

•       part_name);
  

• fastboot_okay(NULL, response);

+}

+void fastboot_block_write_sparse_image(struct blk_desc *dev_desc, struct disk_partition *info,

• 		       const char *part_name, void *buffer, char *response)
  

+{

• struct fb_block_sparse sparse_priv;
• struct sparse_storage sparse;
• int err;
• sparse_priv.dev_desc = dev_desc;
• sparse.blksz = info->blksz;
• sparse.start = info->start;
• sparse.size = info->size;
• sparse.write = fb_block_sparse_write;
• sparse.reserve = fb_block_sparse_reserve;
• sparse.mssg = fastboot_fail;
• printf("Flashing sparse image at offset " LBAFU "\n",

•       sparse.start);
  

• sparse.priv = &sparse_priv;
• err = write_sparse_image(&sparse, part_name, buffer,

• 		 response);
  

• if (!err)

• fastboot_okay(NULL, response);
  

+}

+void fastboot_block_flash_write(const char *part_name, void *download_buffer,

• 		u32 download_bytes, char *response)
  

+{

• struct blk_desc *dev_desc;
• struct disk_partition part_info;
• if (fastboot_block_get_part_info(part_name, &dev_desc, &part_info, response) < 0)

• return;
  

• if (is_sparse_image(download_buffer)) {

• fastboot_block_write_sparse_image(dev_desc, &part_info, part_name,
  

• 				  download_buffer, response);
  

• } else {

• fastboot_block_write_raw_image(dev_desc, &part_info, part_name,
  

• 			       download_buffer, download_bytes, response);
  

• }

+} diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index 060918e491..7894f01f63 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -9,6 +9,7 @@ #include <env.h> #include <fastboot.h> #include <fastboot-internal.h> +#include <fb_block.h> #include <fb_mmc.h> #include <image-sparse.h> #include <image.h> @@ -21,10 +22,6 @@

#define BOOT_PARTITION_NAME "boot"

-struct fb_mmc_sparse {

• struct blk_desc *dev_desc;

-};

static int raw_part_get_info_by_name(struct blk_desc *dev_desc, const char *name, struct disk_partition *info) @@ -115,88 +112,6 @@ static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, return do_get_part_info(dev_desc, name, info); }

-/**

• • fb_mmc_blk_write() - Write/erase MMC in chunks of FASTBOOT_MAX_BLK_WRITE
• • @block_dev: Pointer to block device
• • @start: First block to write/erase
• • @blkcnt: Count of blocks
• • @buffer: Pointer to data buffer for write or NULL for erase
• */

-static lbaint_t fb_mmc_blk_write(struct blk_desc *block_dev, lbaint_t start,

• 		 lbaint_t blkcnt, const void *buffer)
  

-{

• lbaint_t blk = start;
• lbaint_t blks_written;
• lbaint_t cur_blkcnt;
• lbaint_t blks = 0;
• int i;
• for (i = 0; i < blkcnt; i += FASTBOOT_MAX_BLK_WRITE) {

• cur_blkcnt = min((int)blkcnt - i, FASTBOOT_MAX_BLK_WRITE);
  

• if (buffer) {
  

• 	if (fastboot_progress_callback)
  

• 		fastboot_progress_callback("writing");
  

• 	blks_written = blk_dwrite(block_dev, blk, cur_blkcnt,
  

• 				  buffer + (i * block_dev->blksz));
  

• } else {
  

• 	if (fastboot_progress_callback)
  

• 		fastboot_progress_callback("erasing");
  

• 	blks_written = blk_derase(block_dev, blk, cur_blkcnt);
  

• }
  

• blk += blks_written;
  

• blks += blks_written;
  

• }
• return blks;

-}

-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 = info->priv;
• struct blk_desc *dev_desc = sparse->dev_desc;
• return fb_mmc_blk_write(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,

• 	    struct disk_partition *info, const char *part_name,
  

• 	    void *buffer, u32 download_bytes, char *response)
  

-{

• lbaint_t blkcnt;
• lbaint_t blks;
• /* determine number of blocks to write */
• blkcnt = ((download_bytes + (info->blksz - 1)) & ~(info->blksz - 1));
• blkcnt = lldiv(blkcnt, info->blksz);
• if (blkcnt > info->size) {

• pr_err("too large for partition: '%s'\n", part_name);
  

• fastboot_fail("too large for partition", response);
  

• return;
  

• }
• puts("Flashing Raw Image\n");
• blks = fb_mmc_blk_write(dev_desc, info->start, blkcnt, buffer);
• if (blks != blkcnt) {

• pr_err("failed writing to device %d\n", dev_desc->devnum);
  

• fastboot_fail("failed writing to device", response);
  

• return;
  

• }
• printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * info->blksz,

•       part_name);
  

• fastboot_okay(NULL, response);

-}

#if defined(CONFIG_FASTBOOT_MMC_BOOT_SUPPORT) || \ defined(CONFIG_FASTBOOT_MMC_USER_SUPPORT) static int fb_mmc_erase_mmc_hwpart(struct blk_desc *dev_desc) @@ -205,7 +120,7 @@ static int fb_mmc_erase_mmc_hwpart(struct blk_desc *dev_desc)

debug("Start Erasing mmc hwpart[%u]...\n", dev_desc->hwpart);

• blks = fb_mmc_blk_write(dev_desc, 0, dev_desc->lba, NULL);

• blks = fb_block_write(dev_desc, 0, dev_desc->lba, NULL);

  if (blks != dev_desc->lba) { pr_err("Failed to erase mmc hwpart[%u]\n", dev_desc->hwpart);

@@ -249,7 +164,7 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,

debug("Start Flashing Image to EMMC_BOOT%d...\n", hwpart);

• blks = fb_mmc_blk_write(dev_desc, 0, blkcnt, buffer);
  

• blks = fb_block_write(dev_desc, 0, blkcnt, buffer);
  
  

  if (blks != blkcnt) { pr_err("Failed to write EMMC_BOOT%d\n", hwpart);

@@ -610,30 +525,11 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer, return;

if (is_sparse_image(download_buffer)) {

• struct fb_mmc_sparse sparse_priv;
  

• struct sparse_storage sparse;
  

• int err;
  

• sparse_priv.dev_desc = dev_desc;
  

• sparse.blksz = info.blksz;
  

• sparse.start = info.start;
  

• sparse.size = info.size;
  

• sparse.write = fb_mmc_sparse_write;
  

• sparse.reserve = fb_mmc_sparse_reserve;
  

• sparse.mssg = fastboot_fail;
  

• printf("Flashing sparse image at offset " LBAFU "\n",
  

•        sparse.start);
  

• sparse.priv = &sparse_priv;
  

• err = write_sparse_image(&sparse, cmd, download_buffer,
  

• 			 response);
  

• if (!err)
  

• 	fastboot_okay(NULL, response);
  

• fastboot_block_write_sparse_image(dev_desc, &info, cmd,
  

• 				  download_buffer, response);
  

  } else {

• write_raw_image(dev_desc, &info, cmd, download_buffer,
  

• 		download_bytes, response);
  

• fastboot_block_write_raw_image(dev_desc, &info, cmd, download_buffer,
  

• 			       download_bytes, response);
  

  }

}

@@ -697,7 +593,7 @@ void fastboot_mmc_erase(const char *cmd, char *response) printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n", blks_start, blks_start + blks_size);

• blks = fb_mmc_blk_write(dev_desc, blks_start, blks_size, NULL);

• blks = fb_block_write(dev_desc, blks_start, blks_size, NULL);

  if (blks != blks_size) { pr_err("failed erasing from device %d\n", dev_desc->devnum);

diff --git a/include/fb_block.h b/include/fb_block.h new file mode 100644 index 0000000000..90208e21a7 --- /dev/null +++ b/include/fb_block.h @@ -0,0 +1,70 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/*

• • Copyright (C) 2024 The Android Open Source Project
• */

+#ifndef _FB_BLOCK_H_ +#define _FB_BLOCK_H_

+struct blk_desc; +struct disk_partition;

+/**

• • fastboot_block_get_part_info() - Lookup block partition by name
• • @part_name: Named partition to lookup
• • @dev_desc: Pointer to returned blk_desc pointer
• • @part_info: Pointer to returned struct disk_partition
• • @response: Pointer to fastboot response buffer
• */

+int fastboot_block_get_part_info(const char *part_name,

• 		 struct blk_desc **dev_desc,
  

• 		 struct disk_partition *part_info,
  

• 		 char *response);
  

+/**

• • fastboot_block_erase() - Erase partition on block device for fastboot
• • @part_name: Named partition to erase
• • @response: Pointer to fastboot response buffer
• */

+void fastboot_block_erase(const char *part_name, char *response);

+/**

• • fastboot_block_write_raw_image() - Write raw image to block device
• • @dev_desc: Block device we're going write to
• • @info: Partition we're going write to
• • @part_name: Name of partition we're going write to
• • @buffer: Downloaded buffer pointer
• • @download_bytes: Size of content on downloaded buffer pointer
• • @response: Pointer to fastboot response buffer
• */

+void fastboot_block_write_raw_image(struct blk_desc *dev_desc,

• 		    struct disk_partition *info, const char *part_name,
  

• 		    void *buffer, u32 download_bytes, char *response);
  

+/**

• • fastboot_block_write_sparse_image() - Write sparse image to block device
• • @dev_desc: Block device we're going write to
• • @info: Partition we're going write to
• • @part_name: Name of partition we're going write to
• • @buffer: Downloaded buffer pointer
• • @response: Pointer to fastboot response buffer
• */

+void fastboot_block_write_sparse_image(struct blk_desc *dev_desc, struct disk_partition *info,

• 		       const char *part_name, void *buffer, char *response);
  

+/**

• • fastboot_block_flash_write() - Write image to block device for fastboot
• • @part_name: Named partition to write image to
• • @download_buffer: Pointer to image data
• • @download_bytes: Size of image data
• • @response: Pointer to fastboot response buffer
• */

+void fastboot_block_flash_write(const char *part_name, void *download_buffer,

• 		u32 download_bytes, char *response);
  

+#endif // _FB_BLOCK_H_

2.44.0.278.ge034bb2e1d-goog