[PATCH v3] cmd: mmc: Allow using partition name in mmc erase command

[Quentin Schulz]

Hi Tomas,

On 9/2/24 8:49 PM, Tomas Paukrt wrote:
> The mmc erase command currently requires blk# and cnt parameters
> which can be obtained using the part start and part size commands
> if the entire partition needs to be erased.
> 
> Simplify the use of the mmc erase command by allowing the partition
> name to be specified directly.
> 
> Signed-off-by: Tomas Paukrt <tomaspaukrt at email.cz>
> ---
> Changes since v1:
> - Swapped init_mmc_device call and argc check
> - Removed round down info.size to a multiple of erase_grp_size
> - Split the syntax into two separate lines
> ---
>   cmd/mmc.c             | 17 +++++++++++++----
>   doc/usage/cmd/mmc.rst |  6 +++++-
>   2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 7244a90..2fc05f9 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -472,18 +472,26 @@ static int do_mmc_erase(struct cmd_tbl *cmdtp, int flag,
>   			int argc, char *const argv[])
>   {
>   	struct mmc *mmc;
> +	struct disk_partition info;
>   	u32 blk, cnt, n;
>   
> -	if (argc != 3)
> +	if (argc < 2 || argc > 3)
>   		return CMD_RET_USAGE;
>   
> -	blk = hextoul(argv[1], NULL);
> -	cnt = hextoul(argv[2], NULL);
> -
>   	mmc = init_mmc_device(curr_device, false);
>   	if (!mmc)
>   		return CMD_RET_FAILURE;
>   
> +	if (argc == 3) {
> +		blk = hextoul(argv[1], NULL);
> +		cnt = hextoul(argv[2], NULL);
> +	} else if (part_get_info_by_name(mmc_get_blk_desc(mmc), argv[1], &info) >= 0) {
> +		blk = info.start;
> +		cnt = info.size;

Same question as for v1, are we sure that the partition block size is 
the same as the MMC erase block size? Is this guaranteed by all 
standards (all MMC standards, and all partition table standards)? 
Otherwise I assume we should do some maths here to figure out the MMC 
start block AND block size based on the partition start block AND block 
size and to pass the correct information to blk_derase(). I hope they 
also are guaranteed to be aligned somehow and that a partition block 
size is a multiple of an MMC erase block size? This maths seems to be 
done in mmc_berase() (at a quick glance, not sure it's appropriate).

Cheers,
Quentin

> +	} else {
> +		return CMD_RET_FAILURE;
> +	}
> +
>   	printf("\nMMC erase: dev # %d, block # %d, count %d ... ",
>   	       curr_device, blk, cnt);
>   
> @@ -1271,6 +1279,7 @@ U_BOOT_CMD(
>   	"mmc swrite addr blk#\n"
>   #endif
>   	"mmc erase blk# cnt\n"
> +	"mmc erase partname\n"
>   	"mmc rescan [mode]\n"
>   	"mmc part - lists available partition on current mmc device\n"
>   	"mmc dev [dev] [part] [mode] - show or set current mmc device [partition] and set mode\n"
> diff --git a/doc/usage/cmd/mmc.rst b/doc/usage/cmd/mmc.rst
> index 5a64400..55391fd 100644
> --- a/doc/usage/cmd/mmc.rst
> +++ b/doc/usage/cmd/mmc.rst
> @@ -15,6 +15,7 @@ Synopsis
>       mmc read addr blk# cnt
>       mmc write addr blk# cnt
>       mmc erase blk# cnt
> +    mmc erase partname
>       mmc rescan [mode]
>       mmc part
>       mmc dev [dev] [part] [mode]
> @@ -44,12 +45,15 @@ The 'mmc write' command writes raw data to MMC device from memory address with b
>       cnt
>           block count
>   
> -The 'mmc erase' command erases *cnt* blocks on the MMC device starting at block *blk#*.
> +The 'mmc erase' command erases *cnt* blocks on the MMC device starting at block *blk#* or
> +the entire partition specified by *partname*.
>   
>       blk#
>           start block offset
>       cnt
>           block count
> +    partname
> +        partition name
>   
>   The 'mmc rescan' command scans the available MMC device.
>