U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] cmd: fuse: add switch for quiet operation
@ 2025-03-14 19:54 Rogerio Guerra Borin
  0 siblings, 0 replies; 4+ messages in thread
From: Rogerio Guerra Borin @ 2025-03-14 19:54 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Rogerio Guerra Borin

Add switch -q for quiet operation to all fuse subcommands. This helps
avoid bloating the console with messages that can be distracting to
users (particularly when the command is employed by scripts and multiple
fuse values are read/compared/programmed). For example, the "fuse cmp"
command normally produces five lines of output:

    U-Boot # fuse cmp 6 0 0x70af49db
    Comparing bank 6:

    Word 0x00000000:
    Value 0x70af49db:0x70af49db
    passed
    U-Boot # echo $?
    0

But scripts issuing the command do not know or care about that output
since the command exit code is the only relevant information visible to
them. With the new switch one can avoid the unnecessary output:

    U-Boot # fuse cmp -q 6 0 0x70af49db
    U-Boot # echo $?
    0

Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
---
 cmd/fuse.c | 79 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/cmd/fuse.c b/cmd/fuse.c
index 598ef496a43..5ed221b1ecf 100644
--- a/cmd/fuse.c
+++ b/cmd/fuse.c
@@ -45,14 +45,30 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {
 	const char *op = cmd_arg1(argc, argv);
-	int confirmed = argc >= 3 && !strcmp(argv[2], "-y");
+	int confirmed = 0, quiet = 0;
 	u32 bank, word, cnt, val, cmp;
 	ulong addr;
 	void *buf, *start;
 	int ret, i;
 
-	argc -= 2 + confirmed;
-	argv += 2 + confirmed;
+#define IF_NOT_QUIET(stmt) do { if (!quiet) { stmt; } } while(0)
+
+	if (argc >= 2) {
+		argc -= 2;
+		argv += 2;
+	}
+
+	/* Handle common switches. */
+	while (argc) {
+		if (!strcmp(argv[0], "-y"))
+			confirmed = 1;
+		else if (!strcmp(argv[0], "-q"))
+			quiet = 1;
+		else
+			break;
+		argc--;
+		argv++;
+	}
 
 	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
 			strtou32(argv[1], 0, &word))
@@ -64,18 +80,18 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
 			return CMD_RET_USAGE;
 
-		printf("Reading bank %u:\n", bank);
+		IF_NOT_QUIET(printf("Reading bank %u:\n", bank));
 		for (i = 0; i < cnt; i++, word++) {
 			if (!(i % 4))
-				printf("\nWord 0x%.8x:", word);
+				IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
 
 			ret = fuse_read(bank, word, &val);
 			if (ret)
 				goto err;
 
-			printf(" %.8x", val);
+			IF_NOT_QUIET(printf(" %.8x", val));
 		}
