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

2019-01-15 Thread Tom Rini
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 
> Reviewed-by: Tom Rini 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-01-15 Thread Tom Rini
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 
> Reviewed-by: Tom Rini 
> ---
> 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, );
> +}
> +
>  /**
>   * 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