[PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[neil.armstrong]

This serie permits using any block device as target
for fastboot by moving the generic block logic into
a common set of helpers and also use them as generic
backend.

The erase logic has been extended to support software
erase since only 2 block drivers exposes the erase
operation.

Tests are welcome to make sure this series doesn't
introduce any regressions on the emmc backend.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v3:
- Move Kconfig/Makefile changes over the 2 patches
- Relicence to GPL2 with Dmitrii approval
- Move soft erase in a separate function
- Update help text of Kconfig BLOCK entries
- Add warning at init if MMC was selected with BLOCK backend
- Link to v2: https://lore.kernel.org/r/20250409-topic-fastboot-blk-v2-0-c676f21d414f@linaro.org

Changes in v2:
- Dropped applied virtio erase patch
- Reorganize patches, introducing helpers first, using them in mmc afterwards
- Added soft-erase logic
- Added move helpers to handle the partitions erase & flash from emmc
- Fixed const var on last patch
- Link to v1: https://lore.kernel.org/all/20240306185921.1854109-1-dimorinny@google.com/

---
Dmitrii Merkurev (3):
      fastboot: blk: introduce fastboot block flashing support
      fastboot: blk: switch emmc to use the block helpers
      fastboot: integrate block flashing back-end

 drivers/fastboot/Kconfig      |  28 +++-
 drivers/fastboot/Makefile     |   4 +-
 drivers/fastboot/fb_block.c   | 323 ++++++++++++++++++++++++++++++++++++++++++
 drivers/fastboot/fb_command.c |   8 ++
 drivers/fastboot/fb_common.c  |  22 ++-
 drivers/fastboot/fb_getvar.c  |   8 +-
 drivers/fastboot/fb_mmc.c     | 210 ++-------------------------
 include/fb_block.h            | 105 ++++++++++++++
 8 files changed, 502 insertions(+), 206 deletions(-)
---
base-commit: 4d3b5c679bc9d5c6cbbeedcc1e4a186f1cc35541
change-id: 20250408-topic-fastboot-blk-c5e14cd59224

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH RFT v3 1/3] fastboot: blk: introduce fastboot block flashing support

[neil.armstrong]

From: Dmitrii Merkurev <dimorinny@google.com>

Introduce fastboot block flashing functions and helpers
to be shared with the MMC implementation.

The write logic comes from the mmc implementation, while
the partition lookup is much simpler and could be extended.

For the erase logic, allmost no block drivers exposes the
erase operation, except mmc & virtio, so in order to allow
erasiong any partition a soft-erase logic has been added
to write zero-ed buffers in a loop.

Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/fastboot/fb_block.c | 323 ++++++++++++++++++++++++++++++++++++++++++++
 include/fb_block.h          | 105 ++++++++++++++
 2 files changed, 428 insertions(+)

