[PATCH] drivers: fastboot: Enable flashing support for UFS
Caleb Connolly
caleb.connolly at linaro.org
Mon Nov 25 18:59:40 CET 2024
Hi Varadarajan,
On 25/11/2024 12:03, Varadarajan Narayanan wrote:
> MMC and UFS seem to execute very similar steps to identify the
> partition and write to it. Hence try to enable fastboot flashing
> support in UFS leveraging the MMC functions.
Thanks for this! Do you think we could make this dynamic at runtime?
Since it's currently possible to build a single binary with
qcom_defconfig that will work across a wide range of boards (both with
eMMC and UFS), I don't think it makes sense to pick a fastboot backend
at build time.
Some additional comments below.
>
> Tested on rb3gen2 platform.
>
> Signed-off-by: Varadarajan Narayanan <quic_varada at quicinc.com>
> ---
> drivers/fastboot/Kconfig | 19 +++++++++++++++---
> drivers/fastboot/Makefile | 1 +
> drivers/fastboot/fb_command.c | 6 ++++--
> drivers/fastboot/fb_mmc.c | 36 ++++++++++++++++++++++++-----------
> 4 files changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 1eb460f5a0..5c41fc4684 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -92,7 +92,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 || UFS || (MTD_RAW_NAND && CMD_MTDPARTS)
> select IMAGE_SPARSE
> help
> The fastboot protocol includes a "flash" command for writing
> @@ -120,6 +120,10 @@ config FASTBOOT_FLASH_NAND
> bool "FASTBOOT on NAND"
> depends on MTD_RAW_NAND && CMD_MTDPARTS
>
> +config FASTBOOT_FLASH_UFS
> + bool "FASTBOOT on UFS"
> + depends on UFS && EFI_PARTITION
> +
> endchoice
>
> config FASTBOOT_FLASH_MMC_DEV
> @@ -133,6 +137,15 @@ config FASTBOOT_FLASH_MMC_DEV
> regarding the non-volatile storage device. Define this to
> the eMMC device that fastboot should use to store the image.
>
> +config FASTBOOT_FLASH_UFS_DEV
> + int "Define FASTBOOT UFS FLASH default device"
> + depends on FASTBOOT_FLASH_UFS
> + default 0 if CONFIG_ARCH_SNAPDRAGON
> + help
> + The fastboot "flash" command requires additional information
> + regarding the non-volatile storage device. Define this to
> + the UFS device that fastboot should use to store the image.
This doesn't map at all to how fastboot is intended to work. We would
expect to see all partitions exposed not just those of a specific LUN.
> +
> config FASTBOOT_FLASH_NAND_TRIMFFS
> bool "Skip empty pages when flashing NAND"
> depends on FASTBOOT_FLASH_NAND
> @@ -196,7 +209,7 @@ config FASTBOOT_MMC_USER_NAME
>
> config FASTBOOT_GPT_NAME
> string "Target name for updating GPT"
> - depends on FASTBOOT_FLASH_MMC && EFI_PARTITION
> + depends on (FASTBOOT_FLASH_MMC || FASTBOOT_FLASH_UFS) && EFI_PARTITION
> default "gpt"
> help
> The fastboot "flash" command supports writing the downloaded
> @@ -209,7 +222,7 @@ config FASTBOOT_GPT_NAME
>
> config FASTBOOT_MBR_NAME
> string "Target name for updating MBR"
> - depends on FASTBOOT_FLASH_MMC && DOS_PARTITION
> + depends on (FASTBOOT_FLASH_MMC || FASTBOOT_FLASH_UFS) && DOS_PARTITION
> default "mbr"
> help
> The fastboot "flash" command allows to write the downloaded image
> diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> index 048af5aa82..772276cb6c 100644
> --- a/drivers/fastboot/Makefile
> +++ b/drivers/fastboot/Makefile
> @@ -4,4 +4,5 @@ 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_UFS) += 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 e4484d65ac..a6fa8299b8 100644
> --- a/drivers/fastboot/fb_command.c
> +++ b/drivers/fastboot/fb_command.c
> @@ -336,7 +336,8 @@ void fastboot_data_complete(char *response)
> */
> static void __maybe_unused flash(char *cmd_parameter, char *response)
> {
> - if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC))
> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC) ||
> + IS_ENABLED(CONFIG_FASTBOOT_FLASH_UFS))
> fastboot_mmc_flash_write(cmd_parameter, fastboot_buf_addr,
> image_size, response);
>
> @@ -356,7 +357,8 @@ 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_MMC))
> + if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_MMC) ||
> + IS_ENABLED(CONFIG_FASTBOOT_FLASH_UFS))
> fastboot_mmc_erase(cmd_parameter, response);
>
> if (IS_ENABLED(CONFIG_FASTBOOT_FLASH_NAND))
> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
> index f11eb66761..bf242e4b2a 100644
> --- a/drivers/fastboot/fb_mmc.c
> +++ b/drivers/fastboot/fb_mmc.c
> @@ -20,6 +20,16 @@
>
> #define BOOT_PARTITION_NAME "boot"
>
> +#if defined(CONFIG_FASTBOOT_FLASH_MMC)
> +#define fb_flash_str "mmc"
Macros be in upper case.
This is something I think would make more sense as a variable. Perhaps
the fastboot command could be extended to allow passing in a device (or
list of devices?) to use.
> +#define fb_flash_dev CONFIG_FASTBOOT_FLASH_MMC_DEV
> +#elif defined(CONFIG_FASTBOOT_FLASH_UFS)
> +#define fb_flash_str "scsi"
> +#define fb_flash_dev CONFIG_FASTBOOT_FLASH_UFS_DEV
> +#else
> +#error "Incorrect flash type"
> +#endif
> +
> struct fb_mmc_sparse {
> struct blk_desc *dev_desc;
> };
> @@ -78,7 +88,7 @@ static int do_get_part_info(struct blk_desc **dev_desc, const char *name,
> int ret;
>
> /* First try partition names on the default device */
> - *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
> + *dev_desc = blk_get_dev(fb_flash_str, fb_flash_dev);
> if (*dev_desc) {
> ret = part_get_info_by_name(*dev_desc, name, info);
> if (ret >= 0)
> @@ -91,8 +101,8 @@ static int do_get_part_info(struct blk_desc **dev_desc, const char *name,
> }
>
> /* Then try dev.hwpart:part */
> - ret = part_get_info_by_dev_and_name_or_num("mmc", name, dev_desc,
> - info, true);
> + ret = part_get_info_by_dev_and_name_or_num(fb_flash_str, name,
> + dev_desc, info, true);
> return ret;
> }
>
> @@ -202,17 +212,20 @@ 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);
> + debug("Start Erasing %s hwpart[%u]...\n",
> + fb_flash_str, 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);
> + pr_err("Failed to erase %s hwpart[%u]\n",
> + fb_flash_str, dev_desc->hwpart);
> return 1;
> }
>
> - printf("........ erased %lu bytes from mmc hwpart[%u]\n",
> - dev_desc->lba * dev_desc->blksz, dev_desc->hwpart);
> + printf("........ erased %lu bytes from %s hwpart[%u]\n",
> + dev_desc->lba * dev_desc->blksz, fb_flash_str,
> + dev_desc->hwpart);
>
> return 0;
> }
> @@ -487,11 +500,10 @@ int fastboot_mmc_get_part_info(const char *part_name,
>
> static struct blk_desc *fastboot_mmc_get_dev(char *response)
> {
> - struct blk_desc *ret = blk_get_dev("mmc",
> - CONFIG_FASTBOOT_FLASH_MMC_DEV);
> + struct blk_desc *ret = blk_get_dev(fb_flash_str, fb_flash_dev);
>
> if (!ret || ret->type == DEV_TYPE_UNKNOWN) {
> - pr_err("invalid mmc device\n");
> + pr_err("invalid %s device\n", fb_flash_str);
pr_err("invalid "fb_flash_str" device\n"); -- and the same for all
other cases.
> fastboot_fail("invalid mmc device", response);
> return NULL;
> }
> @@ -644,10 +656,11 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer,
> */
> void fastboot_mmc_erase(const char *cmd, char *response)
> {
> +#ifdef CONFIG_FASTBOOT_FLASH_MMC
> 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);
> + struct mmc *mmc = find_mmc_device(fb_flash_dev);
>
> #ifdef CONFIG_FASTBOOT_MMC_BOOT_SUPPORT
> if (strcmp(cmd, CONFIG_FASTBOOT_MMC_BOOT1_NAME) == 0) {
> @@ -706,5 +719,6 @@ void fastboot_mmc_erase(const char *cmd, char *response)
>
> printf("........ erased " LBAFU " bytes from '%s'\n",
> blks_size * info.blksz, cmd);
> +#endif
> fastboot_okay(NULL, response);
I don't think it's really acceptable to just lie that we erase a
partition when in fact we did nothing at all.
This should respond with some error indicating that the command isn't
supported.
> }
--
// Caleb (they/them)
More information about the U-Boot
mailing list