[U-Boot] [PATCH 0/9] Add CONFIG_CMDLINE to allow removal of all commands

A large chunk of the U-Boot code is its wide variety of commands. For some applications this is not needed, since the boot can be controlled by a board-specific hard-coded boot procedure.
Any attempt to use commands, such as running script, will result in an error. U-Boot acts as if it supports commands in general, but there are no actual commands to run.
This saves a significant amount of space.
For example on snow this saves 300KB of code space, with the code size dropping from 505KB to 202KB. BSS drops from 238KB to 4KB. The size of u-boot.bin (which includes the device tree) drops from 541KB to 228KB.
If LCD and USB support is disabled on this platform, code size drops to 139KB.
This makes U-Boot proper look a little more like SPL in terms of size.
The CONFIG_CMDLINE setting is enabled by default, but can be disabled by boards as needed.
Simon Glass (9): cbfs: Update a function to be static Add an option to enable the command line arm: x86: Drop command-line code when CONFIG_CMDLINE is disabled sandbox: Avoid calling commands when not available Drop command-processing code when CONFIG_CMDLINE is disabled Hang when no command line processing can be performed Allow command code to compile to nothing Allow command-line files to be dropped Drop various features when the command line is not available
README | 8 ++++++++ arch/arm/cpu/u-boot.lds | 3 +++ arch/sandbox/cpu/start.c | 10 +++++++++- arch/x86/cpu/u-boot.lds | 4 ++++ cmd/Kconfig | 12 ++++++++++++ cmd/cbfs.c | 12 ++++++++---- cmd/help.c | 4 ++++ common/Makefile | 4 ++-- common/cli.c | 17 +++++++++++++++- common/command.c | 6 ++++++ common/main.c | 1 + include/command.h | 49 ++++++++++++++++++++++++++++++++++++++++++---- include/config_fallbacks.h | 10 ++++++++++ 13 files changed, 128 insertions(+), 12 deletions(-)