-		putc('\n');
+		IF_NOT_QUIET(putc('\n'));
 	} else if (!strcmp(op, "readm")) {
 		if (argc == 3)
 			cnt = 1;
@@ -87,7 +103,8 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		start = map_sysmem(addr, 4);
 		buf = start;
 
-		printf("Reading bank %u len %u to 0x%lx\n", bank, cnt, addr);
+		IF_NOT_QUIET(printf("Reading bank %u len %u to 0x%lx\n",
+				    bank, cnt, addr));
 		for (i = 0; i < cnt; i++, word++) {
 			ret = fuse_read(bank, word, &val);
 			if (ret)
@@ -102,38 +119,38 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		if (argc != 3 || strtou32(argv[2], 0, &cmp))
 			return CMD_RET_USAGE;
 
-		printf("Comparing bank %u:\n", bank);
-		printf("\nWord 0x%.8x:", word);
-		printf("\nValue 0x%.8x:", cmp);
+		IF_NOT_QUIET(printf("Comparing bank %u:\n", bank));
+		IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
+		IF_NOT_QUIET(printf("\nValue 0x%.8x:", cmp));
 
 		ret = fuse_read(bank, word, &val);
 		if (ret)
 			goto err;
 
-		printf("0x%.8x\n", val);
+		IF_NOT_QUIET(printf("0x%.8x\n", val));
 		if (val != cmp) {
-			printf("failed\n");
+			IF_NOT_QUIET(printf("failed\n"));
 			return CMD_RET_FAILURE;
 		}
-		printf("passed\n");
+		IF_NOT_QUIET(printf("passed\n"));
 	} else if (!strcmp(op, "sense")) {
 		if (argc == 2)
 			cnt = 1;
 		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
 			return CMD_RET_USAGE;
 
-		printf("Sensing bank %u:\n", bank);
+		IF_NOT_QUIET(printf("Sensing bank %u:\n", bank));
 		for (i = 0; i < cnt; i++, word++) {
 			if (!(i % 4))
-				printf("\nWord 0x%.8x:", word);
+				IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
 
 			ret = fuse_sense(bank, word, &val);
 			if (ret)
 				goto err;
 
-			printf(" %.8x", val);
+			IF_NOT_QUIET(printf(" %.8x", val));
 		}
-		putc('\n');
+		IF_NOT_QUIET(putc('\n'));
 	} else if (!strcmp(op, "prog")) {
 		if (argc < 3)
 			return CMD_RET_USAGE;
@@ -142,8 +159,9 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			if (strtou32(argv[i], 16, &val))
 				return CMD_RET_USAGE;
 
-			printf("Programming bank %u word 0x%.8x to 0x%.8x...\n",
-					bank, word, val);
+			IF_NOT_QUIET(printf("Programming bank %u word "
+					    "0x%.8x to 0x%.8x...\n",
+					    bank, word, val));
 			if (!confirmed && !confirm_prog())
 				return CMD_RET_FAILURE;
 			ret = fuse_prog(bank, word, val);
@@ -158,8 +176,9 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			if (strtou32(argv[i], 16, &val))
 				return CMD_RET_USAGE;
 
-			printf("Overriding bank %u word 0x%.8x with "
-					"0x%.8x...\n", bank, word, val);
+			IF_NOT_QUIET(printf("Overriding bank %u word "
+					    "0x%.8x with 0x%.8x...\n",
+					    bank, word, val));
 			ret = fuse_override(bank, word, val);
 			if (ret)
 				goto err;
@@ -171,23 +190,25 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 
 err:
-	puts("ERROR\n");
+	IF_NOT_QUIET(puts("ERROR\n"));
 	return CMD_RET_FAILURE;
+
+#undef IF_NOT_QUIET
 }
 
 U_BOOT_CMD(
 	fuse, CONFIG_SYS_MAXARGS, 0, do_fuse,
 	"Fuse sub-system",
-	     "read <bank> <word> [<cnt>] - read 1 or 'cnt' fuse words,\n"
+	     "read [-q] <bank> <word> [<cnt>] - read 1 or 'cnt' fuse words,\n"
 	"    starting at 'word'\n"
-	"fuse cmp <bank> <word> <hexval> - compare 'hexval' to fuse\n"
+	"fuse cmp [-q] <bank> <word> <hexval> - compare 'hexval' to fuse\n"
 	"    at 'word'\n"
-	"fuse readm <bank> <word> <addr> [<cnt>] - read 1 or 'cnt' fuse words,\n"
+	"fuse readm [-q] <bank> <word> <addr> [<cnt>] - read 1 or 'cnt' fuse words,\n"
 	"    starting at 'word' into memory at 'addr'\n"
-	"fuse sense <bank> <word> [<cnt>] - sense 1 or 'cnt' fuse words,\n"
+	"fuse sense [-q] <bank> <word> [<cnt>] - sense 1 or 'cnt' fuse words,\n"
 	"    starting at 'word'\n"
-	"fuse prog [-y] <bank> <word> <hexval> [<hexval>...] - program 1 or\n"
+	"fuse prog [-q] [-y] <bank> <word> <hexval> [<hexval>...] - program 1 or\n"
 	"    several fuse words, starting at 'word' (PERMANENT)\n"
-	"fuse override <bank> <word> <hexval> [<hexval>...] - override 1 or\n"
+	"fuse override [-q] <bank> <word> <hexval> [<hexval>...] - override 1 or\n"
 	"    several fuse words, starting at 'word'"
 );
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] cmd: fuse: add switch for quiet operation
@ 2025-03-17 22:53 Rogerio Guerra Borin
  2025-03-24  9:55 ` Quentin Schulz
  0 siblings, 1 reply; 4+ messages in thread
From: Rogerio Guerra Borin @ 2025-03-17 22:53 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Rogerio Guerra Borin

Add switch -q for quiet operation to all fuse subcommands. This helps
avoid bloating the console with messages that can be distracting to
users (particularly when the command is employed by scripts and multiple
fuse values are read/compared/programmed). For example, the "fuse cmp"
command normally produces five lines of output:

    U-Boot # fuse cmp 6 0 0x70af49db
    Comparing bank 6:

    Word 0x00000000:
    Value 0x70af49db:0x70af49db
    passed
    U-Boot # echo $?
    0

But scripts issuing the command do not know or care about that output
since the command exit code is the only relevant information visible to
them. With the new switch one can avoid the unnecessary output:

    U-Boot # fuse cmp -q 6 0 0x70af49db
    U-Boot # echo $?
    0

Signed-off-by: Rogerio Guerra Borin <rogerio.borin@toradex.com>
---
 cmd/fuse.c | 79 ++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 50 insertions(+), 29 deletions(-)

diff --git a/cmd/fuse.c b/cmd/fuse.c
index 598ef496a43..5ed221b1ecf 100644
--- a/cmd/fuse.c
+++ b/cmd/fuse.c
@@ -45,14 +45,30 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {
 	const char *op = cmd_arg1(argc, argv);
-	int confirmed = argc >= 3 && !strcmp(argv[2], "-y");
+	int confirmed = 0, quiet = 0;
 	u32 bank, word, cnt, val, cmp;
 	ulong addr;
 	void *buf, *start;
 	int ret, i;
 
-	argc -= 2 + confirmed;
-	argv += 2 + confirmed;
+#define IF_NOT_QUIET(stmt) do { if (!quiet) { stmt; } } while(0)
+
+	if (argc >= 2) {
+		argc -= 2;
+		argv += 2;
+	}
+
+	/* Handle common switches. */
+	while (argc) {
+		if (!strcmp(argv[0], "-y"))
+			confirmed = 1;
+		else if (!strcmp(argv[0], "-q"))
+			quiet = 1;
+		else
+			break;
+		argc--;
+		argv++;
+	}
 
 	if (argc < 2 || strtou32(argv[0], 0, &bank) ||
 			strtou32(argv[1], 0, &word))
@@ -64,18 +80,18 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
 			return CMD_RET_USAGE;
 
-		printf("Reading bank %u:\n", bank);
+		IF_NOT_QUIET(printf("Reading bank %u:\n", bank));
 		for (i = 0; i < cnt; i++, word++) {
 			if (!(i % 4))
-				printf("\nWord 0x%.8x:", word);
+				IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
 
 			ret = fuse_read(bank, word, &val);
 			if (ret)
 				goto err;
 
-			printf(" %.8x", val);
+			IF_NOT_QUIET(printf(" %.8x", val));
 		}
-		putc('\n');
+		IF_NOT_QUIET(putc('\n'));
 	} else if (!strcmp(op, "readm")) {
 		if (argc == 3)
 			cnt = 1;
@@ -87,7 +103,8 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		start = map_sysmem(addr, 4);
 		buf = start;
 
-		printf("Reading bank %u len %u to 0x%lx\n", bank, cnt, addr);
+		IF_NOT_QUIET(printf("Reading bank %u len %u to 0x%lx\n",
+				    bank, cnt, addr));
 		for (i = 0; i < cnt; i++, word++) {
 			ret = fuse_read(bank, word, &val);
 			if (ret)
@@ -102,38 +119,38 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 		if (argc != 3 || strtou32(argv[2], 0, &cmp))
 			return CMD_RET_USAGE;
 
-		printf("Comparing bank %u:\n", bank);
-		printf("\nWord 0x%.8x:", word);
-		printf("\nValue 0x%.8x:", cmp);
+		IF_NOT_QUIET(printf("Comparing bank %u:\n", bank));
+		IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
+		IF_NOT_QUIET(printf("\nValue 0x%.8x:", cmp));
 
 		ret = fuse_read(bank, word, &val);
 		if (ret)
 			goto err;
 
-		printf("0x%.8x\n", val);
+		IF_NOT_QUIET(printf("0x%.8x\n", val));
 		if (val != cmp) {
-			printf("failed\n");
+			IF_NOT_QUIET(printf("failed\n"));
 			return CMD_RET_FAILURE;
 		}
-		printf("passed\n");
+		IF_NOT_QUIET(printf("passed\n"));
 	} else if (!strcmp(op, "sense")) {
 		if (argc == 2)
 			cnt = 1;
 		else if (argc != 3 || strtou32(argv[2], 0, &cnt))
 			return CMD_RET_USAGE;
 
-		printf("Sensing bank %u:\n", bank);
+		IF_NOT_QUIET(printf("Sensing bank %u:\n", bank));
 		for (i = 0; i < cnt; i++, word++) {
 			if (!(i % 4))
-				printf("\nWord 0x%.8x:", word);
+				IF_NOT_QUIET(printf("\nWord 0x%.8x:", word));
 
 			ret = fuse_sense(bank, word, &val);
 			if (ret)
 				goto err;
 
-			printf(" %.8x", val);
+			IF_NOT_QUIET(printf(" %.8x", val));
 		}
-		putc('\n');
+		IF_NOT_QUIET(putc('\n'));
 	} else if (!strcmp(op, "prog")) {
 		if (argc < 3)
 			return CMD_RET_USAGE;
@@ -142,8 +159,9 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			if (strtou32(argv[i], 16, &val))
 				return CMD_RET_USAGE;
 
-			printf("Programming bank %u word 0x%.8x to 0x%.8x...\n",
-					bank, word, val);
+			IF_NOT_QUIET(printf("Programming bank %u word "
+					    "0x%.8x to 0x%.8x...\n",
+					    bank, word, val));
 			if (!confirmed && !confirm_prog())
 				return CMD_RET_FAILURE;
 			ret = fuse_prog(bank, word, val);
@@ -158,8 +176,9 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 			if (strtou32(argv[i], 16, &val))
 				return CMD_RET_USAGE;
 
-			printf("Overriding bank %u word 0x%.8x with "
-					"0x%.8x...\n", bank, word, val);
+			IF_NOT_QUIET(printf("Overriding bank %u word "
+					    "0x%.8x with 0x%.8x...\n",
+					    bank, word, val));
 			ret = fuse_override(bank, word, val);
 			if (ret)
 				goto err;
@@ -171,23 +190,25 @@ static int do_fuse(struct cmd_tbl *cmdtp, int flag, int argc,
 	return 0;
 
 err:
-	puts("ERROR\n");
+	IF_NOT_QUIET(puts("ERROR\n"));
 	return CMD_RET_FAILURE;
+
+#undef IF_NOT_QUIET
 }
 
 U_BOOT_CMD(
 	fuse, CONFIG_SYS_MAXARGS, 0, do_fuse,
 	"Fuse sub-system",
-	     "read <bank> <word> [<cnt>] - read 1 or 'cnt' fuse words,\n"
+	     "read [-q] <bank> <word> [<cnt>] - read 1 or 'cnt' fuse words,\n"
 	"    starting at 'word'\n"
-	"fuse cmp <bank> <word> <hexval> - compare 'hexval' to fuse\n"
+	"fuse cmp [-q] <bank> <word> <hexval> - compare 'hexval' to fuse\n"
 	"    at 'word'\n"
-	"fuse readm <bank> <word> <addr> [<cnt>] - read 1 or 'cnt' fuse words,\n"
+	"fuse readm [-q] <bank> <word> <addr> [<cnt>] - read 1 or 'cnt' fuse words,\n"
 	"    starting at 'word' into memory at 'addr'\n"
-	"fuse sense <bank> <word> [<cnt>] - sense 1 or 'cnt' fuse words,\n"
+	"fuse sense [-q] <bank> <word> [<cnt>] - sense 1 or 'cnt' fuse words,\n"
 	"    starting at 'word'\n"
-	"fuse prog [-y] <bank> <word> <hexval> [<hexval>...] - program 1 or\n"
+	"fuse prog [-q] [-y] <bank> <word> <hexval> [<hexval>...] - program 1 or\n"
 	"    several fuse words, starting at 'word' (PERMANENT)\n"
-	"fuse override <bank> <word> <hexval> [<hexval>...] - override 1 or\n"
+	"fuse override [-q] <bank> <word> <hexval> [<hexval>...] - override 1 or\n"
 	"    several fuse words, starting at 'word'"
 );
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] cmd: fuse: add switch for quiet operation
  2025-03-17 22:53 [PATCH] cmd: fuse: add switch for quiet operation Rogerio Guerra Borin
@ 2025-03-24  9:55 ` Quentin Schulz
  2025-03-30 19:37   ` Rogerio Guerra Borin
  0 siblings, 1 reply; 4+ messages in thread
From: Quentin Schulz @ 2025-03-24  9:55 UTC (permalink / raw)
  To: Rogerio Guerra Borin, Tom Rini; +Cc: u-boot, Rogerio Guerra Borin

Hi Rogerio,

On 3/17/25 11:53 PM, Rogerio Guerra Borin wrote:
> [You don't often get email from rogerio.borin@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Add switch -q for quiet operation to all fuse subcommands. This helps
> avoid bloating the console with messages that can be distracting to
> users (particularly when the command is employed by scripts and multiple
> fuse values are read/compared/programmed). For example, the "fuse cmp"
> command normally produces five lines of output:
> 
>      U-Boot # fuse cmp 6 0 0x70af49db
>      Comparing bank 6:
> 
>      Word 0x00000000:
>      Value 0x70af49db:0x70af49db
>      passed
>      U-Boot # echo $?
>      0
> 
> But scripts issuing the command do not know or care about that output
> since the command exit code is the only relevant information visible to
> them. With the new switch one can avoid the unnecessary output:
> 
>      U-Boot # fuse cmp -q 6 0 0x70af49db
>      U-Boot # echo $?
>      0
> 

I think this is a reasonable expectation. I'm wondering if we shouldn't 
think about using the log architecture we have to have a generic 
solution rather than expecting each command driver to handle (possibly 
in their own way) a quiet flag.

https://docs.u-boot.org/en/latest/develop/logging.html

The idea I have in mind is that we could dynamically set the loglevel 
for a specific command driver (or at least for the command subsystem (by 
e.g. defining an LOG_CATEGORY LOGC_CMD) and use log_* functions in the 
command driver rather than IF_NOT_QUIET() below. This would also allow 
different log levels to be used. Maybe there's something to be done with 
filters, or something new needs to be added to set the log level for a 
category maybe? I don't know, not too verse in our log system.

Another option to have a generic logic would be to depend on an 
environment variable instead of needing to expand each command driver's 
logic.

Cheers,
Quentin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cmd: fuse: add switch for quiet operation
  2025-03-24  9:55 ` Quentin Schulz
