Re: [PATCH v2 07/16] x86: zboot: Set up a sub-command structure
Hi Bin,
On Tue, 1 Sep 2020 at 03:30, Bin Meng wrote:
>
> Hi Simon,
>
> On Sun, Aug 30, 2020 at 5:42 AM Simon Glass 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
> > ---
> >
> > 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?
Yes, for this patch. I'll update it and adjust it in each following
patch as needed.
>
> > +)
> > +
> > +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"?
It is defined by the U_BOOT_SUBCMDS() macro. I'll add a comment.
Regards,
SImon
Re: [PATCH v2 07/16] x86: zboot: Set up a sub-command structure
Hi Simon,
On Sun, Aug 30, 2020 at 5:42 AM Simon Glass 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
> ---
>
> 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
Re: [PATCH v2 07/16] x86: zboot: Set up a sub-command structure
Hi Simon, -"Simon Glass" schrieb: - > Betreff: [PATCH v2 07/16] x86: zboot: Set up a sub-command structure > > 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 > --- > > 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(-) Reviewed-by: Wolfgang Wallner
[PATCH v2 07/16] x86: zboot: Set up a sub-command structure
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
---
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, "", ""),
+)
+
+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
);
--
2.28.0.402.g5ffc5be6b7-goog