diff --git a/drivers/fastboot/fb_block.c b/drivers/fastboot/fb_block.c
new file mode 100644
index 0000000000000000000000000000000000000000..b725397c91af2717812e69e2b624076eb30f781d
--- /dev/null
+++ b/drivers/fastboot/fb_block.c
@@ -0,0 +1,323 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * 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>
+#include <malloc.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_SOFT_ERASE - maximum blocks to software erase at once
+ */
+#define FASTBOOT_MAX_BLOCKS_SOFT_ERASE 4096
+/**
+ * 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;
+};
+
+/* Write 0s instead of using erase operation, inefficient but functional */
+static lbaint_t fb_block_soft_erase(struct blk_desc *block_dev, lbaint_t blk,
+				    lbaint_t cur_blkcnt, lbaint_t erase_buf_blks,
+				    void *erase_buffer)
+{
+	lbaint_t blks_written = 0;
+	int j;
+
+	memset(erase_buffer, 0, erase_buf_blks * block_dev->blksz);
+
+	for (j = 0; j < cur_blkcnt; j += erase_buf_blks) {
+		lbaint_t remain = min_t(lbaint_t, cur_blkcnt - j,
+				erase_buf_blks);
+
+		blks_written += blk_dwrite(block_dev, blk + j,
+				remain, erase_buffer);
+		printf(".");
+	}
+
+	return blks_written;
+}
+
+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;
+	void *erase_buf = NULL;
+	int erase_buf_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");
+
+			if (!erase_buf) {
+				blks_written = blk_derase(block_dev, blk, cur_blkcnt);
+
+				/* Allocate erase buffer if erase is not implemented */
+				if ((long)blks_written == -ENOSYS) {
+					erase_buf_blks = min_t(long, blkcnt,
+							       FASTBOOT_MAX_BLOCKS_SOFT_ERASE);
+					erase_buf = malloc(erase_buf_blks * block_dev->blksz);
+
+					printf("Slowly writing empty buffers due to missing erase operation\n");
+				}
+			}
+
+			if (erase_buf)
+				blks_written = fb_block_soft_erase(block_dev, blk, cur_blkcnt,
+								   erase_buf_blks, erase_buf);
+		}
+		blk += blks_written;
+		blks += blks_written;
+	}
+
+	if (erase_buf)
+		free(erase_buf);
+
+	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_raw_erase_disk(struct blk_desc *dev_desc, const char *disk_name,
+				   char *response)
+{
+	lbaint_t written;
+
+	debug("Start Erasing %s...\n", disk_name);
+
+	written = fb_block_write(dev_desc, 0, dev_desc->lba, NULL);
+	if (written != dev_desc->lba) {
+		pr_err("Failed to erase %s\n", disk_name);
+		fastboot_response("FAIL", response, "Failed to erase %s", disk_name);
+		return;
+	}
+
+	printf("........ erased " LBAFU " bytes from '%s'\n",
+	       dev_desc->lba * dev_desc->blksz, disk_name);
+	fastboot_okay(NULL, response);
+}
+
+void fastboot_block_raw_erase(struct blk_desc *dev_desc, struct disk_partition *info,
+			      const char *part_name, uint alignment, char *response)
+{
+	lbaint_t written, blks_start, blks_size;
+
+	if (alignment) {
+		blks_start = (info->start + alignment - 1) & ~(alignment - 1);
+		if (info->size >= alignment)
+			blks_size = (info->size - (blks_start - info->start)) &
+				(~(alignment - 1));
+		else
+			blks_size = 0;
+
+		printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n",
+		       blks_start, blks_start + blks_size);
+	} else {
+		blks_start = info->start;
+		blks_size = info->size;
+	}
+
+	written = fb_block_write(dev_desc, blks_start, blks_size, NULL);
+	if (written != blks_size) {
+		fastboot_fail("failed to erase partition", response);
+		return;
+	}
+
+	printf("........ erased " LBAFU " bytes from '%s'\n",
+	       blks_size * info->blksz, part_name);
+	fastboot_okay(NULL, response);
+}
+
+void fastboot_block_erase(const char *part_name, 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;
+
+	fastboot_block_raw_erase(dev_desc, &part_info, part_name, 0, response);
+}
+
+void fastboot_block_write_raw_disk(struct blk_desc *dev_desc, const char *disk_name,
+				   void *buffer, u32 download_bytes, char *response)
+{
+	lbaint_t blkcnt;
+	lbaint_t blks;
+
+	/* determine number of blocks to write */
+	blkcnt = ((download_bytes + (dev_desc->blksz - 1)) & ~(dev_desc->blksz - 1));
+	blkcnt = lldiv(blkcnt, dev_desc->blksz);
+
+	if (blkcnt > dev_desc->lba) {
+		pr_err("too large for disk: '%s'\n", disk_name);
+		fastboot_fail("too large for disk", response);
+		return;
+	}
+
+	printf("Flashing Raw Image\n");
+
+	blks = fb_block_write(dev_desc, 0, blkcnt, buffer);
+
+	if (blks != blkcnt) {
+		pr_err("failed writing to %s\n", disk_name);
+		fastboot_fail("failed writing to device", response);
+		return;
+	}
+
+	printf("........ wrote " LBAFU " bytes to '%s'\n", blkcnt * dev_desc->blksz,
+	       disk_name);
+	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;
+	}
+
+	printf("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/include/fb_block.h b/include/fb_block.h
new file mode 100644
index 0000000000000000000000000000000000000000..189c708e2f0eaccf4871f68f93c8d38eabc4349a
--- /dev/null
+++ b/include/fb_block.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * 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
+ * Return: 0 on success, -ve on error
+ */
+int fastboot_block_get_part_info(const char *part_name,
+				 struct blk_desc **dev_desc,
+				 struct disk_partition *part_info,
+				 char *response);
+
+/**
+ * fastboot_block_raw_erase() - Erase raw block device partition
+ *
+ * @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
+ * @alignment: erase start and size alignment, specify 0 to ignore
+ * @response: Pointer to fastboot response buffer
+ */
+void fastboot_block_raw_erase(struct blk_desc *dev_desc, struct disk_partition *info,
+			      const char *part_name, uint alignment, char *response);
+
+/**
+ * fastboot_block_raw_erase_disk() - Erase raw block device
+ *
+ * @dev_desc: Block device we're going write to
+ * @disk_name: Name of disk we're going write to
+ * @response: Pointer to fastboot response buffer
+ */
+void fastboot_block_raw_erase_disk(struct blk_desc *dev_desc, const char *disk_name,
+				   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_disk() - Write raw image to block device
+ *
+ * @dev_desc: Block device we're going write to
+ * @disk_name: Name of disk 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_disk(struct blk_desc *dev_desc, const char *disk_name,
+				   void *buffer, u32 download_bytes, char *response);
+
+/**
+ * fastboot_block_write_raw_image() - Write raw image to block device partition
+ *
+ * @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 partition
+ *
+ * @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.34.1

[PATCH RFT v3 2/3] fastboot: blk: switch emmc to use the block helpers

[neil.armstrong]

From: Dmitrii Merkurev <dimorinny@google.com>

Switch the mmc backend to this new shared block helpers,
reducing block logic and only leaving MMC specific logic.

Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/fastboot/Kconfig  |   4 +-
 drivers/fastboot/Makefile |   3 +-
 drivers/fastboot/fb_mmc.c | 210 +++-------------------------------------------
 3 files changed, 16 insertions(+), 201 deletions(-)

diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
index 70207573de2bd0d56b4b7fa6f7e17fdc5803ba15..33825ee408fbd9aff26cd390a140421c7c98ecc3 100644
--- a/drivers/fastboot/Kconfig
+++ b/drivers/fastboot/Kconfig
@@ -91,7 +91,7 @@ config FASTBOOT_USB_DEV
 config FASTBOOT_FLASH
 	bool "Enable FASTBOOT FLASH command"
 	default y if ARCH_SUNXI || ARCH_ROCKCHIP
-	depends on MMC || (MTD_RAW_NAND && CMD_MTDPARTS)
+	depends on MMC || (MTD_RAW_NAND && CMD_MTDPARTS) || BLK
 	select IMAGE_SPARSE
 	help
 	  The fastboot protocol includes a "flash" command for writing
@@ -113,7 +113,7 @@ choice
 
 config FASTBOOT_FLASH_MMC
 	bool "FASTBOOT on MMC"
-	depends on MMC
+	depends on MMC && BLK
 
 config FASTBOOT_FLASH_NAND
 	bool "FASTBOOT on NAND"
diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
index 048af5aa823436956142a536c5f7dcf1a8948726..c2214c968ab357371f5d3d27ecc9c1a3e9404e89 100644
--- a/drivers/fastboot/Makefile
+++ b/drivers/fastboot/Makefile
@@ -3,5 +3,6 @@
 obj-y += fb_common.o
 obj-y += fb_getvar.o
 obj-y += fb_command.o
-obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.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_mmc.c b/drivers/fastboot/fb_mmc.c
index dca7c222f35659b22d327541b245760a6a6d7b35..11d9c8e84602c7434733c060b84c91c38772ac9f 100644
--- a/drivers/fastboot/fb_mmc.c
+++ b/drivers/fastboot/fb_mmc.c
@@ -8,6 +8,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>
@@ -20,10 +21,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)
@@ -114,118 +111,10 @@ 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)
-{
-	lbaint_t blks;
-
-	debug("Start Erasing mmc hwpart[%u]...\n", dev_desc->hwpart);
-
-	blks = fb_mmc_blk_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);
-		return 1;
-	}
-
-	printf("........ erased %llu bytes from mmc hwpart[%u]\n",
-	       (u64)(dev_desc->lba * dev_desc->blksz), dev_desc->hwpart);
-
-	return 0;
-}
-#endif
-
 #ifdef CONFIG_FASTBOOT_MMC_BOOT_SUPPORT
 static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
 			    int hwpart, u32 buff_sz, char *response)
 {
-	lbaint_t blkcnt;
-	lbaint_t blks;
-	unsigned long blksz;
-
 	// To operate on EMMC_BOOT1/2 (mmc0boot0/1) we first change the hwpart
 	if (blk_dselect_hwpart(dev_desc, hwpart)) {
 		pr_err("Failed to select hwpart\n");
@@ -233,42 +122,11 @@ static void fb_mmc_boot_ops(struct blk_desc *dev_desc, void *buffer,
 		return;
 	}
 
-	if (buffer) { /* flash */
-
-		/* determine number of blocks to write */
-		blksz = dev_desc->blksz;
-		blkcnt = ((buff_sz + (blksz - 1)) & ~(blksz - 1));
-		blkcnt = lldiv(blkcnt, blksz);
-
-		if (blkcnt > dev_desc->lba) {
-			pr_err("Image size too large\n");
-			fastboot_fail("Image size too large", response);
-			return;
-		}
-
-		debug("Start Flashing Image to EMMC_BOOT%d...\n", hwpart);
-
-		blks = fb_mmc_blk_write(dev_desc, 0, blkcnt, buffer);
-
-		if (blks != blkcnt) {
-			pr_err("Failed to write EMMC_BOOT%d\n", hwpart);
-			fastboot_fail("Failed to write EMMC_BOOT part",
-				      response);
-			return;
-		}
-
-		printf("........ wrote %llu bytes to EMMC_BOOT%d\n",
-		       (u64)(blkcnt * blksz), hwpart);
-	} else { /* erase */
-		if (fb_mmc_erase_mmc_hwpart(dev_desc)) {
-			pr_err("Failed to erase EMMC_BOOT%d\n", hwpart);
-			fastboot_fail("Failed to erase EMMC_BOOT part",
-				      response);
-			return;
-		}
-	}
-
-	fastboot_okay(NULL, response);
+	if (buffer) /* flash */
+		fastboot_block_write_raw_disk(dev_desc, "EMMC_BOOT",
+					      buffer, buff_sz, response);
+	else /* erase */
+		fastboot_block_raw_erase_disk(dev_desc, "EMMC_BOOT", response);
 }
 #endif
 
