[PATCH 0/3] Code improvements and argument checks for mmc dev subcommand

[PATCH 0/3] Code improvements and argument checks for mmc dev subcommand

[Yao Zi]

This series reduces code duplication by utilizing fallthrough, then adds
extra checks for arguments of mmc dev to ensure they're valid numbers,
preventing unexpected behavior.

The last patch replaces an integer literal returned in do_mmc_dev() by
its corresponding symbolic constant, CMD_RET_FAILURE to improve
readability, no function change is involved.

Yao Zi (3):
  cmd: mmc: Simplify dev subcommand handling
  cmd: mmc: Check whether arguments are valid numbers in dev subcommand
  cmd: mmc: Return symbolic value when part switching fails in mmc dev

 cmd/mmc.c | 55 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 23 deletions(-)

-- 
2.52.0

[PATCH 1/3] cmd: mmc: Simplify dev subcommand handling

[Yao Zi]

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 <me@ziyao.cc>
---
 cmd/mmc.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 5340a58be8ee..f2a59af087cb 100644
--- a/cmd/mmc.c
+++ b/cmd/mmc.c
@@ -16,6 +16,7 @@
 #include <sparse_format.h>
 #include <image-sparse.h>
 #include <vsprintf.h>
+#include <linux/compiler_attributes.h>
 #include <linux/ctype.h>
 
 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;
 	}
 
-- 
2.52.0

[PATCH 2/3] cmd: mmc: Check whether arguments are valid numbers in dev subcommand

[Yao Zi]

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 <me@ziyao.cc>
---
 cmd/mmc.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index f2a59af087cb..512bd482ae82 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);
-- 
2.52.0

[PATCH 3/3] cmd: mmc: Return symbolic value when part switching fails in mmc dev

[Yao Zi]

Return symbolic value CMD_RET_FAILURE instead of literal "1" when
failing to switch the partition to improve readability.

Signed-off-by: Yao Zi <me@ziyao.cc>
---
 cmd/mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/mmc.c b/cmd/mmc.c
index 512bd482ae82..bcbe963f8ac0 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)
-- 
2.52.0

Re: [PATCH 0/3] Code improvements and argument checks for mmc dev subcommand

[Anshul Dalal]

On Fri Jan 30, 2026 at 11:33 PM IST, Yao Zi wrote:
> This series reduces code duplication by utilizing fallthrough, then adds
> extra checks for arguments of mmc dev to ensure they're valid numbers,
> preventing unexpected behavior.
>
> The last patch replaces an integer literal returned in do_mmc_dev() by
> its corresponding symbolic constant, CMD_RET_FAILURE to improve
> readability, no function change is involved.

The overall patch series looks good to me, thanks for the cleanup!

Tested-by: Anshul Dalal <anshuld@ti.com>

>
> Yao Zi (3):
>   cmd: mmc: Simplify dev subcommand handling
>   cmd: mmc: Check whether arguments are valid numbers in dev subcommand
>   cmd: mmc: Return symbolic value when part switching fails in mmc dev
>
>  cmd/mmc.c | 55 ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 23 deletions(-)

Re: [PATCH 0/3] Code improvements and argument checks for mmc dev subcommand

[Peng Fan]

On Fri, Jan 30, 2026 at 06:03:50PM +0000, Yao Zi wrote:
>This series reduces code duplication by utilizing fallthrough, then adds
>extra checks for arguments of mmc dev to ensure they're valid numbers,
>preventing unexpected behavior.
>
>The last patch replaces an integer literal returned in do_mmc_dev() by
>its corresponding symbolic constant, CMD_RET_FAILURE to improve
>readability, no function change is involved.
>
>Yao Zi (3):
>  cmd: mmc: Simplify dev subcommand handling
>  cmd: mmc: Check whether arguments are valid numbers in dev subcommand
>  cmd: mmc: Return symbolic value when part switching fails in mmc dev
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

Re: [PATCH 0/3] Code improvements and argument checks for mmc dev subcommand

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>


On Fri, 30 Jan 2026 18:03:50 +0000, Yao Zi wrote:
> This series reduces code duplication by utilizing fallthrough, then adds
> extra checks for arguments of mmc dev to ensure they're valid numbers,
> preventing unexpected behavior.
> 
> The last patch replaces an integer literal returned in do_mmc_dev() by
> its corresponding symbolic constant, CMD_RET_FAILURE to improve
> readability, no function change is involved.
> 
> [...]

Applied to mmc for 2026.04-rc2, thanks!

[1/3] cmd: mmc: Simplify dev subcommand handling
      commit: 97cc26f6b6a85312305e5b953ef44861891936bf
[2/3] cmd: mmc: Check whether arguments are valid numbers in dev subcommand
      commit: f955e00e42e9b7ef5a3adf1ba1947f46c05a3d36
[3/3] cmd: mmc: Return symbolic value when part switching fails in mmc dev
      commit: b4f0479e076014f403319fdce53c5c1ac278f8ae

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>