Re: [PATCH RFC u-boot-mvebu 20/59] cmd: mvebu/bubt: Add support for selecting eMMC HW partition

[Stefan Roese <sr@denx.de>]

Hi Pali,

On 2/21/23 21:18, Pali Rohár wrote:
> Support for burning into the correct eMMC HW boot partition was broken and
> removed in commit 96be2f072768 ("mvebu: bubt: Drop dead code"). Reimplement
> this functionality and bring it back again.
> 
> Fixes: 96be2f072768 ("mvebu: bubt: Drop dead code")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   cmd/mvebu/bubt.c       | 53 ++++++++++++++++++++++++++++++++++++++----
>   doc/mvebu/cmd/bubt.txt | 21 ++++++++++++-----
>   2 files changed, 63 insertions(+), 11 deletions(-)
> 
> diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
> index 2bcdf145f64a..4bad9a69527c 100644
> --- a/cmd/mvebu/bubt.c
> +++ b/cmd/mvebu/bubt.c
> @@ -189,6 +189,11 @@ static int mmc_burn_image(size_t image_size)
>   #ifdef CONFIG_BLK
>   	struct blk_desc *blk_desc;
>   #endif
> +#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +	u8		part;
> +	u8		orig_part;
> +#endif
> +
>   	mmc = find_mmc_device(mmc_dev_num);
>   	if (!mmc) {
>   		printf("No SD/MMC/eMMC card found\n");
> @@ -202,6 +207,38 @@ static int mmc_burn_image(size_t image_size)
>   		return err;
>   	}
>   
> +#ifdef CONFIG_BLK
> +	blk_desc = mmc_get_blk_desc(mmc);
> +	if (!blk_desc) {
> +		printf("Error - failed to obtain block descriptor\n");
> +		return -ENODEV;
> +	}
> +#endif

This #ifdef usage really gets out of hand IMHO. Yes I know, you did not
introduce it here. Still, perhaps some of this can be moved to using
IS_ENABLED or CONFIG_IS_ENABLED instead?

And I just checked that "bubt" is not compiled for CONFIG_BLK not
defined. So please depend this file on CONFIG_BLK in Kconfig and remove
the non CONFIG_BLK code here.

Thanks,
Stefan

> +
> +#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +#ifdef CONFIG_BLK
> +	orig_part = blk_desc->hwpart;
> +#else
> +	orig_part = mmc->block_dev.hwpart;
> +#endif
> +
> +	part = (mmc->part_config >> 3) & PART_ACCESS_MASK;
> +
> +	if (part == 7)
> +		part = 0;
> +
> +#ifdef CONFIG_BLK
> +	err = blk_dselect_hwpart(blk_desc, part);
> +#else
> +	err = mmc_switch_part(mmc, part);
> +#endif
> +
> +	if (err) {
> +		printf("Error - MMC partition switch failed\n");
> +		return err;
> +	}
> +#endif
> +
>   	/* SD reserves LBA-0 for MBR and boots from LBA-1,
>   	 * MMC/eMMC boots from LBA-0
>   	 */
> @@ -211,11 +248,6 @@ static int mmc_burn_image(size_t image_size)
>   	if (image_size % mmc->write_bl_len)
>   		blk_count += 1;
>   
> -	blk_desc = mmc_get_blk_desc(mmc);
> -	if (!blk_desc) {
> -		printf("Error - failed to obtain block descriptor\n");
> -		return -ENODEV;
> -	}
>   	blk_written = blk_dwrite(blk_desc, start_lba, blk_count,
>   				 (void *)get_load_addr());
>   #else
> @@ -227,6 +259,17 @@ static int mmc_burn_image(size_t image_size)
>   						 start_lba, blk_count,
>   						 (void *)get_load_addr());
>   #endif /* CONFIG_BLK */
> +
> +#ifdef CONFIG_SUPPORT_EMMC_BOOT
> +#ifdef CONFIG_BLK
> +	err = blk_dselect_hwpart(blk_desc, orig_part);
> +#else
> +	err = mmc_switch_part(mmc, orig_part);
> +#endif
> +	if (err)
> +		printf("Error - MMC failed to switch back to original partition\n");
> +#endif
> +
>   	if (blk_written != blk_count) {
>   		printf("Error - written %#lx blocks\n", blk_written);
>   		return -ENOSPC;
> diff --git a/doc/mvebu/cmd/bubt.txt b/doc/mvebu/cmd/bubt.txt
> index 6051243f1165..1fe1f07dd187 100644
> --- a/doc/mvebu/cmd/bubt.txt
> +++ b/doc/mvebu/cmd/bubt.txt
> @@ -14,8 +14,7 @@ Examples:
>   
>   Notes:
>   - For the TFTP interface set serverip and ipaddr.
> -- To burn image to SD/eMMC device, the target is defined
> -  by parameters CONFIG_SYS_MMC_ENV_DEV and CONFIG_SYS_MMC_ENV_PART.
> +- To burn image to SD/eMMC device, the target is defined by HW partition.
>   
>   Bubt command details (burn image step by-step)
>   ----------------------------------------------
> @@ -40,10 +39,20 @@ Notes:
>     Number 0 is used for user data partition and should not be utilized for storing
>     boot images and U-Boot environment in RAW mode since it will break file system
>     structures usually located here.
> -  The default boot partition is BOOT0. It is selected by the following parameter:
> -  CONFIG_SYS_MMC_ENV_PART=1
> -  Valid values for this parameter are 1 for BOOT0 and 2 for BOOT1.
> -  Please never use partition number 0 here!
> +
> +  Currently configured boot partition can be printed by command:
> +  # mmc partconf 0
> +  (search for BOOT_PARTITION_ACCESS output, number 7 is user data)
> +
> +  Change it to BOOT0:
> +  # mmc partconf 0 0 1 1
> +
> +  Change it to BOOT1:
> +  # mmc partconf 0 0 2 2
> +
> +  Change it to user data:
> +  # mmc partconf 0 0 7 0
> +
>   - The partition number is ignored if the target device is SD card.
>   - The boot image offset starts at block 0 for eMMC and block 1 for SD devices.
>     The block 0 on SD devices is left for MBR storage.

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de