[PATCH v2 07/16] x86: zboot: Set up a sub-command structure

[Bin Meng]

Hi Simon,

On Sun, Aug 30, 2020 at 5:42 AM Simon Glass <sjg at chromium.org> wrote:
>
> Add subcommands to zboot. At present there is only one called 'start'
> which does the whole boot. It is the default command so is optional.
>
> Change the 's' string variable to const while we are here.
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2:
> - Fix comment about argv[0] in do_zboot_parent()
>
>  arch/x86/lib/zimage.c | 64 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/lib/zimage.c b/arch/x86/lib/zimage.c
> index 8651dea93b3..e3e3efdde43 100644
> --- a/arch/x86/lib/zimage.c
> +++ b/arch/x86/lib/zimage.c
> @@ -63,6 +63,12 @@ struct zboot_state {
>         ulong load_address;
>  } state;
>
> +enum {
> +       ZBOOT_STATE_START       = BIT(0),
> +
> +       ZBOOT_STATE_COUNT       = 1,
> +};
> +
>  static void build_command_line(char *command_line, int auto_boot)
>  {
>         char *env_command_line;
> @@ -328,10 +334,11 @@ int setup_zimage(struct boot_params *setup_base, char *cmd_line, int auto_boot,
>         return 0;
>  }
>
> -int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> +static int do_zboot_start(struct cmd_tbl *cmdtp, int flag, int argc,
> +                         char *const argv[])
>  {
>         struct boot_params *base_ptr;
> -       char *s;
> +       const char *s;
>
>         memset(&state, '\0', sizeof(state));
>         if (argc >= 2) {
> @@ -373,9 +380,53 @@ int do_zboot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>         return boot_linux_kernel((ulong)base_ptr, state.load_address, false);
>  }
>
> -U_BOOT_CMD(
> -       zboot, 5, 0,    do_zboot,
> -       "Boot bzImage",
> +U_BOOT_SUBCMDS(zboot,
> +       U_BOOT_CMD_MKENT(start, 8, 1, do_zboot_start, "", ""),

should the maxargs be 5 instead of 8?

> +)
> +
> +int do_zboot_states(struct cmd_tbl *cmdtp, int flag, int argc,
> +                   char *const argv[], int state_mask)
> +{
> +       int i;
> +
> +       for (i = 0; i < ZBOOT_STATE_COUNT; i++) {
> +               struct cmd_tbl *cmd = &zboot_subcmds[i];
> +               int mask = 1 << i;
> +               int ret;
> +
> +               if (mask & state_mask) {
> +                       ret = cmd->cmd(cmd, flag, argc, argv);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
> +                   char *const argv[], int *repeatable)
> +{
> +       /* determine if we have a sub command */
> +       if (argc > 1) {
> +               char *endp;
> +
> +               simple_strtoul(argv[1], &endp, 16);
> +               /*
> +                * endp pointing to nul means that argv[1] was just a valid
> +                * number, so pass it along to the normal processing
> +                */
> +               if (*endp)
> +                       return do_zboot(cmdtp, flag, argc, argv, repeatable);
> +       }
> +
> +       do_zboot_states(cmdtp, flag, argc, argv, ZBOOT_STATE_START);
> +
> +       return CMD_RET_FAILURE;
> +}
> +
> +U_BOOT_CMDREP_COMPLETE(
> +       zboot, 8, do_zboot_parent, "Boot bzImage",
>         "[addr] [size] [initrd addr] [initrd size]\n"
>         "      addr -        The optional starting address of the bzimage.\n"
>         "                    If not set it defaults to the environment\n"
> @@ -383,5 +434,6 @@ U_BOOT_CMD(
>         "      size -        The optional size of the bzimage. Defaults to\n"
>         "                    zero.\n"
>         "      initrd addr - The address of the initrd image to use, if any.\n"
> -       "      initrd size - The size of the initrd image to use, if any.\n"
> +       "      initrd size - The size of the initrd image to use, if any.\n",
> +       complete_zboot

What's "complete_zboot"?

>  );

Regards,
Bin