[PATCH v3] sysboot: add zboot support to boot x86 Linux kernel image

[Bin Meng]

HI Köry,

On Tue, Feb 2, 2021 at 5:35 PM Köry Maincent <kory.maincent at bootlin.com> wrote:
>
> Hi Simon,
>
> Thanks for the review.
>
> On Mon, 1 Feb 2021 13:44:46 -0700
> Simon Glass <sjg at chromium.org> wrote:
>
> > Hi Kory,
> >
> > On Mon, 1 Feb 2021 at 08:31, Kory Maincent <kory.maincent at bootlin.com> wrote:
> > >
> > > Add "zboot" command to the list of supported boot in the label_boot
> > > function.
> > >
> > > Signed-off-by: Kory Maincent <kory.maincent at bootlin.com>
> > > ---
> > >
> > > Change since v1:
> > >  - Modify comment
> > >
> > > Change since v2:
> > >  - Update do_zboot to do_zboot_parent function to follow the patch:
> > >    5588e776b0
> > >
> > >  cmd/pxe_utils.c   | 4 ++++
> > >  include/command.h | 3 +++
> > >  2 files changed, 7 insertions(+)
> > >
> > > diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
> > > index 3526a651d7..06611262c1 100644
> > > --- a/cmd/pxe_utils.c
> > > +++ b/cmd/pxe_utils.c
> > > @@ -657,6 +657,10 @@ static int label_boot(struct cmd_tbl *cmdtp, struct
> > > pxe_label *label) /* Try booting a Image */
> > >         else
> > >                 do_bootz(cmdtp, 0, bootm_argc, bootm_argv);
> > > +#elif defined(CONFIG_CMD_ZBOOT)
> >
> > Can this use IS_ENABLED() ?
>
> Yes it can use IS_ENABLED indeed.

A separate clean-up patch to convert existing ones using IS_ENABLED()

>
> >
> > > +       /* Try booting an x86_64 Linux kernel image */
> > > +       else
> > > +               do_zboot_parent(cmdtp, 0, bootm_argc, bootm_argv, NULL);
> > >  #endif
> > >         unmap_sysmem(buf);
> > >
> > > diff --git a/include/command.h b/include/command.h
> > > index e229bf2825..cb91ba81b5 100644
> > > --- a/include/command.h
> > > +++ b/include/command.h
> > > @@ -165,6 +165,9 @@ extern int do_bootz(struct cmd_tbl *cmdtp, int flag,
> > > int argc, extern int do_booti(struct cmd_tbl *cmdtp, int flag, int argc,
> > >                     char *const argv[]);
> > >
> > > +extern int do_zboot_parent(struct cmd_tbl *cmdtp, int flag, int argc,
> > > +                          char *const argv[], int *repeatable);
> >
> > We don't normally use extern for function decls in header files.
>
> Ok, why the other boot commands (do_booti, do_bootz) use extern?
>

I think you can create a clean-up patch to remove extern.

Regards,
Bin