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

2024-04-05 Thread Mattijs Korpershoek
Hi Dmitrii,

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

On mer., mars 06, 2024 at 18:59, Dmitrii Merkurev  wrote:

> Reuse common logic between existing mmc and new introduced
> block implementation
>
> Signed-off-by: Dmitrii Merkurev 
> Cc: Alex Kiernan 
> Cc: Patrick Delaunay 
> Cc: Simon Glass 
> Cc: Mattijs Korpershoek 
> Cc: Ying-Chun Liu (PaulLiu) 

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 00..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * 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,
> +

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

2024-03-06 Thread Dmitrii Merkurev
Reuse common logic between existing mmc and new introduced
block implementation

Signed-off-by: Dmitrii Merkurev 
Cc: Alex Kiernan 
Cc: Patrick Delaunay 
Cc: Simon Glass 
Cc: Mattijs Korpershoek 
Cc: Ying-Chun Liu (PaulLiu) 
---
 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 00..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/**
+ * 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