[PATCH v2] cmd: mmc: add mmc bootpart-check

[Jaehoon Chung]

Hi,

On 2/18/21 1:35 AM, grygorii tertychnyi wrote:
> This patch allows to check the current boot partition. It is useful
> when you use two different sets of (bootloader, kernel) images and
> want to boot the corresponding kernel image in boot script. E.g.:
> 
> if mmc bootpart-check ${mmcdev} 1; then
>     echo "booted from eMMC boot0 partition"
>     ...load kernel image1
> elif mmc bootpart-check ${mmcdev} 2; then
>     echo "booted from eMMC boot1 partition"
>     ...load kernel image2
> fi

If mmc_bootpart-check <dev> <boot_partition> is used, it's just for your bootsripts.
Your patch is doing nothing...

U-boot> mmc bootpart-check 1 1
U-boot> mmc bootpart-check 1 2
U-boot> mmc bootpart-check 1 3

To use this command by other developer, it needs to be more useful than now.
So i think good that displayed the bootpart value in your patch.

Then it doesn't need to use "echo booted from.." in your bootscript.

e.g) if success..
U-boot> mmc bootpart-check 1 1 
Boot partition 1 enabled for boot 

if fail
U-boot> mmc bootpart-check 1 1
Boot partition 2 enabled for boot

Code can be the below..

switch (part_emmc) {
case 0x0:
	printf("Device not boot enabled");
	break;
case 0x1:
	printf("Boot partition 1 ...);
	break;
case 0x2:
	...
default:
	printf("BOOT_PARTITION_ENABLE is reserved\n");
}


Then other developer and you can get more information with this command.
Also when checking is failed, it can provide which bootpart is valid.
And this patch's commit-msg can be described more common, not only bootscript's case.

how about this?

> 
> Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi at leica-geosystems.com>

I think better that Signed-off-by and Author's email address maintains to same.

Author: grygorii tertychnyi <grembeter at gmail.com>
Date:   Wed Feb 17 17:35:16 2021 +0100

    cmd: mmc: add mmc bootpart-check

Best Regards,
Jaehoon Chung

> ---
> v2:
>  - renamed the command, it is "bootpart-check" now
>  - added <> to mandatory arguments help string
>  - added more details to commit message
> 
>  cmd/mmc.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 1529a3e05ddd..3332dee4b130 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -771,6 +771,41 @@ static int do_mmc_boot_resize(struct cmd_tbl *cmdtp, int flag,
>  	return CMD_RET_SUCCESS;
>  }
>  
> +static int do_mmc_boot_check(struct cmd_tbl *cmdtp, int flag,
> +			     int argc, char *const argv[])
> +{
> +	int dev;
> +	struct mmc *mmc;
> +	u8 part_args, part_emmc;
> +
> +	if (argc != 3)
> +		return CMD_RET_USAGE;
> +
> +	dev = simple_strtoul(argv[1], NULL, 10);
> +
> +	mmc = init_mmc_device(dev, false);
> +	if (!mmc)
> +		return CMD_RET_FAILURE;
> +
> +	if (IS_SD(mmc)) {
> +		printf("It is not an eMMC device\n");
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	if (mmc->part_config == MMCPART_NOAVAILABLE) {
> +		printf("No part_config info for ver. 0x%x\n", mmc->version);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	part_emmc = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> +	part_args = simple_strtoul(argv[2], NULL, 10);
> +
> +	if (part_emmc == part_args)
> +		return CMD_RET_SUCCESS;
> +	else
> +		return CMD_RET_FAILURE;
> +}
> +
>  static int mmc_partconf_print(struct mmc *mmc)
>  {
>  	u8 ack, access, part;
> @@ -952,6 +987,7 @@ static struct cmd_tbl cmd_mmc[] = {
>  #endif
>  #ifdef CONFIG_SUPPORT_EMMC_BOOT
>  	U_BOOT_CMD_MKENT(bootbus, 5, 0, do_mmc_bootbus, "", ""),
> +	U_BOOT_CMD_MKENT(bootpart-check, 3, 0, do_mmc_boot_check, "", ""),
>  	U_BOOT_CMD_MKENT(bootpart-resize, 4, 0, do_mmc_boot_resize, "", ""),
>  	U_BOOT_CMD_MKENT(partconf, 5, 0, do_mmc_partconf, "", ""),
>  	U_BOOT_CMD_MKENT(rst-function, 3, 0, do_mmc_rst_func, "", ""),
> @@ -1019,6 +1055,9 @@ U_BOOT_CMD(
>  #ifdef CONFIG_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-check <dev> <boot_partition>\n"
> +	" - Return success if the given boot_partition value matches BOOT_PARTITION_ENABLE\n"
> +	"   bit field of the specified device\n"
>  	"mmc bootpart-resize <dev> <boot part size MB> <RPMB part size MB>\n"
>  	" - Change sizes of boot and RPMB partitions of specified device\n"
>  	"mmc partconf dev [boot_ack boot_partition partition_access]\n"
>