@@ -609,30 +467,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);
 	}
 }
 
@@ -646,7 +485,6 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 {
 	struct blk_desc *dev_desc;
 	struct disk_partition info;
-	lbaint_t blks, blks_start, blks_size, grp_size;
 	struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV);
 
 #ifdef CONFIG_FASTBOOT_MMC_BOOT_SUPPORT
@@ -673,10 +511,7 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 		if (!dev_desc)
 			return;
 
-		if (fb_mmc_erase_mmc_hwpart(dev_desc))
-			fastboot_fail("Failed to erase EMMC_USER", response);
-		else
-			fastboot_okay(NULL, response);
+		fastboot_block_raw_erase_disk(dev_desc, "EMMC_USER", response);
 		return;
 	}
 #endif
@@ -685,26 +520,5 @@ void fastboot_mmc_erase(const char *cmd, char *response)
 		return;
 
 	/* Align blocks to erase group size to avoid erasing other partitions */
-	grp_size = mmc->erase_grp_size;
-	blks_start = (info.start + grp_size - 1) & ~(grp_size - 1);
-	if (info.size >= grp_size)
-		blks_size = (info.size - (blks_start - info.start)) &
-				(~(grp_size - 1));
-	else
-		blks_size = 0;
-
-	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);
-
-	if (blks != blks_size) {
-		pr_err("failed erasing from device %d\n", dev_desc->devnum);
-		fastboot_fail("failed erasing from device", response);
-		return;
-	}
-
-	printf("........ erased " LBAFU " bytes from '%s'\n",
-	       blks_size * info.blksz, cmd);
-	fastboot_okay(NULL, response);
+	fastboot_block_raw_erase(dev_desc, &info, cmd, mmc->erase_grp_size, response);
 }

-- 
2.34.1

[PATCH RFT v3 3/3] fastboot: integrate block flashing back-end

[neil.armstrong]

From: Dmitrii Merkurev <dimorinny@google.com>

1. Get partition info/size
2. Erase partition
3. Flash partition
4. BCB

Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/fastboot/Kconfig      | 24 ++++++++++++++++++++++++
 drivers/fastboot/Makefile     |  1 +
 drivers/fastboot/fb_command.c |  8 ++++++++
 drivers/fastboot/fb_common.c  | 22 ++++++++++++++++++----
 drivers/fastboot/fb_getvar.c  |  8 +++++++-
 5 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
index 33825ee408fbd9aff26cd390a140421c7c98ecc3..68967abb751ed512f157bc32bfafc863943a2265 100644
--- a/drivers/fastboot/Kconfig
+++ b/drivers/fastboot/Kconfig
@@ -119,6 +119,10 @@ config FASTBOOT_FLASH_NAND
 	bool "FASTBOOT on NAND"
 	depends on MTD_RAW_NAND && CMD_MTDPARTS
 
+config FASTBOOT_FLASH_BLOCK
+	bool "FASTBOOT on block device"
+	depends on BLK
+
 endchoice
 
 config FASTBOOT_FLASH_MMC_DEV
@@ -193,6 +197,26 @@ config FASTBOOT_MMC_USER_NAME
 	  defined here.
 	  The default target name for erasing EMMC_USER is "mmc0".
 
+config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
+	string "Define FASTBOOT block interface name"
+	depends on FASTBOOT_FLASH_BLOCK
+	help
+	  The fastboot "flash" and "erase" commands support operations
+	  on any Block device, this should specify the block device name
+	  like virtio, scsi, etc...
+	  The mmc block device type can be used but most of the features
+	  available in the FASTBOOT_MMC will be missing.
+	  Consider using FASTBOOT_MMC on a MMC block device until all
+	  features are migrated.
+
+config FASTBOOT_FLASH_BLOCK_DEVICE_ID
+	int "Define FASTBOOT block device identifier"
+	depends on FASTBOOT_FLASH_BLOCK
+	help
+	  The fastboot "flash" and "erase" commands support operations
+	  on any Block device, this should specify the block device
+	  identifier on the system, as a number.
+
 config FASTBOOT_GPT_NAME
 	string "Target name for updating GPT"
 	depends on FASTBOOT_FLASH_MMC && EFI_PARTITION
diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
index c2214c968ab357371f5d3d27ecc9c1a3e9404e89..91e98763e8eab84ccd9b8e5354ff1419f61ef372 100644
--- a/drivers/fastboot/Makefile
+++ b/drivers/fastboot/Makefile
@@ -3,6 +3,7 @@
 obj-y += fb_common.o
 obj-y += fb_getvar.o
 obj-y += fb_command.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_command.c b/drivers/fastboot/fb_command.c
index 2cdbac50ac4a0ce501753e95c1918ffa5d11158d..e6aee13e01618ee6567bf00527d3df327ae06f1c 100644
--- a/drivers/fastboot/fb_command.c
+++ b/drivers/fastboot/fb_command.c
@@ -8,6 +8,7 @@
 #include <env.h>
 #include <fastboot.h>
 #include <fastboot-internal.h>
+#include <fb_block.h>
 #include <fb_mmc.h>
 #include <fb_nand.h>
 #include <part.h>
@@ -337,6 +338,10 @@ void fastboot_data_complete(char *response)
  */
 static void __maybe_unused flash(char *cmd_parameter, char *response)
 {
+	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
+		fastboot_block_flash_write(cmd_parameter, fastboot_buf_addr,
+					   image_size, response);
+
 	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
 		fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
 					 image_size, response);
@@ -357,6 +362,9 @@ static void __maybe_unused flash(char *cmd_parameter, char *response)
  */
 static void __maybe_unused erase(char *cmd_parameter, char *response)
 {
+	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK))
+		fastboot_block_erase(cmd_parameter, response);
+
 	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
 		fastboot_mmc_erase(cmd_parameter, response);
 
diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
index 68f92c4b887c8442cc212b8613fb70c7251cdcdf..dac5528f80908bf5b1224284c9ecd492394e4f0e 100644
--- a/drivers/fastboot/fb_common.c
+++ b/drivers/fastboot/fb_common.c
@@ -97,16 +97,24 @@ int __weak fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
 		[FASTBOOT_REBOOT_REASON_FASTBOOTD] = "boot-fastboot",
 		[FASTBOOT_REBOOT_REASON_RECOVERY] = "boot-recovery"
 	};
-	const int mmc_dev = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
-					       CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
 
-	if (!IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
+	int device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
+					CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID, -1);
+	if (device == -1) {
+		device = config_opt_enabled(CONFIG_FASTBOOT_FLASH_MMC,
+					    CONFIG_FASTBOOT_FLASH_MMC_DEV, -1);
+	}
+	const char *bcb_iface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
+						   CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME,
+						   "mmc");
+
+	if (device == -1)
 		return -EINVAL;
 
 	if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
 		return -EINVAL;
 
-	ret = bcb_find_partition_and_load("mmc", mmc_dev, "misc");
+	ret = bcb_find_partition_and_load(bcb_iface, device, "misc");
 	if (ret)
 		goto out;
 
@@ -226,8 +234,14 @@ void fastboot_set_progress_callback(void (*progress)(const char *msg))
  */
 void fastboot_init(void *buf_addr, u32 buf_size)
 {
+#if IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)
+	if (!strcmp(CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME, "mmc"))
+		printf("Warning: the fastboot block backend features are limited, consider using the MMC backend\n");
+#endif
+
 	fastboot_buf_addr = buf_addr ? buf_addr :
 				       (void *)CONFIG_FASTBOOT_BUF_ADDR;
 	fastboot_buf_size = buf_size ? buf_size : CONFIG_FASTBOOT_BUF_SIZE;
 	fastboot_set_progress_callback(NULL);
+
 }
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 9c2ce65a4e5bce0da6b18aa1b2818f7db556c528..f083b21c797dc7e55315f2cba017a4372483fa92 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -7,6 +7,7 @@
 #include <fastboot.h>
 #include <fastboot-internal.h>
 #include <fb_mmc.h>
+#include <fb_block.h>
 #include <fb_nand.h>
 #include <fs.h>
 #include <part.h>
@@ -114,7 +115,12 @@ static int getvar_get_part_info(const char *part_name, char *response,
 	struct disk_partition disk_part;
 	struct part_info *part_info;
 
-	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
+	if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_BLOCK)) {
+		r = fastboot_block_get_part_info(part_name, &dev_desc, &disk_part,
+						 response);
+		if (r >= 0 && size)
+			*size = disk_part.size * disk_part.blksz;
+	} else if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC)) {
 		r = fastboot_mmc_get_part_info(part_name, &dev_desc, &disk_part,
 					       response);
 		if (r >= 0 && size)

-- 
2.34.1

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Mattijs Korpershoek]

Hi Neil,

Thank you for the series.

On mar., mai 06, 2025 at 18:10, neil.armstrong@linaro.org wrote:

> This serie permits using any block device as target
> for fastboot by moving the generic block logic into
> a common set of helpers and also use them as generic
> backend.
>
> The erase logic has been extended to support software
> erase since only 2 block drivers exposes the erase
> operation.
>
> Tests are welcome to make sure this series doesn't
> introduce any regressions on the emmc backend.

I've tested the series on khadas-vim3_android_defconfig
keeping CONFIG_FASTBOOT_FLASH_MMC=y (so with the emmc backend)

I tested the following:
# flashing a raw partition with a descriptor:
$ fastboot flash bootloader u-boot_kvim3_noab.bin
# flashing a sparse image:
$ fastboot flash super super.img

I also testing reflashing the super.img when using the generic block
layer:
=> fastboot usb 0
Warning: the fastboot block backend features are limited, consider using the MMC backend

$ fastboot flash super super.img

Both work great!

Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

I've also opened a issue on gitlab to harmonize the fastboot emmc
backend with the generic block backend:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/issues/6

