[PATCH 07/13] smegw01: Enable EMMC boot from multiple partitions

[Pali Rohár]

On Tuesday 18 April 2023 10:47:38 Fabio Estevam wrote:
> From: Eduard Strehlau <eduard at lionizers.com>
> 
> GPT Partition labels are used for determining the right
> root filesystem to boot from.
> 
> The U-Boot environment is configured to reside in the eMMC hardware
> boot partition we are currently booted from.
> 
> This should enable a dual copy approach for upgrading the bootloader.
> One can overwrite the inactive hardware partition with new bootloader
> and environment and afterwards switch the eMMC boot partition for an
> atomic bootloader switch.
> 
> Signed-off-by: Eduard Strehlau <eduard at lionizers.com>
> Signed-off-by: Fabio Estevam <festevam at denx.de>
> ---
>  board/storopack/smegw01/smegw01.c | 20 ++++++++++++++++++++
>  configs/smegw01_defconfig         |  2 ++
>  include/configs/smegw01.h         | 12 ++++++++----
>  3 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/board/storopack/smegw01/smegw01.c b/board/storopack/smegw01/smegw01.c
> index e6bff80e55..9482f88773 100644
> --- a/board/storopack/smegw01/smegw01.c
> +++ b/board/storopack/smegw01/smegw01.c
> @@ -17,6 +17,7 @@
>  #include <asm/arch/crm_regs.h>
>  #include <asm/setup.h>
>  #include <asm/bootm.h>
> +#include <mmc.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -93,3 +94,22 @@ int board_late_init(void)
>  
>  	return 0;
>  }
> +
> +/* defined in JESD84-B50, PARTITION_CONFIG */
> +#define BOOT_PARTITION_ENABLE_MASK (0x7)
> +#define BOOT_PARTITION_ENABLE_SHIFT (0x3)

Please do not define a new mmc part macros. U-Boot already has them
available, and recently I removed usage of custom macros, see commit:
https://source.denx.de/u-boot/u-boot/-/commit/8b8820669646ceb08d6ceed4181b53042639f3ab

> +uint mmc_get_env_part(struct mmc *mmc)
> +{
> +	uint part = (mmc->part_config >> BOOT_PARTITION_ENABLE_SHIFT) &
> +			BOOT_PARTITION_ENABLE_MASK;
> +	/* Default to Boot Partition 1 if not explicitly set. */
> +	if (part != 1 && part != 2 && part != 7)
> +		part = 1;

I'm not sure if this logic is complete. For example there is missing
case for MMCPART_NOAVAILABLE or what is expected when booting from eMMC
is disabled (value 0x0).

For example, recently I sent this patch for U-Boot bubt command,
hopefully handling all possible options:
https://patchwork.ozlabs.org/project/uboot/patch/20230413211057.10975-3-pali@kernel.org/

> +
> +	/* 7 in BOOT_PARTITION_ENABLE is equivalent to 0 in PARTITION_ACCESS. */
> +	if (part == 7)
> +		part = 0;
> +
> +	return part;
> +}
> diff --git a/configs/smegw01_defconfig b/configs/smegw01_defconfig
> index b3580d5d6e..54cf1cfc1f 100644
> --- a/configs/smegw01_defconfig
> +++ b/configs/smegw01_defconfig
> @@ -30,6 +30,7 @@ CONFIG_CMD_MEMTEST=y
>  CONFIG_CMD_UNZIP=y
>  CONFIG_CMD_DFU=y
>  CONFIG_CMD_GPIO=y
> +CONFIG_CMD_GPT=y
>  CONFIG_CMD_MMC=y
>  CONFIG_CMD_PART=y
>  CONFIG_CMD_DHCP=y
> @@ -44,6 +45,7 @@ CONFIG_OF_CONTROL=y
>  CONFIG_ENV_OVERWRITE=y
>  CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
>  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_SYS_MMC_ENV_DEV=1
>  CONFIG_NET_RANDOM_ETHADDR=y
>  CONFIG_BOUNCE_BUFFER=y
>  CONFIG_BOOTCOUNT_LIMIT=y
> diff --git a/include/configs/smegw01.h b/include/configs/smegw01.h
> index 277c828d0e..71f2d9c8e8 100644
> --- a/include/configs/smegw01.h
> +++ b/include/configs/smegw01.h
> @@ -32,17 +32,21 @@
>  	"mmcpart=1\0" \
>  	"mmcpart_committed=1\0" \
>  	"mmcargs=setenv bootargs console=${console},${baudrate} " \
> -		"root=/dev/mmcblk0p${mmcpart_committed} rootwait rw " \
> -		__stringify(EXTRA_BOOTPARAMS) "\0" \
> +		"root=/dev/mmcblk${mmcdev}p${gpt_partition_entry} rootwait rw " \
> +		__stringify(EXTRA_BOOTPARAMS) " SM_ROOT_DEV=${mmcdev} SM_ROOT_PART=${gpt_partition_entry} SM_BOOT_PART=${boot_part}\0" \
>  	"commit_mmc=if test \"${ustate}\" = 1 -a \"${mmcpart}\" != \"${mmcpart_committed}\"; then " \
>  	              "setenv mmcpart_committed ${mmcpart};" \
>  								"saveenv;" \
>  						  "fi;\0" \
>  	"bootlimit=3\0" \
> -	"loadimage=load mmc ${mmcdev}:${mmcpart_committed} ${loadaddr} boot/${image}\0" \
> -	"loadfdt=load mmc ${mmcdev}:${mmcpart_committed} ${fdt_addr} boot/${fdtfile}\0" \
> +	"loadimage=load mmc ${mmcdev}#rootfs-${mmcpart_committed} ${loadaddr} boot/${image}\0" \
> +	"loadfdt=load mmc ${mmcdev}#rootfs-${mmcpart_committed} ${fdt_addr} boot/${fdtfile}\0" \
> +	"loadpart=gpt setenv mmc ${mmcdev} rootfs-${mmcpart_committed}\0" \
> +	"loadbootpart=mmc partconf 1 boot_part\0" \
>  	"mmcboot=echo Booting from mmc ...; " \
>  	  "run commit_mmc; " \
> +		"run loadpart; " \
> +		"run loadbootpart; " \
>  		"run mmcargs; " \
>  		"if run loadfdt; then " \
>  			"if bootz ${loadaddr} - ${fdt_addr}; then " \
> -- 
> 2.38.1
>