@ 2025-03-30 19:37   ` Rogerio Guerra Borin
  0 siblings, 0 replies; 4+ messages in thread
From: Rogerio Guerra Borin @ 2025-03-30 19:37 UTC (permalink / raw)
  To: Quentin Schulz, Rogerio Guerra Borin, Tom Rini; +Cc: u-boot

Hello Quentin,

Thanks for you feedback.

On 3/24/25 06:55, Quentin Schulz wrote:
> I think this is a reasonable expectation. I'm wondering if we shouldn't 
> think about using the log architecture we have to have a generic 
> solution rather than expecting each command driver to handle (possibly 
> in their own way) a quiet flag.
> 
> https://docs.u-boot.org/en/latest/develop/logging.html
> 
> The idea I have in mind is that we could dynamically set the loglevel 
> for a specific command driver (or at least for the command subsystem (by 
> e.g. defining an LOG_CATEGORY LOGC_CMD) and use log_* functions in the 
> command driver rather than IF_NOT_QUIET() below. This would also allow 
> different log levels to be used. Maybe there's something to be done with 
> filters, or something new needs to be added to set the log level for a 
> category maybe? I don't know, not too verse in our log system.
> 
> Another option to have a generic logic would be to depend on an 
> environment variable instead of needing to expand each command driver's 
> logic.

That's a great idea. I'll look into it and get back when I have something.

Regards,
Rogerio


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-03-31  2:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-17 22:53 [PATCH] cmd: fuse: add switch for quiet operation Rogerio Guerra Borin
2025-03-24  9:55 ` Quentin Schulz
2025-03-30 19:37   ` Rogerio Guerra Borin
  -- strict thread matches above, loose matches on Subject: below --
2025-03-14 19:54 Rogerio Guerra Borin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox