| Message ID | 1458375522-23219-6-git-send-email-sjg@chromium.org |
|---|---|
| State | Accepted |
| Commit | f8bb69643550fccbf1df514deda53089da7940e3 |
| Delegated to: | Tom Rini |
| Headers | show |
On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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> Applied to u-boot/master, thanks!
Hi Simon, Hi Tom! On 02.04.2016 03:55, Tom Rini wrote: > On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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> > > 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
Hi Stefan, On Apr 4, 2016 03:16, "Stefan Roese" <sr@denx.de> wrote: > > Hi Simon, Hi Tom! > > > On 02.04.2016 03:55, Tom Rini wrote: >> >> On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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> >> >> >> 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? All boards should have CONFIG_CMDLINE enables by default. Is the problem with the build or at run-time? Regards, Simon
Hi Simon, On 04.04.2016 15:18, Simon Glass wrote: > On Apr 4, 2016 03:16, "Stefan Roese" <sr@denx.de <mailto:sr@denx.de>> wrote: > > > > Hi Simon, Hi Tom! > > > > > > On 02.04.2016 03:55, Tom Rini wrote: > >> > >> On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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 <mailto:sjg@chromium.org>> > >>> Reviewed-by: Tom Rini <trini@konsulko.com <mailto:trini@konsulko.com>> > >> > >> > >> 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? > > 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. > Is the problem > with the build or at run-time? Run-time. Thanks, Stefan
Hi Stefan, On 5 April 2016 at 01:38, Stefan Roese <sr@denx.de> wrote: > > Hi Simon, > > On 04.04.2016 15:18, Simon Glass wrote: >> >> On Apr 4, 2016 03:16, "Stefan Roese" <sr@denx.de <mailto:sr@denx.de>> wrote: >> > >> > Hi Simon, Hi Tom! >> > >> > >> > On 02.04.2016 03:55, Tom Rini wrote: >> >> >> >> On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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 <mailto:sjg@chromium.org>> >> >>> Reviewed-by: Tom Rini <trini@konsulko.com <mailto:trini@konsulko.com>> >> >> >> >> >> >> 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? >> >> 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. > 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. > >> 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
Hi Simon, On 04.04.2016 16:05, Simon Glass wrote: > On 5 April 2016 at 01:38, Stefan Roese <sr@denx.de> wrote: >> >> Hi Simon, >> >> On 04.04.2016 15:18, Simon Glass wrote: >>> >>> On Apr 4, 2016 03:16, "Stefan Roese" <sr@denx.de <mailto:sr@denx.de>> wrote: >>> > >>> > Hi Simon, Hi Tom! >>> > >>> > >>> > On 02.04.2016 03:55, Tom Rini wrote: >>> >> >>> >> On Sat, Mar 19, 2016 at 02:18:38AM -0600, 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 <mailto:sjg@chromium.org>> >>> >>> Reviewed-by: Tom Rini <trini@konsulko.com <mailto:trini@konsulko.com>> >>> >> >>> >> >>> >> 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? >>> >>> 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. I didn't notice this. >> 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. >> >>> 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? 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
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..5e17da8 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[]) diff --git a/include/command.h b/include/command.h index 0524c0b..0d2b653 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__ */ /*