[PATCH] drivers: fastboot: Enable flashing support for UFS

[PATCH] drivers: fastboot: Enable flashing support for UFS

[Varadarajan Narayanan]

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.

Tested on rb3gen2 platform.

Signed-off-by: Varadarajan Narayanan <quic_varada@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.
+
 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"
+#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);
 		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);
 }
-- 
2.34.1

Re: [PATCH] drivers: fastboot: Enable flashing support for UFS

[Caleb Connolly]

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@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)

Re: [PATCH] drivers: fastboot: Enable flashing support for UFS

[Varadarajan Narayanan]

On Mon, Nov 25, 2024 at 06:59:40PM +0100, Caleb Connolly wrote:
> 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@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.

Yes. Took this approach as this would easily align with the MMC's
way of searching for a partition. Will fix this.

> > +
> >  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.

Considered that approach but didn't follow through since it might
break the existing command syntax. Should we extend the 'flash'
command itself or should we add an 'oem' command to select mmc or
ufs.

> > +#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.

Not sure if this will be applicable if we move to dynamic mmc or
ufs selection. Will see.

> >  		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.

Sure.

Will post a new version shortly.

Thanks for the feedback.

-Varada

Re: [PATCH] drivers: fastboot: Enable flashing support for UFS

[Mattijs Korpershoek]

Hi Varadarajan,

Thank you for the patch.

On mar., nov. 26, 2024 at 10:46, Varadarajan Narayanan <quic_varada@quicinc.com> wrote:

> On Mon, Nov 25, 2024 at 06:59:40PM +0100, Caleb Connolly wrote:
>> 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@quicinc.com>

Seems there is a bit of an overlap with Dmitrii's work here:

https://patchwork.ozlabs.org/project/uboot/patch/20240306185921.1854109-2-dimorinny@google.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

Remove CONFIG_*
Should be:

	default 0 if 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.
>
> Yes. Took this approach as this would easily align with the MMC's
> way of searching for a partition. Will fix this.

I agree with Caleb's point.

>
>> > +
>> >  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.

I agree with Caleb's point.

Also, if this has to stay at compile time, I prefer Dmitrii's approach
using FASTBOOT_FLASH_BLOCK_INTERFACE_NAME.

See: https://patchwork.ozlabs.org/project/uboot/patch/20240306185921.1854109-2-dimorinny@google.com/

>>
>> 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.
>
> Considered that approach but didn't follow through since it might
> break the existing command syntax. Should we extend the 'flash'
> command itself or should we add an 'oem' command to select mmc or
> ufs.

I don't think we should extend the 'flash' command since the fastboot
documentation states:

flash %s           Flash a given partition. Optional arguments include
                   --slot-other, {filename_path}, --apply-vbmeta

See:
https://android.googlesource.com/platform/system/core/+/main/fastboot/README.md#flashing-logic

If this is made dynamically (which seems like a good idea to me), then
I'd prefer this to be an oem command.

>
>> > +#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

With the current implementation, if both CONFIG_FASTBOOT_FLASH_MMC and
CONFIG_FASTBOOT_FLASH_UFS are enabled, we cannot use UFS because MMC
takes priority.

>> > +
>> >  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.
>
> Not sure if this will be applicable if we move to dynamic mmc or
> ufs selection. Will see.

I agree with Caleb's comment.

>
>> >  		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.
>
> Sure.

I agree with Caleb's comment.

>
> Will post a new version shortly.
>
> Thanks for the feedback.
>
> -Varada