All command functions should be static. Update the CBFS functions to follow this rule.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/cbfs.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/cmd/cbfs.c b/cmd/cbfs.c index 35d8a7a..1faf78e 100644 --- a/cmd/cbfs.c +++ b/cmd/cbfs.c @@ -11,7 +11,8 @@ #include <command.h> #include <cbfs.h>
-int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +static int do_cbfs_init(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) { uintptr_t end_of_rom = 0xffffffff; char *ep; @@ -44,7 +45,8 @@ U_BOOT_CMD( " CBFS is in. It defaults to 0xFFFFFFFF\n" );
-int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +static int do_cbfs_fsload(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) { const struct cbfs_cachenode *file; unsigned long offset; @@ -90,7 +92,8 @@ U_BOOT_CMD( " - load binary file 'filename' from the cbfs to address 'addr'\n" );
-int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +static int do_cbfs_ls(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) { const struct cbfs_cachenode *file = file_cbfs_get_first(); int files = 0; @@ -167,7 +170,8 @@ U_BOOT_CMD( " - list the files in the cbfs\n" );
-int do_cbfs_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]) +static int do_cbfs_fsinfo(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) { const struct cbfs_header *header = file_cbfs_get_header();

On Thu, Feb 25, 2016 at 09:00:48PM -0700, Simon Glass wrote:
All command functions should be static. Update the CBFS functions to follow this rule.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Add a new Kconfig option for the command line. This is enabled by default, but when disabled it will remove the command line.
Signed-off-by: Simon Glass sjg@chromium.org ---
README | 8 ++++++++ cmd/Kconfig | 12 ++++++++++++ 2 files changed, 20 insertions(+)
diff --git a/README b/README index 362ff19..a6d7935 100644 --- a/README +++ b/README @@ -1108,6 +1108,14 @@ The following options need to be configured:
XXX - this list needs to get updated!
+- Removal of commands + If no commands are needed to boot, you can disable + CONFIG_CMDLINE to remove them. In this case, the command line + will not be available, and when U-Boot wants to execute the + boot command (on start-up) it will call board_run_command() + instead. This can reduce image size significantly for very + simple boot procedures. + - Regular expression support: CONFIG_REGEX If this variable is defined, U-Boot is linked against diff --git a/cmd/Kconfig b/cmd/Kconfig index 2ed0263..8ead967 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1,8 +1,20 @@ menu "Command line interface"
+config CMDLINE + bool "Support U-Boot commands" + default y + help + Enable U-Boot's command-line functions. This provides a means + to enter commands into U-Boot for a wide variety of purposes. It + also allows scripts (containing commands) to be executed. + Various commands and command categorys can be indivdually enabled. + Depending on the number of commands enabled, this can add + substantially to the size of U-Boot. + config HUSH_PARSER bool "Use hush shell" select SYS_HUSH_PARSER + depends on CMDLINE help This option enables the "hush" shell (from Busybox) as command line interpreter, thus enabling powerful command line syntax like

On Thu, Feb 25, 2016 at 09:00:49PM -0700, Simon Glass wrote:
Add a new Kconfig option for the command line. This is enabled by default, but when disabled it will remove the command line.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Update the link script to drop this code when not needed. This is only done for two architectures at present.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/arm/cpu/u-boot.lds | 3 +++ arch/x86/cpu/u-boot.lds | 4 ++++ 2 files changed, 7 insertions(+)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index e148ab7..ebea070 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -14,6 +14,9 @@ OUTPUT_ARCH(arm) ENTRY(_start) SECTIONS { +#ifndef CONFIG_CMDLINE + /DISCARD/ : { *(.u_boot_list_2_cmd_*) } +#endif #if defined(CONFIG_ARMV7_SECURE_BASE) && defined(CONFIG_ARMV7_NONSEC) /* * If CONFIG_ARMV7_SECURE_BASE is true, secure code will not diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds index b0d8531..36f59ea 100644 --- a/arch/x86/cpu/u-boot.lds +++ b/arch/x86/cpu/u-boot.lds @@ -12,6 +12,10 @@ ENTRY(_start)
SECTIONS { +#ifndef CONFIG_CMDLINE + /DISCARD/ : { *(.u_boot_list_2_cmd_*) } +#endif + . = CONFIG_SYS_TEXT_BASE; /* Location of bootcode in flash */ __text_start = .; .text : { *(.text*); }

On Thu, Feb 25, 2016 at 09:00:50PM -0700, Simon Glass wrote:
Update the link script to drop this code when not needed. This is only done for two architectures at present.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Don't try to run commands when not supported.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/sandbox/cpu/start.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 0dda4fc..969618e 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -83,13 +83,14 @@ int sandbox_main_loop_init(void)
cli_init();
+#ifdef CONFIG_CMDLINE if (state->cmd) retval = run_command_list(state->cmd, -1, 0);
if (state->run_distro_boot) retval = cli_simple_run_command("run distro_bootcmd", 0); - +#endif if (!state->interactive) os_exit(retval); } @@ -265,6 +266,13 @@ static int sandbox_cmdline_cb_verbose(struct sandbox_state *state, } SANDBOX_CMDLINE_OPT_SHORT(verbose, 'v', 0, "Show test output");
+int board_run_command(const char *cmdline) +{ + printf("## Commands are disabled. Please enable CONFIG_CMDLINE.\n"); + + return 1; +} + int main(int argc, char *argv[]) { struct sandbox_state *state;

On Thu, Feb 25, 2016 at 09:00:51PM -0700, Simon Glass wrote:
Don't try to run commands when not supported.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On 02/25/2016 09:00 PM, Simon Glass wrote:
Don't try to run commands when not supported.
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
+int board_run_command(const char *cmdline) +{
- printf("## Commands are disabled. Please enable CONFIG_CMDLINE.\n");
- return 1;
+}
Isn't this part of patch 5/9?

Hi Stephen,
On 29 February 2016 at 16:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
Don't try to run commands when not supported.
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
+int board_run_command(const char *cmdline) +{
printf("## Commands are disabled. Please enable
CONFIG_CMDLINE.\n");
return 1;
+}
Isn't this part of patch 5/9?
No - this is the sandbox support. That patch merely declares the function in a header. Any board can implement it, and each would do so in a separate patch.
Regards, Simon

On 03/02/2016 05:25 PM, Simon Glass wrote:
Hi Stephen,
On 29 February 2016 at 16:39, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
Don't try to run commands when not supported.
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
+int board_run_command(const char *cmdline) +{
printf("## Commands are disabled. Please enable
CONFIG_CMDLINE.\n");
return 1;
+}
Isn't this part of patch 5/9?
No - this is the sandbox support. That patch merely declares the function in a header. Any board can implement it, and each would do so in a separate patch.
OK, but this patch implements the function before there's a prototype for it in the header. That doesn't seem like the correct order.

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 sjg@chromium.org ---
cmd/help.c | 4 ++++ common/cli.c | 17 ++++++++++++++++- common/command.c | 6 ++++++ 3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/cmd/help.c b/cmd/help.c index 6ff494d..701ae7e 100644 --- a/cmd/help.c +++ b/cmd/help.c @@ -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( diff --git a/common/cli.c b/common/cli.c index 119d282..9b4b3e5 100644 --- a/common/cli.c +++ b/common/cli.c @@ -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*/ }
diff --git a/common/command.c b/common/command.c index 858e288..e5d9b9c 100644 --- a/common/command.c +++ b/common/command.c @@ -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[])

On Thu, Feb 25, 2016 at 09:00:52PM -0700, Simon Glass wrote:
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 sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

Normally board_run_command() will handle command processed. But if for some reason it returns then we should hang to avoid further processing.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/main.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/common/main.c b/common/main.c index 1a2ef39..a9f7f71 100644 --- a/common/main.c +++ b/common/main.c @@ -72,4 +72,5 @@ void main_loop(void) autoboot_command(s);
cli_loop(); + hang(); }

On Thu, Feb 25, 2016 at 09:00:53PM -0700, Simon Glass wrote:
Normally board_run_command() will handle command processed. But if for some reason it returns then we should hang to avoid further processing.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
... but can we maybe try and force this to happen as an experiment and see if panic here?

Hi Tom,
On 26 February 2016 at 10:17, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 25, 2016 at 09:00:53PM -0700, Simon Glass wrote:
Normally board_run_command() will handle command processed. But if for some reason it returns then we should hang to avoid further processing.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
... but can we maybe try and force this to happen as an experiment and see if panic here?
Yes this comes up with sandbox when the option is enabled.
Regards, Simon

On Fri, Feb 26, 2016 at 10:31:36AM -0700, Simon Glass wrote:
Hi Tom,
On 26 February 2016 at 10:17, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 25, 2016 at 09:00:53PM -0700, Simon Glass wrote:
Normally board_run_command() will handle command processed. But if for some reason it returns then we should hang to avoid further processing.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
... but can we maybe try and force this to happen as an experiment and see if panic here?
Yes this comes up with sandbox when the option is enabled.
Sorry, I mean that instead of hang() we call panic("..something") ?

When CONFIG_CMDLINE is disabled we need to remove all the command-line code. Most can be removed by dropping the appropriate linker lists from the images, but sub-commands must be dealt with specially.
A simple mechanism is used to avoid 'unused static function' errors.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/command.h | 49 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/include/command.h b/include/command.h index 0524c0b..08f0486 100644 --- a/include/command.h +++ b/include/command.h @@ -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__ */
/* @@ -164,21 +182,44 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size); # define _CMD_HELP(x) #endif
+#ifdef CONFIG_CMDLINE #define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp) \ { #_name, _maxargs, _rep, _cmd, _usage, \ _CMD_HELP(_help) _CMD_COMPLETE(_comp) }
-#define U_BOOT_CMD_MKENT(_name, _maxargs, _rep, _cmd, _usage, _help) \ - U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ - _usage, _help, NULL) - #define U_BOOT_CMD_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, _help, _comp) \ ll_entry_declare(cmd_tbl_t, _name, cmd) = \ U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp);
+#else +#define U_BOOT_SUBCMD_START(name) static cmd_tbl_t name[] = {}; +#define U_BOOT_SUBCMD_END + +#define _CMD_REMOVE(_name, _cmd) \ + int __remove_ ## _name(void) \ + { \ + if (0) \ + _cmd(NULL, 0, 0, NULL); \ + return 0; \ + } +#define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, \ + _help, _comp) \ + { #_name, _maxargs, _rep, 0 ? _cmd : NULL, _usage, \ + _CMD_HELP(_help) _CMD_COMPLETE(_comp) } + +#define U_BOOT_CMD_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, _help, \ + _comp) \ + _CMD_REMOVE(sub_ ## _name, _cmd) + +#endif /* CONFIG_CMDLINE */ + #define U_BOOT_CMD(_name, _maxargs, _rep, _cmd, _usage, _help) \ U_BOOT_CMD_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, _help, NULL)
+#define U_BOOT_CMD_MKENT(_name, _maxargs, _rep, _cmd, _usage, _help) \ + U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ + _usage, _help, NULL) + #endif /* __COMMAND_H */

On Thu, Feb 25, 2016 at 09:00:54PM -0700, Simon Glass wrote:
When CONFIG_CMDLINE is disabled we need to remove all the command-line code. Most can be removed by dropping the appropriate linker lists from the images, but sub-commands must be dealt with specially.
A simple mechanism is used to avoid 'unused static function' errors.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On 02/25/2016 09:00 PM, Simon Glass wrote:
When CONFIG_CMDLINE is disabled we need to remove all the command-line code. Most can be removed by dropping the appropriate linker lists from the images, but sub-commands must be dealt with specially.
A simple mechanism is used to avoid 'unused static function' errors.
diff --git a/include/command.h b/include/command.h
+/**
- 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);
Doesn't this need to be part of patch 5/9 too, since the function is first called there?

Hi Stephen,
On 29 February 2016 at 16:40, Stephen Warren swarren@wwwdotorg.org wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
When CONFIG_CMDLINE is disabled we need to remove all the command-line code. Most can be removed by dropping the appropriate linker lists from the images, but sub-commands must be dealt with specially.
A simple mechanism is used to avoid 'unused static function' errors.
diff --git a/include/command.h b/include/command.h
+/**
- 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);
Doesn't this need to be part of patch 5/9 too, since the function is first called there?
See my comment elsewhere. This is just the function declaration - it is not necessarily called by any board. But if CONFIG_CMDLINE is disabled, this function will need to be supplied.
Regards, Simon

These files do not need to be compiled when CONFIG_CMDLINE is disabled. Update the Makefile to reflect this.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/Makefile b/common/Makefile index 117178a..b15113c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -148,10 +148,10 @@ endif endif
# We always have this since drivers/ddr/fs/interactive.c needs it -obj-y += cli_simple.o +obj-$(CONFIG_CMDLINE) += cli_simple.o
obj-y += cli.o -obj-y += cli_readline.o +obj-$(CONFIG_CMDLINE) += cli_readline.o obj-y += command.o obj-y += s_record.o obj-y += xyzModem.o

On Thu, Feb 25, 2016 at 09:00:55PM -0700, Simon Glass wrote:
These files do not need to be compiled when CONFIG_CMDLINE is disabled. Update the Makefile to reflect this.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
... but did you buildman the world here? iirc, we have some cases that fake running a command to make something important happen here.

Hi Tom,
On 26 February 2016 at 10:17, Tom Rini trini@konsulko.com wrote:
On Thu, Feb 25, 2016 at 09:00:55PM -0700, Simon Glass wrote:
These files do not need to be compiled when CONFIG_CMDLINE is disabled. Update the Makefile to reflect this.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com
... but did you buildman the world here? iirc, we have some cases that fake running a command to make something important happen here.
Not on this commit, but for the series. I will though.
It is definitely possible that enabling the option will create build errors. For one, you need a board_run_command() function to be present. I've tested it for sandbox and am actually thinking of adding a sandbox board without commands.
Regards, Simon

Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/config_fallbacks.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/include/config_fallbacks.h b/include/config_fallbacks.h index ddfe045..6b6ec00 100644 --- a/include/config_fallbacks.h +++ b/include/config_fallbacks.h @@ -97,4 +97,14 @@ # endif #endif
+#ifndef CONFIG_CMDLINE +#undef CONFIG_CMDLINE_EDITING +#undef CONFIG_SYS_LONGHELP +#undef CONFIG_CMD_BOOTD +#undef CONFIG_CMD_RUN +#undef CONFIG_SYS_HUSH_PARSER +#undef CONFIG_CMD_ASKENV +#undef CONFIG_MENU +#endif + #endif /* __CONFIG_FALLBACKS_H */

On Thu, Feb 25, 2016 at 09:00:56PM -0700, Simon Glass wrote:
Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On 02/25/2016 09:00 PM, Simon Glass wrote:
Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
How does this series affect the various code that executes other U-Boot functionality by executing commands rather than calling functions? For instance, drivers/dfu/dfu_mmc.c:mmc_file_op() calls run_command() to perform the actual disk I/O. I assume that is incompatible with enabling this new feature? If so, can Kconfig enforce that?

On Mon, Feb 29, 2016 at 04:47:19PM -0700, Stephen Warren wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
How does this series affect the various code that executes other U-Boot functionality by executing commands rather than calling functions? For instance, drivers/dfu/dfu_mmc.c:mmc_file_op() calls run_command() to perform the actual disk I/O. I assume that is incompatible with enabling this new feature? If so, can Kconfig enforce that?
This is what I was trying to get at as well. We may have to initially say that some stuff is dependent on CMDLINE || BROKEN or something as we have some even funkier cases for some FSL DDR things.

Hi,
On 29 February 2016 at 16:56, Tom Rini trini@konsulko.com wrote:
On Mon, Feb 29, 2016 at 04:47:19PM -0700, Stephen Warren wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
How does this series affect the various code that executes other U-Boot functionality by executing commands rather than calling functions? For instance, drivers/dfu/dfu_mmc.c:mmc_file_op() calls run_command() to perform the actual disk I/O. I assume that is incompatible with enabling this new feature? If so, can Kconfig enforce that?
This is what I was trying to get at as well. We may have to initially say that some stuff is dependent on CMDLINE || BROKEN or something as we have some even funkier cases for some FSL DDR things.
Yes, it will break it. I'd like to think we can move to cleaning this stuff up though.
IMO mmc_file_op() is wrong. It should work using function calls, not through the command-line interface. Perhaps that is hard today, and will get easier with driver model?
Regards, Simon

On Wed, Mar 02, 2016 at 05:25:27PM -0700, Simon Glass wrote:
Hi,
On 29 February 2016 at 16:56, Tom Rini trini@konsulko.com wrote:
On Mon, Feb 29, 2016 at 04:47:19PM -0700, Stephen Warren wrote:
On 02/25/2016 09:00 PM, Simon Glass wrote:
Some features are only useful or meaningful when the command line is present. Ensure that these features are not compiled in when CONFIG_CMDLINE is not enabled.
How does this series affect the various code that executes other U-Boot functionality by executing commands rather than calling functions? For instance, drivers/dfu/dfu_mmc.c:mmc_file_op() calls run_command() to perform the actual disk I/O. I assume that is incompatible with enabling this new feature? If so, can Kconfig enforce that?
This is what I was trying to get at as well. We may have to initially say that some stuff is dependent on CMDLINE || BROKEN or something as we have some even funkier cases for some FSL DDR things.
Yes, it will break it. I'd like to think we can move to cleaning this stuff up though.
IMO mmc_file_op() is wrong. It should work using function calls, not through the command-line interface. Perhaps that is hard today, and will get easier with driver model?
The DFU case at least was certainly more ugly at the time when doing function calls, yes. And to start with it's not a big deal to mark some things (even whole SoCs) as requiring CMDLINE.
participants (3)
-
Simon Glass
-
Stephen Warren
-
Tom Rini