[U-Boot] [U-Boot, v4, 3/6] common: command: Rework the 'cmd is repeatable' logic

[Tom Rini <trini@konsulko.com>]

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)		\

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190115/522c52a8/attachment.sig>