From 97cc26f6b6a85312305e5b953ef44861891936bf Mon Sep 17 00:00:00 2001 From: Yao Zi Date: Fri, 30 Jan 2026 18:03:51 +0000 Subject: cmd: mmc: Simplify dev subcommand handling Replace the big if-else block in do_mmc_dev() with switch-case and use fallthrough to remove the duplicated code for parsing dev and part. Signed-off-by: Yao Zi Tested-by: Anshul Dalal Reviewed-by: Peng Fan Signed-off-by: Peng Fan --- cmd/mmc.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) (limited to 'cmd') diff --git a/cmd/mmc.c b/cmd/mmc.c index 5340a58be8e..f2a59af087c 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -16,6 +16,7 @@ #include #include #include +#include #include static int curr_device = -1; @@ -556,37 +557,30 @@ static int do_mmc_part(struct cmd_tbl *cmdtp, int flag, static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { - int dev, part = 0, ret; + enum bus_mode speed_mode = MMC_MODES_END; + int dev = curr_device, part = 0, ret; struct mmc *mmc; - if (argc == 1) { - dev = curr_device; - mmc = init_mmc_device(dev, true); - } else if (argc == 2) { - dev = (int)dectoul(argv[1], NULL); - mmc = init_mmc_device(dev, true); - } else if (argc == 3) { - dev = (int)dectoul(argv[1], NULL); + switch (argc) { + case 4: + speed_mode = (int)dectoul(argv[3], NULL); + fallthrough; + case 3: part = (int)dectoul(argv[2], NULL); if (part > PART_ACCESS_MASK) { printf("#part_num shouldn't be larger than %d\n", PART_ACCESS_MASK); return CMD_RET_FAILURE; } - mmc = init_mmc_device(dev, true); - } else if (argc == 4) { - enum bus_mode speed_mode; + fallthrough; + case 2: dev = (int)dectoul(argv[1], NULL); - part = (int)dectoul(argv[2], NULL); - if (part > PART_ACCESS_MASK) { - printf("#part_num shouldn't be larger than %d\n", - PART_ACCESS_MASK); - return CMD_RET_FAILURE; - } - speed_mode = (int)dectoul(argv[3], NULL); + fallthrough; + case 1: mmc = __init_mmc_device(dev, true, speed_mode); - } else { + break; + default: return CMD_RET_USAGE; } -- cgit v1.2.3 From f955e00e42e9b7ef5a3adf1ba1947f46c05a3d36 Mon Sep 17 00:00:00 2001 From: Yao Zi Date: Fri, 30 Jan 2026 18:03:52 +0000 Subject: cmd: mmc: Check whether arguments are valid numbers in dev subcommand Currently when any of speed_mode, part, or dev fails to be parse as a number, no error is reported. In this case __init_mmc_device() is called with weird arguments, probably zeroes if there's no digit prefixing the argument, which is especially confusing when the invocation occasionally succeeds. Let's check whether arguments are valid numbers without trailing characters. This is quite helpful for speed_mode: it requires an index instead of a mode name, one may easily pass in a string, which will be parsed as zero (MMC_LEGACY), without carefully reading the documentation, then finds the MMC device is under an unexpected mode. Signed-off-by: Yao Zi Tested-by: Anshul Dalal Reviewed-by: Peng Fan Signed-off-by: Peng Fan --- cmd/mmc.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'cmd') diff --git a/cmd/mmc.c b/cmd/mmc.c index f2a59af087c..512bd482ae8 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -559,15 +559,25 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag, { enum bus_mode speed_mode = MMC_MODES_END; int dev = curr_device, part = 0, ret; + char *endp; struct mmc *mmc; switch (argc) { case 4: - speed_mode = (int)dectoul(argv[3], NULL); + speed_mode = (int)dectoul(argv[3], &endp); + if (*endp) { + printf("Invalid speed mode index '%s', " + "did you specify a mode name?\n", argv[3]); + return CMD_RET_USAGE; + } + fallthrough; case 3: - part = (int)dectoul(argv[2], NULL); - if (part > PART_ACCESS_MASK) { + part = (int)dectoul(argv[2], &endp); + if (*endp) { + printf("Invalid part number '%s'\n", argv[2]); + return CMD_RET_USAGE; + } else if (part > PART_ACCESS_MASK) { printf("#part_num shouldn't be larger than %d\n", PART_ACCESS_MASK); return CMD_RET_FAILURE; @@ -575,7 +585,12 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag, fallthrough; case 2: - dev = (int)dectoul(argv[1], NULL); + dev = (int)dectoul(argv[1], &endp); + if (*endp) { + printf("Invalid device number '%s'\n", argv[1]); + return CMD_RET_USAGE; + } + fallthrough; case 1: mmc = __init_mmc_device(dev, true, speed_mode); -- cgit v1.2.3 From b4f0479e076014f403319fdce53c5c1ac278f8ae Mon Sep 17 00:00:00 2001 From: Yao Zi Date: Fri, 30 Jan 2026 18:03:53 +0000 Subject: cmd: mmc: Return symbolic value when part switching fails in mmc dev Return symbolic value CMD_RET_FAILURE instead of literal "1" when failing to switch the partition to improve readability. Signed-off-by: Yao Zi Tested-by: Anshul Dalal Reviewed-by: Peng Fan Signed-off-by: Peng Fan --- cmd/mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cmd') diff --git a/cmd/mmc.c b/cmd/mmc.c index 512bd482ae8..bcbe963f8ac 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -606,7 +606,7 @@ static int do_mmc_dev(struct cmd_tbl *cmdtp, int flag, printf("switch to partitions #%d, %s\n", part, (!ret) ? "OK" : "ERROR"); if (ret) - return 1; + return CMD_RET_FAILURE; curr_device = dev; if (mmc->part_config == MMCPART_NOAVAILABLE) -- cgit v1.2.3