[v2] cmd: mmc: add mmcboot command

Similar to usbboot, add command line to boot from raw mmc partition
using common_diskboot(), which supports legacy or FIT images.

Usage:
mmcboot [loadAaddr] [dev:part]

Where [loadAddr] defaults to CONFIG_SYS_LOAD_ADDR, and [dev:part]
defaults to ${bootdevice}.

Also fixing config macro usage for CONFIG_SUPPORT_EMMC_BOOT (suggested
by checkpatch.pl).

Signed-off-by: Ravik Hasija <rahasij@linux.microsoft.com>
Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
v2:
  * Fixed build failure.
  * Updated syntax of mmcboot cmd.
---
 cmd/Makefile |  2 +-
 cmd/mmc.c    | 22 +++++++++++++++++++---
 2 files changed, 20 insertions(+), 4 deletions(-)

On 1/4/21 7:40 PM, Ravik Hasija wrote:
> Similar to usbboot, add command line to boot from raw mmc partition
> using common_diskboot(), which supports legacy or FIT images.
> 
> Usage:
> mmcboot [loadAaddr] [dev:part]
> 
> Where [loadAddr] defaults to CONFIG_SYS_LOAD_ADDR, and [dev:part]
> defaults to ${bootdevice}.
> 
> Also fixing config macro usage for CONFIG_SUPPORT_EMMC_BOOT (suggested
> by checkpatch.pl).

Can this not be implemented by some U-Boot script ?
I can imagine that reading the first few bytes from the partition (mmc 
read) would permit you to obtain the uImage/fitImage header from the 
partition, from which you can obtain the image size, which is the total 
number of bytes to read (iminfo might help , or fdt header), and then 
read out the entire image and bootm it.

On 1/5/21 3:40 AM, Ravik Hasija wrote:
> Similar to usbboot, add command line to boot from raw mmc partition
> using common_diskboot(), which supports legacy or FIT images.
> 
> Usage:
> mmcboot [loadAaddr] [dev:part]
> 
> Where [loadAddr] defaults to CONFIG_SYS_LOAD_ADDR, and [dev:part]
> defaults to ${bootdevice}.
> 
> Also fixing config macro usage for CONFIG_SUPPORT_EMMC_BOOT (suggested
> by checkpatch.pl).
> 
> Signed-off-by: Ravik Hasija <rahasij@linux.microsoft.com>
> Signed-off-by: Dhananjay Phadke <dphadke@linux.microsoft.com>
> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> v2:
>   * Fixed build failure.
>   * Updated syntax of mmcboot cmd.
> ---
>  cmd/Makefile |  2 +-
>  cmd/mmc.c    | 22 +++++++++++++++++++---
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/Makefile b/cmd/Makefile
> index dd86675bf2..573e4776f5 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -97,7 +97,7 @@ obj-$(CONFIG_CMD_MII) += mii.o
>  obj-$(CONFIG_CMD_MISC) += misc.o
>  obj-$(CONFIG_CMD_MDIO) += mdio.o
>  obj-$(CONFIG_CMD_SLEEP) += sleep.o
> -obj-$(CONFIG_CMD_MMC) += mmc.o
> +obj-$(CONFIG_CMD_MMC) += mmc.o disk.o

I think that it's not good about building disk.c by default when CONFIG_CMD_MMC is enabled.

Best Regards,
Jaehoon Chung

>  obj-$(CONFIG_CMD_OPTEE_RPMB) += optee_rpmb.o
>  obj-$(CONFIG_MP) += mp.o
>  obj-$(CONFIG_CMD_MTD) += mtd.o
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 1529a3e05d..0c3ed6c108 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -711,7 +711,7 @@ static int do_mmc_hwpartition(struct cmd_tbl *cmdtp, int flag,
>  }
>  #endif
>  
> -#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +#if CONFIG_IS_ENABLED(SUPPORT_EMMC_BOOT)
>  static int do_mmc_bootbus(struct cmd_tbl *cmdtp, int flag,
>  			  int argc, char *const argv[])
>  {
> @@ -950,7 +950,7 @@ static struct cmd_tbl cmd_mmc[] = {
>  #if CONFIG_IS_ENABLED(MMC_HW_PARTITIONING)
>  	U_BOOT_CMD_MKENT(hwpartition, 28, 0, do_mmc_hwpartition, "", ""),
>  #endif
> -#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +#if CONFIG_IS_ENABLED(SUPPORT_EMMC_BOOT)
>  	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
>  	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
>  	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
> @@ -992,6 +992,14 @@ static int do_mmcops(struct cmd_tbl *cmdtp, int flag, int argc,
>  	return cp->cmd(cmdtp, flag, argc, argv);
>  }
>  
> +#if CONFIG_IS_ENABLED(SUPPORT_EMMC_BOOT)
> +static int do_mmcboot(struct cmd_tbl *cmdtp, int flag, int argc,
> +		char *const argv[])
> +{
> +	return common_diskboot(cmdtp, "mmc", argc, argv);
> +}
> +#endif
> +
>  U_BOOT_CMD(
>  	mmc, 29, 1, do_mmcops,
>  	"MMC sub system",
> @@ -1016,7 +1024,7 @@ U_BOOT_CMD(
>  	"  WARNING: Partitioning is a write-once setting once it is set to complete.\n"
>  	"  Power cycling is required to initialize partitions after set to complete.\n"
>  #endif
> -#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +#if CONFIG_IS_ENABLED(SUPPORT_EMMC_BOOT)
>  	"mmc bootbus dev boot_bus_width reset_boot_bus_width boot_mode\n"
>  	" - Set the BOOT_BUS_WIDTH field of the specified device\n"
>  	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
> @@ -1046,3 +1054,11 @@ U_BOOT_CMD(
>  	"display MMC info",
>  	"- display info of the current MMC device"
>  );
> +
> +#if CONFIG_IS_ENABLED(SUPPORT_EMMC_BOOT)
> +U_BOOT_CMD(
> +	mmcboot, 3, 1, do_mmcboot,
> +	"boot from eMMC",
> +	"[loadAddr] [dev:part]"
> +);
> +#endif
>

> Can this not be implemented by some U-Boot script ?
> I can imagine that reading the first few bytes from the partition (mmc
> read) would permit you to obtain the uImage/fitImage header from the
> partition, from which you can obtain the image size, which is the total
> number of bytes to read (iminfo might help , or fdt header), and then
> read out the entire image and bootm it.

Using this command provides the advantage to reduce code replication &
maintainence by leveraging existing APIs (common_diskboot) which
functionally does the same thing as a uboot script would do read one block,
get size from header, read whole image, and bootm it, while additionally
doing basic checks.



--
Sent from: http://u-boot.10912.n7.nabble.com/

> I think that it's not good about building disk.c by default when
CONFIG_CMD_MMC is enabled.

Agree, its not an optimal solution. However, we chose to do it this way
because:
- It causes minimal change to the size of binary (~200 bytes). 
- To be consistent with upstream code where it is built similarly for
usb/scsi etc.



--
Sent from: http://u-boot.10912.n7.nabble.com/