Each struct cmd_tbl entry carries a ->cmd_rep function pointer used to report whether a command may be repeated by pressing Enter at an empty prompt. For all but one command it points at cmd_always_repeatable() or cmd_never_repeatable(), trampolines that just set the repeatable flag and call cmd_invoke() - so a whole pointer per command encodes a single bit, on every board.
Replace it with a CMDF_REPEATABLE flag in the existing cmd_flags byte. The _rep argument of the U_BOOT_CMD*() macros now sets this bit, and cmd_call() reads it directly before invoking ->cmd, dropping the trampolines entirely. Sub-command dispatchers (U_BOOT_CMD_WITH_SUBCMDS, and the x86 zboot command) still need to report the repeatability of the sub-command they pick, which they do through the extended (..., int *repeatable) signature. Keep that ability with a CMDF_SUBCMD_REP flag: such a command stores its dispatcher in ->cmd and is tagged CMDF_SUBCMD_REP, and cmd_call() calls it with the repeatable pointer. The dispatcher refines *repeatable from the chosen sub-command's CMDF_REPEATABLE bit. This removes a pointer from every struct cmd_tbl entry: the struct shrinks from 56 to 48 bytes on 64-bit and by 4 bytes per entry on 32-bit, with no change in behaviour and no new config option. On turris_1x_nor (ppc32, ~100 commands) this recovers about 670 bytes, enough to keep the board within its NOR section after the getopt conversion. Signed-off-by: Simon Glass <[email protected]> --- Changes in v4: - Add new patch to replace the ->cmd_rep pointer with a CMDF_REPEATABLE flag, dropping a pointer from every command entry cmd/help.c | 2 +- cmd/test.c | 2 +- cmd/x86/zboot.c | 2 +- common/command.c | 44 +++++++------------- include/command.h | 104 ++++++++++++++++++++++------------------------ 5 files changed, 67 insertions(+), 87 deletions(-) diff --git a/cmd/help.c b/cmd/help.c index b87c19003d4..ab6f9c95739 100644 --- a/cmd/help.c +++ b/cmd/help.c @@ -28,7 +28,7 @@ U_BOOT_CMD( * nor can we rely on the CONFIG_SYS_LONGHELP helper macro */ ll_entry_declare(struct cmd_tbl, question_mark, cmd) = { - "?", CONFIG_SYS_MAXARGS, 0, cmd_always_repeatable, do_help, + "?", CONFIG_SYS_MAXARGS, CMDF_REPEATABLE, do_help, "alias for 'help'", #ifdef CONFIG_SYS_LONGHELP "" diff --git a/cmd/test.c b/cmd/test.c index f0b89aa0af0..32c5238972d 100644 --- a/cmd/test.c +++ b/cmd/test.c @@ -244,7 +244,7 @@ U_BOOT_CMD( * This does not use the U_BOOT_CMD macro as [ can't be used in symbol names */ ll_entry_declare(struct cmd_tbl, lbracket, cmd) = { - "[", CONFIG_SYS_MAXARGS, 0, cmd_always_repeatable, do_test, + "[", CONFIG_SYS_MAXARGS, CMDF_REPEATABLE, do_test, "alias for 'test'", #ifdef CONFIG_SYS_LONGHELP " <test expression> ]" diff --git a/cmd/x86/zboot.c b/cmd/x86/zboot.c index 3876d163236..ef3e27e622d 100644 --- a/cmd/x86/zboot.c +++ b/cmd/x86/zboot.c @@ -165,7 +165,7 @@ int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc, return CMD_RET_FAILURE; } -U_BOOT_CMDREP_COMPLETE( +U_BOOT_SUBCMD_DECL( zboot, 8, do_zboot_parent, "Boot bzImage", "[addr] [size] [initrd addr] [initrd size] [setup] [cmdline]\n" " addr - The optional starting address of the bzimage.\n" diff --git a/common/command.c b/common/command.c index c747bef7883..4204c6e2df2 100644 --- a/common/command.c +++ b/common/command.c @@ -508,11 +508,6 @@ void fixup_cmdtable(struct cmd_tbl *cmdtp, int size) for (i = 0; i < size; i++) { ulong addr; - addr = (ulong)(cmdtp->cmd_rep) + gd->reloc_off; - cmdtp->cmd_rep = - (int (*)(struct cmd_tbl *, int, int, - char * const [], int *))addr; - addr = (ulong)(cmdtp->cmd) + gd->reloc_off; #ifdef DEBUG_COMMANDS printf("Command \"%s\": 0x%08lx => 0x%08lx\n", @@ -558,30 +553,6 @@ int cmd_invoke(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return cmdtp->cmd(cmdtp, flag, argc, argv); } -int cmd_always_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[], int *repeatable) -{ - *repeatable = 1; - - return cmd_invoke(cmdtp, flag, argc, argv); -} - -int cmd_never_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[], int *repeatable) -{ - *repeatable = 0; - - return cmd_invoke(cmdtp, flag, argc, argv); -} - -int cmd_discard_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]) -{ - int repeatable; - - return cmdtp->cmd_rep(cmdtp, flag, argc, argv, &repeatable); -} - /** * Call a command function. This should be the only route in U-Boot to call * a command, so that we can track whether we are waiting for input or @@ -599,7 +570,20 @@ static int cmd_call(struct cmd_tbl *cmdtp, int flag, int argc, { int result; - result = cmdtp->cmd_rep(cmdtp, flag, argc, argv, repeatable); + *repeatable = !!(cmdtp->cmd_flags & CMDF_REPEATABLE); + + if (cmdtp->cmd_flags & CMDF_SUBCMD_REP) { + /* + * A sub-command dispatcher refines *repeatable for the + * sub-command it picks, so call it with the extended signature. + */ + cmd_rep_func_t func = (cmd_rep_func_t)cmdtp->cmd; + + result = func(cmdtp, flag, argc, argv, repeatable); + } else { + result = cmd_invoke(cmdtp, flag, argc, argv); + } + if (result) debug("Command failed, result=%d\n", result); return result; diff --git a/include/command.h b/include/command.h index 495d6a07d75..b201100870a 100644 --- a/include/command.h +++ b/include/command.h @@ -39,16 +39,6 @@ struct cmd_tbl { char *name; /* Command Name */ short maxargs; /* maximum number of arguments */ u8 cmd_flags; /* CMDF_... flags */ - /* - * Same as ->cmd() except the command - * tells us if it can be repeated. - * Replaces the old ->repeatable field - * which was not able to make - * repeatable property different for - * the main command and sub-commands. - */ - int (*cmd_rep)(struct cmd_tbl *cmd, int flags, int argc, - char *const argv[], int *repeatable); /* Implementation function */ int (*cmd)(struct cmd_tbl *cmd, int flags, int argc, char *const argv[]); @@ -76,8 +66,24 @@ enum { * (int (*)(struct getopt_state *)) instead of the classic one */ CMDF_GETOPT = BIT(0), + /* the command may be repeated by pressing Enter at an empty prompt */ + CMDF_REPEATABLE = BIT(1), + /* + * ->cmd holds a sub-command dispatcher with the extended signature + * (..., int *repeatable), so it can report the repeatable property of + * the sub-command it dispatched to. cmd_call() calls it accordingly. + */ + CMDF_SUBCMD_REP = BIT(2), }; +/* + * Type of a sub-command dispatcher (a CMDF_SUBCMD_REP command's ->cmd). It has + * the classic command arguments plus a @repeatable output which it sets from + * the sub-command it dispatches to. + */ +typedef int (*cmd_rep_func_t)(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[], int *repeatable); + /** * cmd_arg_get() - Get a particular argument * @@ -142,17 +148,9 @@ int cmd_usage(const struct cmd_tbl *cmdtp); */ int cmd_invoke(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); -/* Dummy ->cmd and ->cmd_rep wrappers. */ -int cmd_always_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[], int *repeatable); -int cmd_never_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[], int *repeatable); -int cmd_discard_repeatable(struct cmd_tbl *cmdtp, int flag, int argc, - char *const argv[]); - static inline bool cmd_is_repeatable(struct cmd_tbl *cmdtp) { - return cmdtp->cmd_rep == cmd_always_repeatable; + return cmdtp->cmd_flags & CMDF_REPEATABLE; } #ifdef CONFIG_AUTO_COMPLETE @@ -416,8 +414,9 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); !cmd_is_repeatable(subcmd)) \ return CMD_RET_SUCCESS; \ \ - return subcmd->cmd_rep(subcmd, flag, argc - 1, \ - argv + 1, repeatable); \ + *repeatable &= cmd_is_repeatable(subcmd); \ + \ + return cmd_invoke(subcmd, flag, argc - 1, argv + 1); \ } #ifdef CONFIG_AUTO_COMPLETE @@ -441,21 +440,17 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); U_BOOT_SUBCMDS_COMPLETE(_cmdname) #if CONFIG_IS_ENABLED(CMDLINE) -#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ - _usage, _help, _comp) \ - { #_name, _maxargs, 0, _cmd_rep, cmd_discard_repeatable, \ - _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } +/* Encode the _rep argument as the CMDF_REPEATABLE flag bit */ +#define _CMD_REP_FLAG(_rep) ((_rep) ? CMDF_REPEATABLE : 0) #define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp) \ - { #_name, _maxargs, 0, \ - _rep ? cmd_always_repeatable : cmd_never_repeatable, \ + { #_name, _maxargs, _CMD_REP_FLAG(_rep), \ _cmd, _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } #define U_BOOT_CMD_MKENT_GETOPT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp) \ - { #_name, _maxargs, CMDF_GETOPT, \ - _rep ? cmd_always_repeatable : cmd_never_repeatable, \ + { #_name, _maxargs, CMDF_GETOPT | _CMD_REP_FLAG(_rep), \ (int (*)(struct cmd_tbl *, int, int, \ char *const []))(_cmd), \ _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } @@ -480,11 +475,18 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); U_BOOT_CMD_MKENT_GETOPT_COMPLETE(_name, _maxargs, _rep, \ _cmd, _usage, _help, _comp) -#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ - _help, _comp) \ - ll_entry_declare(struct cmd_tbl, _name, cmd) = \ - U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ - _usage, _help, _comp) +/* + * Declare a top-level command whose ->cmd is a sub-command dispatcher with + * the extended (..., int *repeatable) signature (see CMDF_SUBCMD_REP). The + * dispatcher is stored in ->cmd and tagged so cmd_call() invokes it with the + * repeatable pointer. + */ +#define U_BOOT_SUBCMD_DECL(_name, _maxargs, _do_cmd, _usage, _help, _comp) \ + ll_entry_declare(struct cmd_tbl, _name, cmd) = { \ + #_name, _maxargs, CMDF_SUBCMD_REP | CMDF_REPEATABLE, \ + (int (*)(struct cmd_tbl *, int, int, \ + char *const []))(_do_cmd), \ + _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } #else #define U_BOOT_SUBCMD_START(name) static struct cmd_tbl name[] = {}; @@ -498,14 +500,6 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); return 0; \ } -#define _CMD_REMOVE_REP(_name, _cmd) \ - int __remove_ ## _name(void) \ - { \ - if (0) \ - _cmd(NULL, 0, 0, NULL, NULL); \ - return 0; \ - } - #define _CMD_REMOVE_GETOPT(_name, _cmd) \ int __remove_ ## _name(void) \ { \ @@ -514,19 +508,14 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); return 0; \ } -#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ - _usage, _help, _comp) \ - { #_name, _maxargs, 0, 0 ? _cmd_rep : NULL, NULL, _usage, \ - _CMD_HELP(_help) _CMD_COMPLETE(_comp) } - #define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, _usage, \ _help, _comp) \ - { #_name, _maxargs, 0, NULL, 0 ? _cmd : NULL, _usage, \ + { #_name, _maxargs, 0, 0 ? _cmd : NULL, _usage, \ _CMD_HELP(_help) _CMD_COMPLETE(_comp) } #define U_BOOT_CMD_MKENT_GETOPT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp) \ - { #_name, _maxargs, CMDF_GETOPT, NULL, \ + { #_name, _maxargs, CMDF_GETOPT, \ 0 ? (int (*)(struct cmd_tbl *, int, int, \ char *const []))(_cmd) : NULL, _usage, \ _CMD_HELP(_help) _CMD_COMPLETE(_comp) } @@ -547,9 +536,16 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); _help, _comp) \ _CMD_REMOVE_GETOPT(sub_ ## _name, _cmd) -#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ - _help, _comp) \ - _CMD_REMOVE_REP(sub_ ## _name, _cmd_rep) +#define _CMD_REMOVE_REP(_name, _cmd) \ + int __remove_ ## _name(void) \ + { \ + if (0) \ + _cmd(NULL, 0, 0, NULL, NULL); \ + return 0; \ + } + +#define U_BOOT_SUBCMD_DECL(_name, _maxargs, _do_cmd, _usage, _help, _comp) \ + _CMD_REMOVE_REP(sub_ ## _name, _do_cmd) #endif /* CONFIG_CMDLINE */ @@ -571,7 +567,7 @@ int cmd_source_script(ulong addr, const char *fit_uname, const char *confname); #define U_BOOT_CMD_WITH_SUBCMDS(_name, _usage, _help, ...) \ U_BOOT_SUBCMDS(_name, __VA_ARGS__) \ - U_BOOT_CMDREP_COMPLETE(_name, CONFIG_SYS_MAXARGS, do_##_name, \ - _usage, _help, complete_##_name) + U_BOOT_SUBCMD_DECL(_name, CONFIG_SYS_MAXARGS, do_##_name, \ + _usage, _help, complete_##_name) #endif /* __COMMAND_H */ -- 2.43.0
