Skip to contentSkip to conversation

Drop command-processing code when CONFIG_CMDLINE is disabled

PATCH New Simon Glass 3 participants 7 messages Mar 19, 2016 – Apr 4, 2016

lore
Share & tools
lore

Simon Glass

author
#0 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Command parsing and processing code is not needed when the command line is disabled. Remove this code in that case.

Signed-off-by Simon Glass <[email protected]>
Reviewed-by Tom Rini <[email protected]>

---

Changes in v3:
- Drop the indentation on the #if...#else...#endif

Changes in v2:
- Move the board_run_command() prototype into this patch

 cmd/help.c        |  4 ++++
 common/cli.c      | 17 ++++++++++++++++-
 common/command.c  |  6 ++++++
 include/command.h | 18 ++++++++++++++++++
 4 files changed, 44 insertions(+), 1 deletion(-)
cmd/help.c +4 -0
index 6ff494d..701ae7e 100644
@@ -10,9 +10,13 @@
 
 static int do_help(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
+#ifdef CONFIG_CMDLINE
 	cmd_tbl_t *start = ll_entry_start(cmd_tbl_t, cmd);
 	const int len = ll_entry_count(cmd_tbl_t, cmd);
 	return _do_help(start, len, cmdtp, flag, argc, argv);
+#else
+	return 0;
+#endif
 }
 
 U_BOOT_CMD(
common/cli.c +16 -1
index 119d282..5e17da8 100644
@@ -18,6 +18,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifdef CONFIG_CMDLINE
 /*
  * Run a command using the selected parser.
  *

@@ -68,6 +69,7 @@ int run_command_repeatable(const char *cmd, int flag)
 	return 0;
 #endif
 }
+#endif /* CONFIG_CMDLINE */
 
 int run_command_list(const char *cmd, int len, int flag)
 {

@@ -102,7 +104,11 @@ int run_command_list(const char *cmd, int len, int flag)
 	 * doing a malloc() which is actually required only in a case that
 	 * is pretty rare.
 	 */
+#ifdef CONFIG_CMDLINE
 	rcode = cli_simple_run_command_list(buff, flag);
+#else
+	rcode = board_run_command(buff);
+#endif
 #endif
 	if (need_buff)
 		free(buff);

@@ -166,7 +172,9 @@ bool cli_process_fdt(const char **cmdp)
  */
 void cli_secure_boot_cmd(const char *cmd)
 {
+#ifdef CONFIG_CMDLINE
 	cmd_tbl_t *cmdtp;
+#endif
 	int rc;
 
 	if (!cmd) {

@@ -178,6 +186,7 @@ void cli_secure_boot_cmd(const char *cmd)
 	disable_ctrlc(1);
 
 	/* Find the command directly. */
+#ifdef CONFIG_CMDLINE
 	cmdtp = find_cmd(cmd);
 	if (!cmdtp) {
 		printf("## Error: \"%s\" not defined\n", cmd);

@@ -187,6 +196,10 @@ void cli_secure_boot_cmd(const char *cmd)
 	/* Run the command, forcing no flags and faking argc and argv. */
 	rc = (cmdtp->cmd)(cmdtp, 0, 1, (char **)&cmd);
 
+#else
+	rc = board_run_command(cmd);
+#endif
+
 	/* Shouldn't ever return from boot command. */
 	printf("## Error: \"%s\" returned (code %d)\n", cmd, rc);
 

@@ -205,8 +218,10 @@ void cli_loop(void)
 	parse_file_outer();
 	/* This point is never reached */
 	for (;;);
-#else
+#elif defined(CONFIG_CMDINE)
 	cli_simple_loop();
+#else
+	printf("## U-Boot command line is disabled. Please enable CONFIG_CMDLINE\n");
 #endif /*CONFIG_SYS_HUSH_PARSER*/
 }
common/command.c +6 -0
index 858e288..e5d9b9c 100644
@@ -85,6 +85,7 @@ int _do_help(cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t *cmdtp, int flag,
 /* find command table entry for a command */
 cmd_tbl_t *find_cmd_tbl(const char *cmd, cmd_tbl_t *table, int table_len)
 {
+#ifdef CONFIG_CMDLINE
 	cmd_tbl_t *cmdtp;
 	cmd_tbl_t *cmdtp_temp = table;	/* Init value */
 	const char *p;

@@ -111,6 +112,7 @@ cmd_tbl_t *find_cmd_tbl(const char *cmd, cmd_tbl_t *table, int table_len)
 	if (n_found == 1) {			/* exactly one match */
 		return cmdtp_temp;
 	}
+#endif /* CONFIG_CMDLINE */
 
 	return NULL;	/* not found or ambiguous command */
 }

@@ -162,6 +164,7 @@ int var_complete(int argc, char * const argv[], char last_char, int maxv, char *
 
 static int complete_cmdv(int argc, char * const argv[], char last_char, int maxv, char *cmdv[])
 {
+#ifdef CONFIG_CMDLINE
 	cmd_tbl_t *cmdtp = ll_entry_start(cmd_tbl_t, cmd);
 	const int count = ll_entry_count(cmd_tbl_t, cmd);
 	const cmd_tbl_t *cmdend = cmdtp + count;

@@ -231,6 +234,9 @@ static int complete_cmdv(int argc, char * const argv[], char last_char, int maxv
 
 	cmdv[n_found] = NULL;
 	return n_found;
+#else
+	return 0;
+#endif
 }
 
 static int make_argv(char *s, int argvsz, char *argv[])
include/command.h +18 -0
index 0524c0b..0d2b653 100644
@@ -144,6 +144,24 @@ int cmd_process(int flag, int argc, char * const argv[],
 			       int *repeatable, unsigned long *ticks);
 
 void fixup_cmdtable(cmd_tbl_t *cmdtp, int size);
+
+/**
+ * board_run_command() - Fallback function to execute a command
+ *
+ * When no command line features are enabled in U-Boot, this function is
+ * called to execute a command. Typically the function can look at the
+ * command and perform a few very specific tasks, such as booting the
+ * system in a particular way.
+ *
+ * This function is only used when CONFIG_CMDLINE is not enabled.
+ *
+ * In normal situations this function should not return, since U-Boot will
+ * simply hang.
+ *
+ * @cmdline:	Command line string to execute
+ * @return 0 if OK, 1 for error
+ */
+int board_run_command(const char *cmdline);
 #endif	/* __ASSEMBLY__ */
 
 /*
Signature · 1 line
2.8.0.rc3.226.g39d4020
#1 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Quoted 5 lines · Simon Glass

Command parsing and processing code is not needed when the command line is disabled. Remove this code in that case.

Signed-off-by Simon Glass <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160401/31b76c24/attachment.sig>
#2 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Hi Simon, Hi Tom!

Quoted 9 lines · Tom Rini
Quoted 5 lines · Simon Glass

Command parsing and processing code is not needed when the command line is disabled. Remove this code in that case.

Signed-off-by Simon Glass <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

I just noticed that this patch breaks U-Boot on some boards. All that don't have CONFIG_SYS_HUSH_PARSER or CONFIG_CMDLINE enabled (e.g. some MVEBU board, like the "db-mv784mp-gp".

How should this be solved? Should we enable CONFIG_CMDLINE per default (via config_defaults.h ?)? Or should we enable CONFIG_CMDLINE in all board config headers that are currently missing it?

Thanks,
Stefan

author
#3 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Hi Stefan,

Quoted 9 lines · Stefan Roese

Hi Simon, Hi Tom!

Quoted 4 lines · Tom Rini
Quoted 1 line · Simon Glass

Command parsing and processing code is not needed when the command line

is

Quoted 9 lines
Quoted 7 lines
Quoted 4 lines

disabled. Remove this code in that case.

Signed-off-by Simon Glass <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Applied to u-boot/master, thanks!

Quoted 7 lines

I just noticed that this patch breaks U-Boot on some boards. All that don't have CONFIG_SYS_HUSH_PARSER or CONFIG_CMDLINE enabled (e.g. some MVEBU board, like the "db-mv784mp-gp".

How should this be solved? Should we enable CONFIG_CMDLINE per default (via config_defaults.h ?)? Or should we enable CONFIG_CMDLINE in all board config headers that are currently missing it?

All boards should have CONFIG_CMDLINE enables by default. Is the problem with the build or at run-time?

Regards,
Simon

#4 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Hi Simon,

Quoted 24 lines · Simon Glass
Quoted 9 lines · On Apr 4, 2016 03:16, "Stefan Roese" <sr at denx.de <mailto:[email protected]>> wrote:

Hi Simon, Hi Tom!

Quoted 4 lines · Tom Rini
Quoted 1 line · Simon Glass

Command parsing and processing code is not needed when the command

line is

Quoted 13 lines
Quoted 7 lines
Quoted 4 lines

disabled. Remove this code in that case.

Signed-off-by Simon Glass <sjg at chromium.org <mailto:[email protected]>>
Reviewed-by Tom Rini <trini at konsulko.com <mailto:[email protected]>>

Applied to u-boot/master, thanks!

I just noticed that this patch breaks U-Boot on some boards. All that don't have CONFIG_SYS_HUSH_PARSER or CONFIG_CMDLINE enabled (e.g. some MVEBU board, like the "db-mv784mp-gp".

Quoted 5 lines
Quoted 3 lines

How should this be solved? Should we enable CONFIG_CMDLINE per default (via config_defaults.h ?)? Or should we enable CONFIG_CMDLINE in all board config headers that are currently missing it?

All boards should have CONFIG_CMDLINE enables by default.

That's currently not the case. At least some MVEBU (Armada XP / 38x) boards don't have it enabled. Here a short (most likely incomplete) list:

clearfog
db-88f6820-gp
db-mv784mp-gp_defconfig
ds414
maxbcm

And probably some (most?) Orion & Kirkwood based board as well. I assume that most boards that include "mv-common.h" have it not enabled (I didn't check all of them).

Some board, like theadorable have HUSH support enabled but CMDLINE not. These boards are not broken.

Quoted 2 lines

Is the problem
with the build or at run-time?

Run-time.

Thanks,
Stefan

author
#5 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Hi Stefan,

Quoted 33 lines · Stefan Roese

Hi Simon,

Quoted 29 lines · Simon Glass
Quoted 9 lines · On Apr 4, 2016 03:16, "Stefan Roese" <sr at denx.de <mailto:[email protected]>> wrote:

Hi Simon, Hi Tom!

Quoted 4 lines · Tom Rini
Quoted 1 line · Simon Glass

Command parsing and processing code is not needed when the command

line is

Quoted 16 lines
Quoted 7 lines
Quoted 4 lines

disabled. Remove this code in that case.

Signed-off-by Simon Glass <sjg at chromium.org <mailto:[email protected]>>
Reviewed-by Tom Rini <trini at konsulko.com <mailto:[email protected]>>

Applied to u-boot/master, thanks!

I just noticed that this patch breaks U-Boot on some boards. All that don't have CONFIG_SYS_HUSH_PARSER or CONFIG_CMDLINE enabled (e.g. some MVEBU board, like the "db-mv784mp-gp".

How should this be solved? Should we enable CONFIG_CMDLINE per default (via config_defaults.h ?)? Or should we enable CONFIG_CMDLINE in all board config headers that are currently missing it?

Quoted 8 lines
Quoted 1 line

All boards should have CONFIG_CMDLINE enables by default.

That's currently not the case. At least some MVEBU (Armada XP / 38x) boards don't have it enabled. Here a short (most likely incomplete) list:

clearfog

$ crosfw -b clearfog -w
$ grep CMDLINE b/clearfog/.config
CONFIG_CMDLINE=y

Also see cmd/Kconfig, where is defaults to y.

Quoted 9 lines

db-88f6820-gp
db-mv784mp-gp_defconfig
ds414
maxbcm

And probably some (most?) Orion & Kirkwood based board as well. I assume that most boards that include "mv-common.h" have it not enabled (I didn't check all of them).

Quoted 8 lines

Some board, like theadorable have HUSH support enabled but CMDLINE not. These boards are not broken.

Quoted 2 lines

Is the problem
with the build or at run-time?

Run-time.

OK so I wonder if the problem is not the option itself but something in the command line code for the non-hush parser? If you enable hush does it work? What exactly is the failure?

Regards,
Simon

#6 lore raw

PATCH v3 5/9 Drop command-processing code when CONFIG_CMDLINE is disabled

Hi Simon,

Quoted 41 lines · Simon Glass
Quoted 40 lines · Stefan Roese

Hi Simon,

Quoted 25 lines · Simon Glass
Quoted 9 lines · On Apr 4, 2016 03:16, "Stefan Roese" <sr at denx.de <mailto:[email protected]>> wrote:

Hi Simon, Hi Tom!

Quoted 4 lines · Tom Rini
Quoted 1 line · Simon Glass

Command parsing and processing code is not needed when the command

line is

Quoted 13 lines
Quoted 7 lines
Quoted 4 lines

disabled. Remove this code in that case.

Signed-off-by Simon Glass <sjg at chromium.org <mailto:[email protected]>>
Reviewed-by Tom Rini <trini at konsulko.com <mailto:[email protected]>>

Applied to u-boot/master, thanks!

I just noticed that this patch breaks U-Boot on some boards. All that don't have CONFIG_SYS_HUSH_PARSER or CONFIG_CMDLINE enabled (e.g. some MVEBU board, like the "db-mv784mp-gp".

Quoted 5 lines
Quoted 3 lines

How should this be solved? Should we enable CONFIG_CMDLINE per default (via config_defaults.h ?)? Or should we enable CONFIG_CMDLINE in all board config headers that are currently missing it?

All boards should have CONFIG_CMDLINE enables by default.

That's currently not the case. At least some MVEBU (Armada XP / 38x) boards don't have it enabled. Here a short (most likely incomplete) list:

Quoted 7 lines
Quoted 1 line

clearfog

$ crosfw -b clearfog -w
$ grep CMDLINE b/clearfog/.config
CONFIG_CMDLINE=y

Also see cmd/Kconfig, where is defaults to y.

I didn't notice this.

Quoted 16 lines
Quoted 16 lines

db-88f6820-gp
db-mv784mp-gp_defconfig
ds414
maxbcm

And probably some (most?) Orion & Kirkwood based board as well. I assume that most boards that include "mv-common.h" have it not enabled (I didn't check all of them).

Some board, like theadorable have HUSH support enabled but CMDLINE not. These boards are not broken.

Quoted 2 lines

Is the problem
with the build or at run-time?

Quoted 5 lines
Quoted 1 line

Run-time.

OK so I wonder if the problem is not the option itself but something in the command line code for the non-hush parser? If you enable hush does it work? What exactly is the failure?

Its a reboot after this message:

## U-Boot command line is disabled. Please enable CONFIG_CMDLINE

And I've just found the problem. Its a typo:

#ifdef CONFIG_SYS_HUSH_PARSER
	parse_file_outer();
	/* This point is never reached */
	for (;;);

#elif defined(CONFIG_CMDINE)

CMDLINE

Notice the missing "L" above.

I'll send a patch in a minute. Stay tuned...

Thanks,
Stefan