>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> Changes in v3:
> - Move Kconfig/Makefile changes over the 2 patches
> - Relicence to GPL2 with Dmitrii approval
> - Move soft erase in a separate function
> - Update help text of Kconfig BLOCK entries
> - Add warning at init if MMC was selected with BLOCK backend
> - Link to v2: https://lore.kernel.org/r/20250409-topic-fastboot-blk-v2-0-c676f21d414f@linaro.org
>
> Changes in v2:
> - Dropped applied virtio erase patch
> - Reorganize patches, introducing helpers first, using them in mmc afterwards
> - Added soft-erase logic
> - Added move helpers to handle the partitions erase & flash from emmc
> - Fixed const var on last patch
> - Link to v1: https://lore.kernel.org/all/20240306185921.1854109-1-dimorinny@google.com/
>
> ---
> Dmitrii Merkurev (3):
>       fastboot: blk: introduce fastboot block flashing support
>       fastboot: blk: switch emmc to use the block helpers
>       fastboot: integrate block flashing back-end
>
>  drivers/fastboot/Kconfig      |  28 +++-
>  drivers/fastboot/Makefile     |   4 +-
>  drivers/fastboot/fb_block.c   | 323 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/fastboot/fb_command.c |   8 ++
>  drivers/fastboot/fb_common.c  |  22 ++-
>  drivers/fastboot/fb_getvar.c  |   8 +-
>  drivers/fastboot/fb_mmc.c     | 210 ++-------------------------
>  include/fb_block.h            | 105 ++++++++++++++
>  8 files changed, 502 insertions(+), 206 deletions(-)
> ---
> base-commit: 4d3b5c679bc9d5c6cbbeedcc1e4a186f1cc35541
> change-id: 20250408-topic-fastboot-blk-c5e14cd59224
>
> Best regards,
> -- 
> Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH RFT v3 1/3] fastboot: blk: introduce fastboot block flashing support

[Mattijs Korpershoek]

Hi Neil,

Thank you for the patch.

On mar., mai 06, 2025 at 18:10, neil.armstrong@linaro.org wrote:

> From: Dmitrii Merkurev <dimorinny@google.com>
>
> Introduce fastboot block flashing functions and helpers
> to be shared with the MMC implementation.
>
> The write logic comes from the mmc implementation, while
> the partition lookup is much simpler and could be extended.
>
> For the erase logic, allmost no block drivers exposes the
> erase operation, except mmc & virtio, so in order to allow
> erasiong any partition a soft-erase logic has been added

s/erasiong/erasing

I can fixup when applying.

> to write zero-ed buffers in a loop.
>
> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

> ---

Re: [PATCH RFT v3 2/3] fastboot: blk: switch emmc to use the block helpers

[Mattijs Korpershoek]

Hi Neil,

Thank you for the patch.

On mar., mai 06, 2025 at 18:10, neil.armstrong@linaro.org wrote:

> From: Dmitrii Merkurev <dimorinny@google.com>
>
> Switch the mmc backend to this new shared block helpers,
> reducing block logic and only leaving MMC specific logic.
>
> Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

> ---
>  drivers/fastboot/Kconfig  |   4 +-
>  drivers/fastboot/Makefile |   3 +-
>  drivers/fastboot/fb_mmc.c | 210 +++-------------------------------------------
>  3 files changed, 16 insertions(+), 201 deletions(-)
>

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Neil Armstrong]

On 07/05/2025 12:02, Mattijs Korpershoek wrote:
> Hi Neil,
> 
> Thank you for the series.
> 
> On mar., mai 06, 2025 at 18:10, neil.armstrong@linaro.org wrote:
> 
>> This serie permits using any block device as target
>> for fastboot by moving the generic block logic into
>> a common set of helpers and also use them as generic
>> backend.
>>
>> The erase logic has been extended to support software
>> erase since only 2 block drivers exposes the erase
>> operation.
>>
>> Tests are welcome to make sure this series doesn't
>> introduce any regressions on the emmc backend.
> 
> I've tested the series on khadas-vim3_android_defconfig
> keeping CONFIG_FASTBOOT_FLASH_MMC=y (so with the emmc backend)
> 
> I tested the following:
> # flashing a raw partition with a descriptor:
> $ fastboot flash bootloader u-boot_kvim3_noab.bin
> # flashing a sparse image:
> $ fastboot flash super super.img
> 
> I also testing reflashing the super.img when using the generic block
> layer:
> => fastboot usb 0
> Warning: the fastboot block backend features are limited, consider using the MMC backend
> 
> $ fastboot flash super super.img
> 
> Both work great!
> 
> Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Great thanks for testing !!

> 
> I've also opened a issue on gitlab to harmonize the fastboot emmc
> backend with the generic block backend:
> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/issues/6

Nice !

> 
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> Changes in v3:
>> - Move Kconfig/Makefile changes over the 2 patches
>> - Relicence to GPL2 with Dmitrii approval
>> - Move soft erase in a separate function
>> - Update help text of Kconfig BLOCK entries
>> - Add warning at init if MMC was selected with BLOCK backend
>> - Link to v2: https://lore.kernel.org/r/20250409-topic-fastboot-blk-v2-0-c676f21d414f@linaro.org
>>
>> Changes in v2:
>> - Dropped applied virtio erase patch
>> - Reorganize patches, introducing helpers first, using them in mmc afterwards
>> - Added soft-erase logic
>> - Added move helpers to handle the partitions erase & flash from emmc
>> - Fixed const var on last patch
>> - Link to v1: https://lore.kernel.org/all/20240306185921.1854109-1-dimorinny@google.com/
>>
>> ---
>> Dmitrii Merkurev (3):
>>        fastboot: blk: introduce fastboot block flashing support
>>        fastboot: blk: switch emmc to use the block helpers
>>        fastboot: integrate block flashing back-end
>>
>>   drivers/fastboot/Kconfig      |  28 +++-
>>   drivers/fastboot/Makefile     |   4 +-
>>   drivers/fastboot/fb_block.c   | 323 ++++++++++++++++++++++++++++++++++++++++++
>>   drivers/fastboot/fb_command.c |   8 ++
>>   drivers/fastboot/fb_common.c  |  22 ++-
>>   drivers/fastboot/fb_getvar.c  |   8 +-
>>   drivers/fastboot/fb_mmc.c     | 210 ++-------------------------
>>   include/fb_block.h            | 105 ++++++++++++++
>>   8 files changed, 502 insertions(+), 206 deletions(-)
>> ---
>> base-commit: 4d3b5c679bc9d5c6cbbeedcc1e4a186f1cc35541
>> change-id: 20250408-topic-fastboot-blk-c5e14cd59224
>>
>> Best regards,
>> -- 
>> Neil Armstrong <neil.armstrong@linaro.org>

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Mattijs Korpershoek]

Hi,

On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
> This serie permits using any block device as target
> for fastboot by moving the generic block logic into
> a common set of helpers and also use them as generic
> backend.
> 
> The erase logic has been extended to support software
> erase since only 2 block drivers exposes the erase
> operation.
> 
> [...]

Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)

[1/3] fastboot: blk: introduce fastboot block flashing support
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
[2/3] fastboot: blk: switch emmc to use the block helpers
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
[3/3] fastboot: integrate block flashing back-end
      https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb

--
Mattijs

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Mattijs Korpershoek]

Hi Neil,

On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:

> Hi,
>
> On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
>> This serie permits using any block device as target
>> for fastboot by moving the generic block logic into
>> a common set of helpers and also use them as generic
>> backend.
>> 
>> The erase logic has been extended to support software
>> erase since only 2 block drivers exposes the erase
>> operation.
>> 
>> [...]
>
> Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>
> [1/3] fastboot: blk: introduce fastboot block flashing support
>       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
> [2/3] fastboot: blk: switch emmc to use the block helpers
>       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
> [3/3] fastboot: integrate block flashing back-end
>       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb

It seems this series cause CI to fail:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238

Without the patches applied, it passes:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235

Do you have any idea what is going wrong?
I could not find anything obvious by skimming through the logs.

>
> --
> Mattijs

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
> Hi Neil,
> 
> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> 
> > Hi,
> >
> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
> >> This serie permits using any block device as target
> >> for fastboot by moving the generic block logic into
> >> a common set of helpers and also use them as generic
> >> backend.
> >> 
> >> The erase logic has been extended to support software
> >> erase since only 2 block drivers exposes the erase
> >> operation.
> >> 
> >> [...]
> >
> > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
> >
> > [1/3] fastboot: blk: introduce fastboot block flashing support
> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
> > [2/3] fastboot: blk: switch emmc to use the block helpers
> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
> > [3/3] fastboot: integrate block flashing back-end
> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
> 
> It seems this series cause CI to fail:
> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
> 
> Without the patches applied, it passes:
> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
> 
> Do you have any idea what is going wrong?
> I could not find anything obvious by skimming through the logs.

It's a Kconfig problem then. Some platform is prompting for a value (not
a y/n) and there's no default.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Mattijs Korpershoek]

Hi Tom,

On mer., mai 21, 2025 at 09:12, Tom Rini <trini@konsulko.com> wrote:

> On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
>> Hi Neil,
>> 
>> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> 
>> > Hi,
>> >
>> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
>> >> This serie permits using any block device as target
>> >> for fastboot by moving the generic block logic into
>> >> a common set of helpers and also use them as generic
>> >> backend.
>> >> 
>> >> The erase logic has been extended to support software
>> >> erase since only 2 block drivers exposes the erase
>> >> operation.
>> >> 
>> >> [...]
>> >
>> > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>> >
>> > [1/3] fastboot: blk: introduce fastboot block flashing support
>> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
>> > [2/3] fastboot: blk: switch emmc to use the block helpers
>> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
>> > [3/3] fastboot: integrate block flashing back-end
>> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
>> 
>> It seems this series cause CI to fail:
>> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
>> 
>> Without the patches applied, it passes:
>> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
>> 
>> Do you have any idea what is going wrong?
>> I could not find anything obvious by skimming through the logs.
>
> It's a Kconfig problem then. Some platform is prompting for a value (not
> a y/n) and there's no default.

You are correct. Thank you for the suggestion.

I've applied the following diff to 3/3:
diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
index 68967abb751e..fdf34a6abe1e 100644
--- a/drivers/fastboot/Kconfig
+++ b/drivers/fastboot/Kconfig
@@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
 config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
        string "Define FASTBOOT block interface name"
        depends on FASTBOOT_FLASH_BLOCK
+       default "none"
        help
          The fastboot "flash" and "erase" commands support operations
          on any Block device, this should specify the block device name
@@ -212,6 +213,7 @@ config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
 config FASTBOOT_FLASH_BLOCK_DEVICE_ID
        int "Define FASTBOOT block device identifier"
        depends on FASTBOOT_FLASH_BLOCK
+       default 0
        help
          The fastboot "flash" and "erase" commands support operations
          on any Block device, this should specify the block device

See:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/e9e185a349fc1dd5066c9b79758b0434e3385003

And it builds successfully:
https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26268

Neil, can you look if the above diff is good for you?
If so, I can squash this in.

If you prefer, you can resend a v4 with the fix included and I can
re-merge that instead.

Let me know what works for you.

>
> -- 
> Tom

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3532 bytes --]

On Wed, May 21, 2025 at 08:52:41PM +0200, Mattijs Korpershoek wrote:
> Hi Tom,
> 
> On mer., mai 21, 2025 at 09:12, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
> >> Hi Neil,
> >> 
> >> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> >> 
> >> > Hi,
> >> >
> >> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
> >> >> This serie permits using any block device as target
> >> >> for fastboot by moving the generic block logic into
> >> >> a common set of helpers and also use them as generic
> >> >> backend.
> >> >> 
> >> >> The erase logic has been extended to support software
> >> >> erase since only 2 block drivers exposes the erase
> >> >> operation.
> >> >> 
> >> >> [...]
> >> >
> >> > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
> >> >
> >> > [1/3] fastboot: blk: introduce fastboot block flashing support
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
> >> > [2/3] fastboot: blk: switch emmc to use the block helpers
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
> >> > [3/3] fastboot: integrate block flashing back-end
> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
> >> 
> >> It seems this series cause CI to fail:
> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
> >> 
> >> Without the patches applied, it passes:
> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
> >> 
> >> Do you have any idea what is going wrong?
> >> I could not find anything obvious by skimming through the logs.
> >
> > It's a Kconfig problem then. Some platform is prompting for a value (not
> > a y/n) and there's no default.
> 
> You are correct. Thank you for the suggestion.
> 
> I've applied the following diff to 3/3:
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 68967abb751e..fdf34a6abe1e 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
>  config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>         string "Define FASTBOOT block interface name"
>         depends on FASTBOOT_FLASH_BLOCK
> +       default "none"
>         help
>           The fastboot "flash" and "erase" commands support operations
>           on any Block device, this should specify the block device name

Assuming that the code will see "none" and handle the error correctly,
OK. But we really should have a configured true value here, yes?

> @@ -212,6 +213,7 @@ config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>  config FASTBOOT_FLASH_BLOCK_DEVICE_ID
>         int "Define FASTBOOT block device identifier"
>         depends on FASTBOOT_FLASH_BLOCK
> +       default 0
>         help
>           The fastboot "flash" and "erase" commands support operations
>           on any Block device, this should specify the block device

I do not like this at first. If FASTBOOT_FLASH_BLOCK_DEVICE_ID is set,
there should be a valid ID set too yes? Potentially worse, is 0 a valid
option here? If so, is that likely to be a real and common one? In that
case, we should also be updating the help text to make sure it's clear
what the normal range is I think.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Mattijs Korpershoek]

Hi Tom,

On mer., mai 21, 2025 at 13:03, Tom Rini <trini@konsulko.com> wrote:

> On Wed, May 21, 2025 at 08:52:41PM +0200, Mattijs Korpershoek wrote:
>> Hi Tom,
>> 
>> On mer., mai 21, 2025 at 09:12, Tom Rini <trini@konsulko.com> wrote:
>> 
>> > On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
>> >> Hi Neil,
>> >> 
>> >> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>> >> 
>> >> > Hi,
>> >> >
>> >> > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
>> >> >> This serie permits using any block device as target
>> >> >> for fastboot by moving the generic block logic into
>> >> >> a common set of helpers and also use them as generic
>> >> >> backend.
>> >> >> 
>> >> >> The erase logic has been extended to support software
>> >> >> erase since only 2 block drivers exposes the erase
>> >> >> operation.
>> >> >> 
>> >> >> [...]
>> >> >
>> >> > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>> >> >
>> >> > [1/3] fastboot: blk: introduce fastboot block flashing support
>> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
>> >> > [2/3] fastboot: blk: switch emmc to use the block helpers
>> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
>> >> > [3/3] fastboot: integrate block flashing back-end
>> >> >       https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
>> >> 
>> >> It seems this series cause CI to fail:
>> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
>> >> 
>> >> Without the patches applied, it passes:
>> >> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
>> >> 
>> >> Do you have any idea what is going wrong?
>> >> I could not find anything obvious by skimming through the logs.
>> >
>> > It's a Kconfig problem then. Some platform is prompting for a value (not
>> > a y/n) and there's no default.
>> 
>> You are correct. Thank you for the suggestion.
>> 
>> I've applied the following diff to 3/3:
>> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>> index 68967abb751e..fdf34a6abe1e 100644
>> --- a/drivers/fastboot/Kconfig
>> +++ b/drivers/fastboot/Kconfig
>> @@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
>>  config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>>         string "Define FASTBOOT block interface name"
>>         depends on FASTBOOT_FLASH_BLOCK
>> +       default "none"
>>         help
>>           The fastboot "flash" and "erase" commands support operations
>>           on any Block device, this should specify the block device name
>
> Assuming that the code will see "none" and handle the error correctly,
> OK. But we really should have a configured true value here, yes?

The code does not handle any special cases.
If FASTBOOT_FLASH_BLOCK_INTERFACE_NAME is "none", we will call:

blk_get_dev("none", 0);

Which will then be handled via:

	if (!dev_desc) {
		fastboot_fail("no such device", response);
		return -ENODEV;
	}

>
>> @@ -212,6 +213,7 @@ config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>>  config FASTBOOT_FLASH_BLOCK_DEVICE_ID
>>         int "Define FASTBOOT block device identifier"
>>         depends on FASTBOOT_FLASH_BLOCK
>> +       default 0
>>         help
>>           The fastboot "flash" and "erase" commands support operations
>>           on any Block device, this should specify the block device
>
> I do not like this at first. If FASTBOOT_FLASH_BLOCK_DEVICE_ID is set,
> there should be a valid ID set too yes? Potentially worse, is 0 a valid
> option here? If so, is that likely to be a real and common one? In that

Yes, 0 is a valid option here. Think of this symbol as a similar one to
FASTBOOT_FLASH_MMC_DEV however it's for generic block device type
(virtio, scsi, ...)

I'd think that 0 is typically the most common value, since it's the
first block controller of a specific type.

> case, we should also be updating the help text to make sure it's clear
> what the normal range is I think.

Ok. That's a bit too much for me to do in a fixup.

Neil, can you send a v4?

>
> -- 
> Tom

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[neil.armstrong]

On 22/05/2025 08:58, Mattijs Korpershoek wrote:
> Hi Tom,
> 
> On mer., mai 21, 2025 at 13:03, Tom Rini <trini@konsulko.com> wrote:
> 
>> On Wed, May 21, 2025 at 08:52:41PM +0200, Mattijs Korpershoek wrote:
>>> Hi Tom,
>>>
>>> On mer., mai 21, 2025 at 09:12, Tom Rini <trini@konsulko.com> wrote:
>>>
>>>> On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
>>>>> Hi Neil,
>>>>>
>>>>> On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
>>>>>>> This serie permits using any block device as target
>>>>>>> for fastboot by moving the generic block logic into
>>>>>>> a common set of helpers and also use them as generic
>>>>>>> backend.
>>>>>>>
>>>>>>> The erase logic has been extended to support software
>>>>>>> erase since only 2 block drivers exposes the erase
>>>>>>> operation.
>>>>>>>
>>>>>>> [...]
>>>>>>
>>>>>> Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
>>>>>>
>>>>>> [1/3] fastboot: blk: introduce fastboot block flashing support
>>>>>>        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
>>>>>> [2/3] fastboot: blk: switch emmc to use the block helpers
>>>>>>        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
>>>>>> [3/3] fastboot: integrate block flashing back-end
>>>>>>        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
>>>>>
>>>>> It seems this series cause CI to fail:
>>>>> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
>>>>>
>>>>> Without the patches applied, it passes:
>>>>> https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
>>>>>
>>>>> Do you have any idea what is going wrong?
>>>>> I could not find anything obvious by skimming through the logs.
>>>>
>>>> It's a Kconfig problem then. Some platform is prompting for a value (not
>>>> a y/n) and there's no default.
>>>
>>> You are correct. Thank you for the suggestion.
>>>
>>> I've applied the following diff to 3/3:
>>> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
>>> index 68967abb751e..fdf34a6abe1e 100644
>>> --- a/drivers/fastboot/Kconfig
>>> +++ b/drivers/fastboot/Kconfig
>>> @@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
>>>   config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>>>          string "Define FASTBOOT block interface name"
>>>          depends on FASTBOOT_FLASH_BLOCK
>>> +       default "none"
>>>          help
>>>            The fastboot "flash" and "erase" commands support operations
>>>            on any Block device, this should specify the block device name
>>
>> Assuming that the code will see "none" and handle the error correctly,
>> OK. But we really should have a configured true value here, yes?
> 
> The code does not handle any special cases.
> If FASTBOOT_FLASH_BLOCK_INTERFACE_NAME is "none", we will call:
> 
> blk_get_dev("none", 0);
> 
> Which will then be handled via:
> 
> 	if (!dev_desc) {
> 		fastboot_fail("no such device", response);
> 		return -ENODEV;
> 	}

