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

[Quentin Schulz]

Hi Tomas,

On 9/1/24 3:59 PM, Tomas Paukrt wrote:
> The mmc erase command currently requires blk# and cnt parameters
> which can be obtained using the part command if the entire partition
> needs to be deleted.
> 
> 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>
> ---
>   cmd/mmc.c             | 21 ++++++++++++++-------
>   doc/usage/cmd/mmc.rst |  4 +++-
>   2 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/cmd/mmc.c b/cmd/mmc.c
> index 7244a90..ba2f506 100644
> --- a/cmd/mmc.c
> +++ b/cmd/mmc.c
> @@ -472,18 +472,25 @@ 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)
> -		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;
>   

I assume we do not want to to init the mmc device if we know the command 
is wrong (based on the number of arguments)?

So keep the first if condition, but check also if it's different from 2 ?

> +	if (argc == 2) {
> +		if (part_get_info_by_name(mmc_get_blk_desc(mmc), argv[1], &info) < 0)
> +			return CMD_RET_FAILURE;
> +		blk = info.start;
> +		cnt = info.size & ~(mmc->erase_grp_size - 1);

Is this basically a way to round down info.size to a multiple of 
erase_grp_size? If I read that correctly, please add a comment to make 
this explicit.

I'm not too familiar with MMC, but do all MMCs have the same block size 
by standard? I know it's 512B for eMMC, but don't know for other MMCs. 
The same question for the disk partition? The fact that there's a 
blksize member in struct disk_partition seems to indicate the opposite.

I'm wondering if we shouldn't be using mmc_berase() instead?

> +	} else if (argc == 3) {
> +		blk = hextoul(argv[1], NULL);
> +		cnt = hextoul(argv[2], NULL);
> +	} else {
> +		return CMD_RET_USAGE;
> +	}
> +
>   	printf("\nMMC erase: dev # %d, block # %d, count %d ... ",
>   	       curr_device, blk, cnt);
>   
> @@ -1270,7 +1277,7 @@ U_BOOT_CMD(
>   #if CONFIG_IS_ENABLED(CMD_MMC_SWRITE)
>   	"mmc swrite addr blk#\n"
>   #endif
> -	"mmc erase blk# cnt\n"
> +	"mmc erase blk# cnt | partname\n"

It's not entirely clear to me if "partname" replaces "blk# cnt" or just 
"cnt" by just reading this line.

Maybe have another line:

mmc erase partname

or is there a better way to show this I don't know about?

Cheers,
Quentin