Message ID | 20181203215423.18772-4-boris.brezillon@bootlin.com |
---|---|
State | Accepted |
Delegated to: | Tom Rini |
Headers | show |
Series | cmd: Simplify support for sub-commands | expand |
On Mon, Dec 03, 2018 at 10:54:20PM +0100, Boris Brezillon wrote: > The repeatable property is currently attached to the main command and > sub-commands have no way to change the repeatable value (the > ->repeatable field in sub-command entries is ignored). > > Replace the ->repeatable field by an extended ->cmd() hook (called > ->cmd_rep()) which takes a new int pointer to store the repeatable cap > of the command being executed. > > With this trick, we can let sub-commands decide whether they are > repeatable or not. > > We also patch mmc and dtimg who are testing the ->repeatable field > directly (they now use cmd_is_repeatable() instead), and fix the help > entry manually since it doesn't use the U_BOOT_CMD() macro. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > Reviewed-by: Tom Rini <trini@konsulko.com> > --- > Changes in v4: > -None > > Changes in v3: > - Add Tom's R-b > > Changes in v2: > - New patch > --- > cmd/dtimg.c | 2 +- > cmd/help.c | 2 +- > cmd/mmc.c | 4 ++-- > common/command.c | 36 ++++++++++++++++++++++++++++---- > include/command.h | 52 +++++++++++++++++++++++++++++++++++++++++++---- > 5 files changed, 84 insertions(+), 12 deletions(-) > > diff --git a/cmd/dtimg.c b/cmd/dtimg.c > index 65c8d101b98e..ae7d82f26dd2 100644 > --- a/cmd/dtimg.c > +++ b/cmd/dtimg.c > @@ -116,7 +116,7 @@ static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > if (!cp || argc > cp->maxargs) > return CMD_RET_USAGE; > - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) > + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) > return CMD_RET_SUCCESS; > > return cp->cmd(cmdtp, flag, argc, argv); > diff --git a/cmd/help.c b/cmd/help.c > index 503fa632e74e..fa2010c67eb1 100644 > --- a/cmd/help.c > +++ b/cmd/help.c > @@ -29,7 +29,7 @@ U_BOOT_CMD( > > /* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */ > ll_entry_declare(cmd_tbl_t, question_mark, cmd) = { > - "?", CONFIG_SYS_MAXARGS, 1, do_help, > + "?", CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_help, > "alias for 'help'", > #ifdef CONFIG_SYS_LONGHELP > "" > diff --git a/cmd/mmc.c b/cmd/mmc.c > index 3920a1836a59..42e38900df12 100644 > --- a/cmd/mmc.c > +++ b/cmd/mmc.c > @@ -247,7 +247,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag, > > if (cp == NULL || argc > cp->maxargs) > return CMD_RET_USAGE; > - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) > + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) > return CMD_RET_SUCCESS; > > mmc = init_mmc_device(curr_device, false); > @@ -907,7 +907,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > if (cp == NULL || argc > cp->maxargs) > return CMD_RET_USAGE; > - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) > + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) > return CMD_RET_SUCCESS; > > if (curr_device < 0) { > diff --git a/common/command.c b/common/command.c > index e13cb47ac18b..19f0534a76ea 100644 > --- a/common/command.c > +++ b/common/command.c > @@ -501,6 +501,30 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) > } > #endif > > +int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, > + char * const argv[], int *repeatable) > +{ > + *repeatable = 1; > + > + return cmdtp->cmd(cmdtp, flag, argc, argv); > +} > + > +int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, > + char * const argv[], int *repeatable) > +{ > + *repeatable = 0; > + > + return cmdtp->cmd(cmdtp, flag, argc, argv); > +} > + > +int cmd_discard_repeatable(cmd_tbl_t *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 > @@ -510,13 +534,15 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) > * @param flag Some flags normally 0 (see CMD_FLAG_.. above) > * @param argc Number of arguments (arg 0 must be the command text) > * @param argv Arguments > + * @param repeatable Can the command be repeated > * @return 0 if command succeeded, else non-zero (CMD_RET_...) > */ > -static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], > + int *repeatable) > { > int result; > > - result = (cmdtp->cmd)(cmdtp, flag, argc, argv); > + result = cmdtp->cmd_rep(cmdtp, flag, argc, argv, repeatable); > if (result) > debug("Command failed, result=%d\n", result); > return result; > @@ -553,12 +579,14 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], > > /* If OK so far, then do the command */ > if (!rc) { > + int newrep; > + > if (ticks) > *ticks = get_timer(0); > - rc = cmd_call(cmdtp, flag, argc, argv); > + rc = cmd_call(cmdtp, flag, argc, argv, &newrep); > if (ticks) > *ticks = get_timer(*ticks); > - *repeatable &= cmdtp->repeatable; > + *repeatable &= newrep; > } > if (rc == CMD_RET_USAGE) > rc = cmd_usage(cmdtp); > diff --git a/include/command.h b/include/command.h > index 89efcecfa926..bb93f022c514 100644 > --- a/include/command.h > +++ b/include/command.h > @@ -29,7 +29,16 @@ > struct cmd_tbl_s { > char *name; /* Command Name */ > int maxargs; /* maximum number of arguments */ > - int repeatable; /* autorepeat allowed? */ > + /* > + * 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_s *cmd, int flags, int argc, > + char * const argv[], int *repeatable); > /* Implementation function */ > int (*cmd)(struct cmd_tbl_s *, int, int, char * const []); > char *usage; /* Usage message (short) */ > @@ -60,6 +69,19 @@ int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc, > > extern int cmd_usage(const cmd_tbl_t *cmdtp); > > +/* Dummy ->cmd and ->cmd_rep wrappers. */ > +int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, > + char * const argv[], int *repeatable); > +int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, > + char * const argv[], int *repeatable); > +int cmd_discard_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, > + char * const argv[]); > + > +static inline bool cmd_is_repeatable(cmd_tbl_t *cmdtp) > +{ > + return cmdtp->cmd_rep == cmd_always_repeatable; > +} > + > #ifdef CONFIG_AUTO_COMPLETE > extern int var_complete(int argc, char * const argv[], char last_char, int maxv, char *cmdv[]); > extern int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp); > @@ -188,16 +210,28 @@ int board_run_command(const char *cmdline); > #endif > > #ifdef CONFIG_CMDLINE > +#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ > + _usage, _help, _comp) \ > + { #_name, _maxargs, _cmd_rep, cmd_discard_repeatable, \ > + _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } > + > #define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ > _usage, _help, _comp) \ > - { #_name, _maxargs, _rep, _cmd, _usage, \ > - _CMD_HELP(_help) _CMD_COMPLETE(_comp) } > + { #_name, _maxargs, \ > + _rep ? cmd_always_repeatable : cmd_never_repeatable, \ > + _cmd, _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } > > #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); > > +#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ > + _help, _comp) \ > + ll_entry_declare(cmd_tbl_t, _name, cmd) = \ > + U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ > + _usage, _help, _comp) > + > #else > #define U_BOOT_SUBCMD_START(name) static cmd_tbl_t name[] = {}; > #define U_BOOT_SUBCMD_END > @@ -209,15 +243,25 @@ int board_run_command(const char *cmdline); > _cmd(NULL, 0, 0, NULL); \ > return 0; \ > } > + > +#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ > + _usage, _help, _comp) \ > + { #_name, _maxargs, 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, _rep, 0 ? _cmd : NULL, _usage, \ > + { #_name, _maxargs, NULL, 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) > > +#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ > + _help, _comp) \ > + _CMD_REMOVE(sub_ ## _name, _cmd_rep) > + > #endif /* CONFIG_CMDLINE */ > > #define U_BOOT_CMD(_name, _maxargs, _rep, _cmd, _usage, _help) \
On Mon, Dec 03, 2018 at 10:54:20PM +0100, Boris Brezillon wrote: > The repeatable property is currently attached to the main command and > sub-commands have no way to change the repeatable value (the > ->repeatable field in sub-command entries is ignored). > > Replace the ->repeatable field by an extended ->cmd() hook (called > ->cmd_rep()) which takes a new int pointer to store the repeatable cap > of the command being executed. > > With this trick, we can let sub-commands decide whether they are > repeatable or not. > > We also patch mmc and dtimg who are testing the ->repeatable field > directly (they now use cmd_is_repeatable() instead), and fix the help > entry manually since it doesn't use the U_BOOT_CMD() macro. > > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > Reviewed-by: Tom Rini <trini@konsulko.com> Applied to u-boot/master, thanks!
diff --git a/cmd/dtimg.c b/cmd/dtimg.c index 65c8d101b98e..ae7d82f26dd2 100644 --- a/cmd/dtimg.c +++ b/cmd/dtimg.c @@ -116,7 +116,7 @@ static int do_dtimg(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!cp || argc > cp->maxargs) return CMD_RET_USAGE; - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) return CMD_RET_SUCCESS; return cp->cmd(cmdtp, flag, argc, argv); diff --git a/cmd/help.c b/cmd/help.c index 503fa632e74e..fa2010c67eb1 100644 --- a/cmd/help.c +++ b/cmd/help.c @@ -29,7 +29,7 @@ U_BOOT_CMD( /* This does not use the U_BOOT_CMD macro as ? can't be used in symbol names */ ll_entry_declare(cmd_tbl_t, question_mark, cmd) = { - "?", CONFIG_SYS_MAXARGS, 1, do_help, + "?", CONFIG_SYS_MAXARGS, cmd_always_repeatable, do_help, "alias for 'help'", #ifdef CONFIG_SYS_LONGHELP "" diff --git a/cmd/mmc.c b/cmd/mmc.c index 3920a1836a59..42e38900df12 100644 --- a/cmd/mmc.c +++ b/cmd/mmc.c @@ -247,7 +247,7 @@ static int do_mmcrpmb(cmd_tbl_t *cmdtp, int flag, if (cp == NULL || argc > cp->maxargs) return CMD_RET_USAGE; - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) return CMD_RET_SUCCESS; mmc = init_mmc_device(curr_device, false); @@ -907,7 +907,7 @@ static int do_mmcops(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (cp == NULL || argc > cp->maxargs) return CMD_RET_USAGE; - if (flag == CMD_FLAG_REPEAT && !cp->repeatable) + if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) return CMD_RET_SUCCESS; if (curr_device < 0) { diff --git a/common/command.c b/common/command.c index e13cb47ac18b..19f0534a76ea 100644 --- a/common/command.c +++ b/common/command.c @@ -501,6 +501,30 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) } #endif +int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[], int *repeatable) +{ + *repeatable = 1; + + return cmdtp->cmd(cmdtp, flag, argc, argv); +} + +int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[], int *repeatable) +{ + *repeatable = 0; + + return cmdtp->cmd(cmdtp, flag, argc, argv); +} + +int cmd_discard_repeatable(cmd_tbl_t *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 @@ -510,13 +534,15 @@ void fixup_cmdtable(cmd_tbl_t *cmdtp, int size) * @param flag Some flags normally 0 (see CMD_FLAG_.. above) * @param argc Number of arguments (arg 0 must be the command text) * @param argv Arguments + * @param repeatable Can the command be repeated * @return 0 if command succeeded, else non-zero (CMD_RET_...) */ -static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +static int cmd_call(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], + int *repeatable) { int result; - result = (cmdtp->cmd)(cmdtp, flag, argc, argv); + result = cmdtp->cmd_rep(cmdtp, flag, argc, argv, repeatable); if (result) debug("Command failed, result=%d\n", result); return result; @@ -553,12 +579,14 @@ enum command_ret_t cmd_process(int flag, int argc, char * const argv[], /* If OK so far, then do the command */ if (!rc) { + int newrep; + if (ticks) *ticks = get_timer(0); - rc = cmd_call(cmdtp, flag, argc, argv); + rc = cmd_call(cmdtp, flag, argc, argv, &newrep); if (ticks) *ticks = get_timer(*ticks); - *repeatable &= cmdtp->repeatable; + *repeatable &= newrep; } if (rc == CMD_RET_USAGE) rc = cmd_usage(cmdtp); diff --git a/include/command.h b/include/command.h index 89efcecfa926..bb93f022c514 100644 --- a/include/command.h +++ b/include/command.h @@ -29,7 +29,16 @@ struct cmd_tbl_s { char *name; /* Command Name */ int maxargs; /* maximum number of arguments */ - int repeatable; /* autorepeat allowed? */ + /* + * 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_s *cmd, int flags, int argc, + char * const argv[], int *repeatable); /* Implementation function */ int (*cmd)(struct cmd_tbl_s *, int, int, char * const []); char *usage; /* Usage message (short) */ @@ -60,6 +69,19 @@ int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc, extern int cmd_usage(const cmd_tbl_t *cmdtp); +/* Dummy ->cmd and ->cmd_rep wrappers. */ +int cmd_always_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[], int *repeatable); +int cmd_never_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[], int *repeatable); +int cmd_discard_repeatable(cmd_tbl_t *cmdtp, int flag, int argc, + char * const argv[]); + +static inline bool cmd_is_repeatable(cmd_tbl_t *cmdtp) +{ + return cmdtp->cmd_rep == cmd_always_repeatable; +} + #ifdef CONFIG_AUTO_COMPLETE extern int var_complete(int argc, char * const argv[], char last_char, int maxv, char *cmdv[]); extern int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp); @@ -188,16 +210,28 @@ int board_run_command(const char *cmdline); #endif #ifdef CONFIG_CMDLINE +#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ + _usage, _help, _comp) \ + { #_name, _maxargs, _cmd_rep, cmd_discard_repeatable, \ + _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } + #define U_BOOT_CMD_MKENT_COMPLETE(_name, _maxargs, _rep, _cmd, \ _usage, _help, _comp) \ - { #_name, _maxargs, _rep, _cmd, _usage, \ - _CMD_HELP(_help) _CMD_COMPLETE(_comp) } + { #_name, _maxargs, \ + _rep ? cmd_always_repeatable : cmd_never_repeatable, \ + _cmd, _usage, _CMD_HELP(_help) _CMD_COMPLETE(_comp) } #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); +#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ + _help, _comp) \ + ll_entry_declare(cmd_tbl_t, _name, cmd) = \ + U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ + _usage, _help, _comp) + #else #define U_BOOT_SUBCMD_START(name) static cmd_tbl_t name[] = {}; #define U_BOOT_SUBCMD_END @@ -209,15 +243,25 @@ int board_run_command(const char *cmdline); _cmd(NULL, 0, 0, NULL); \ return 0; \ } + +#define U_BOOT_CMDREP_MKENT_COMPLETE(_name, _maxargs, _cmd_rep, \ + _usage, _help, _comp) \ + { #_name, _maxargs, 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, _rep, 0 ? _cmd : NULL, _usage, \ + { #_name, _maxargs, NULL, 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) +#define U_BOOT_CMDREP_COMPLETE(_name, _maxargs, _cmd_rep, _usage, \ + _help, _comp) \ + _CMD_REMOVE(sub_ ## _name, _cmd_rep) + #endif /* CONFIG_CMDLINE */ #define U_BOOT_CMD(_name, _maxargs, _rep, _cmd, _usage, _help) \