It could be "" aswell ?

> 
>>
>>> @@ -212,6 +213,7 @@ config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
>>>   config FASTBOOT_FLASH_BLOCK_DEVICE_ID
>>>          int "Define FASTBOOT block device identifier"
>>>          depends on FASTBOOT_FLASH_BLOCK
>>> +       default 0
>>>          help
>>>            The fastboot "flash" and "erase" commands support operations
>>>            on any Block device, this should specify the block device
>>
>> I do not like this at first. If FASTBOOT_FLASH_BLOCK_DEVICE_ID is set,
>> there should be a valid ID set too yes? Potentially worse, is 0 a valid
>> option here? If so, is that likely to be a real and common one? In that
> 
> Yes, 0 is a valid option here. Think of this symbol as a similar one to
> FASTBOOT_FLASH_MMC_DEV however it's for generic block device type
> (virtio, scsi, ...)
> 
> I'd think that 0 is typically the most common value, since it's the
> first block controller of a specific type.

Yes 0 is a sentible value

> 
>> case, we should also be updating the help text to make sure it's clear
>> what the normal range is I think.
> 
> Ok. That's a bit too much for me to do in a fixup.
> 
> Neil, can you send a v4?


Sure

Thanks,
Neil

> 
>>
>> -- 
>> Tom

Re: [PATCH RFT v3 0/3] fastboot: add support for generic block flashing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4287 bytes --]

On Thu, May 22, 2025 at 09:52:30AM +0200, neil.armstrong@linaro.org wrote:
> On 22/05/2025 08:58, Mattijs Korpershoek wrote:
> > Hi Tom,
> > 
> > On mer., mai 21, 2025 at 13:03, Tom Rini <trini@konsulko.com> wrote:
> > 
> > > On Wed, May 21, 2025 at 08:52:41PM +0200, Mattijs Korpershoek wrote:
> > > > Hi Tom,
> > > > 
> > > > On mer., mai 21, 2025 at 09:12, Tom Rini <trini@konsulko.com> wrote:
> > > > 
> > > > > On Wed, May 21, 2025 at 04:49:35PM +0200, Mattijs Korpershoek wrote:
> > > > > > Hi Neil,
> > > > > > 
> > > > > > On mar., mai 20, 2025 at 13:35, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:
> > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > On Tue, 06 May 2025 18:10:06 +0200, neil.armstrong@linaro.org wrote:
> > > > > > > > This serie permits using any block device as target
> > > > > > > > for fastboot by moving the generic block logic into
> > > > > > > > a common set of helpers and also use them as generic
> > > > > > > > backend.
> > > > > > > > 
> > > > > > > > The erase logic has been extended to support software
> > > > > > > > erase since only 2 block drivers exposes the erase
> > > > > > > > operation.
> > > > > > > > 
> > > > > > > > [...]
> > > > > > > 
> > > > > > > Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)
> > > > > > > 
> > > > > > > [1/3] fastboot: blk: introduce fastboot block flashing support
> > > > > > >        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/88239b5bb04bea2b58f7bf4c3ea72cf832de818c
> > > > > > > [2/3] fastboot: blk: switch emmc to use the block helpers
> > > > > > >        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/25ab5c32c28b9f25fb193f726f239d75af3c365a
> > > > > > > [3/3] fastboot: integrate block flashing back-end
> > > > > > >        https://source.denx.de/u-boot/custodians/u-boot-dfu/-/commit/a885bd8c969e25d03bf406207d89b1145c9490fb
> > > > > > 
> > > > > > It seems this series cause CI to fail:
> > > > > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26238
> > > > > > 
> > > > > > Without the patches applied, it passes:
> > > > > > https://source.denx.de/u-boot/custodians/u-boot-dfu/-/pipelines/26235
> > > > > > 
> > > > > > Do you have any idea what is going wrong?
> > > > > > I could not find anything obvious by skimming through the logs.
> > > > > 
> > > > > It's a Kconfig problem then. Some platform is prompting for a value (not
> > > > > a y/n) and there's no default.
> > > > 
> > > > You are correct. Thank you for the suggestion.
> > > > 
> > > > I've applied the following diff to 3/3:
> > > > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> > > > index 68967abb751e..fdf34a6abe1e 100644
> > > > --- a/drivers/fastboot/Kconfig
> > > > +++ b/drivers/fastboot/Kconfig
> > > > @@ -200,6 +200,7 @@ config FASTBOOT_MMC_USER_NAME
> > > >   config FASTBOOT_FLASH_BLOCK_INTERFACE_NAME
> > > >          string "Define FASTBOOT block interface name"
> > > >          depends on FASTBOOT_FLASH_BLOCK
> > > > +       default "none"
> > > >          help
> > > >            The fastboot "flash" and "erase" commands support operations
> > > >            on any Block device, this should specify the block device name
> > > 
> > > Assuming that the code will see "none" and handle the error correctly,
> > > OK. But we really should have a configured true value here, yes?
> > 
> > The code does not handle any special cases.
> > If FASTBOOT_FLASH_BLOCK_INTERFACE_NAME is "none", we will call:
> > 
> > blk_get_dev("none", 0);
> > 
> > Which will then be handled via:
> > 
> > 	if (!dev_desc) {
> > 		fastboot_fail("no such device", response);
> > 		return -ENODEV;
> > 	}
> 
> It could be "" aswell ?

I really do not like putting in invalid defaults in order to allow "make
oldconfig" and so forth to not get stuck. If a board needs to set a
value to compile, it needs to put in a valid value. Putting in unusable
defaults to Kconfig leads to runtime problems later (this is more clear
with "" and *much* less clear with a maybe-valid 0). My preference here
would be to update whatever the stuck platform is to either disable the
feature or set correct values.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]