[08/14] fastboot: Remove dependencies on CMDLINE

It is possible to boot a kernel without CMDLINE being enabled. Update
the implementation to handle this, and drop the condition from the
FASTBOOT config.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/fastboot/Kconfig     |  1 -
 drivers/fastboot/fb_common.c | 26 ++++++++++++--------------
 2 files changed, 12 insertions(+), 15 deletions(-)

On Mon, Dec 4, 2023 at 1:31 AM Simon Glass <sjg@chromium.org> wrote:
>
> It is possible to boot a kernel without CMDLINE being enabled. Update
> the implementation to handle this, and drop the condition from the
> FASTBOOT config.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/fastboot/Kconfig     |  1 -
>  drivers/fastboot/fb_common.c | 26 ++++++++++++--------------
>  2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 11fc0fe1c800..837c6f1180da 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -1,5 +1,4 @@
>  menu "Fastboot support"
> -       depends on CMDLINE
>
>  config FASTBOOT
>         bool
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index 07f5946d9ed1..85935380e79d 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -11,6 +11,7 @@
>   */
>
>  #include <bcb.h>
> +#include <bootm.h>
>  #include <common.h>
>  #include <command.h>
>  #include <env.h>
> @@ -144,20 +145,17 @@ void fastboot_boot(void)
>  {
>         char *s;
>
> -       s = env_get("fastboot_bootcmd");
> -       if (s) {
> -               run_command(s, CMD_FLAG_ENV);
> -       } else if (IS_ENABLED(CONFIG_CMD_BOOTM)) {
> -               static char boot_addr_start[20];
> -               static char *const bootm_args[] = {
> -                       "bootm", boot_addr_start, NULL
> -               };
> -
> -               snprintf(boot_addr_start, sizeof(boot_addr_start) - 1,
> -                        "%lx", fastboot_buf_addr);
> -               printf("Booting kernel at %s...\n\n\n", boot_addr_start);
> -
> -               do_bootm(NULL, 0, 2, bootm_args);
> +       if (IS_ENABLED(CONFIG_CMDLINE)) {
> +               s = env_get("fastboot_bootcmd");
> +               if (s) {
> +                       run_command(s, CMD_FLAG_ENV);
> +                       return;
> +               }
> +       } else if (IS_ENABLED(CONFIG_BOOTM)) {
> +               int ret;
> +
> +               printf("Booting kernel at %lx...\n\n\n", fastboot_buf_addr);
> +               ret = bootm_boot_start(fastboot_buf_addr, NULL);
>
>                 /*
>                  * This only happens if image is somehow faulty so we start
> --

Doesn't this change the logic? Previously if you didn't set
fastboot_bootcmd you'd fall into the bootm path (if CONFIG_BOOTM was
enabled), with this, if CONFIG_CMDLINE is enabled then you will never
hit the bootm path.

On Mon, Dec 04, 2023 at 10:27:53AM +0000, Alex Kiernan wrote:
> On Mon, Dec 4, 2023 at 1:31 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > It is possible to boot a kernel without CMDLINE being enabled. Update
> > the implementation to handle this, and drop the condition from the
> > FASTBOOT config.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  drivers/fastboot/Kconfig     |  1 -
> >  drivers/fastboot/fb_common.c | 26 ++++++++++++--------------
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> > index 11fc0fe1c800..837c6f1180da 100644
> > --- a/drivers/fastboot/Kconfig
> > +++ b/drivers/fastboot/Kconfig
> > @@ -1,5 +1,4 @@
> >  menu "Fastboot support"
> > -       depends on CMDLINE
> >
> >  config FASTBOOT
> >         bool
> > diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> > index 07f5946d9ed1..85935380e79d 100644
> > --- a/drivers/fastboot/fb_common.c
> > +++ b/drivers/fastboot/fb_common.c
> > @@ -11,6 +11,7 @@
> >   */
> >
> >  #include <bcb.h>
> > +#include <bootm.h>
> >  #include <common.h>
> >  #include <command.h>
> >  #include <env.h>
> > @@ -144,20 +145,17 @@ void fastboot_boot(void)
> >  {
> >         char *s;
> >
> > -       s = env_get("fastboot_bootcmd");
> > -       if (s) {
> > -               run_command(s, CMD_FLAG_ENV);
> > -       } else if (IS_ENABLED(CONFIG_CMD_BOOTM)) {
> > -               static char boot_addr_start[20];
> > -               static char *const bootm_args[] = {
> > -                       "bootm", boot_addr_start, NULL
> > -               };
> > -
> > -               snprintf(boot_addr_start, sizeof(boot_addr_start) - 1,
> > -                        "%lx", fastboot_buf_addr);
> > -               printf("Booting kernel at %s...\n\n\n", boot_addr_start);
> > -
> > -               do_bootm(NULL, 0, 2, bootm_args);
> > +       if (IS_ENABLED(CONFIG_CMDLINE)) {
> > +               s = env_get("fastboot_bootcmd");
> > +               if (s) {
> > +                       run_command(s, CMD_FLAG_ENV);
> > +                       return;
> > +               }
> > +       } else if (IS_ENABLED(CONFIG_BOOTM)) {
> > +               int ret;
> > +
> > +               printf("Booting kernel at %lx...\n\n\n", fastboot_buf_addr);
> > +               ret = bootm_boot_start(fastboot_buf_addr, NULL);
> >
> >                 /*
> >                  * This only happens if image is somehow faulty so we start
> > --
> 
> Doesn't this change the logic? Previously if you didn't set
> fastboot_bootcmd you'd fall into the bootm path (if CONFIG_BOOTM was
> enabled), with this, if CONFIG_CMDLINE is enabled then you will never
> hit the bootm path.

It's a subtle change in logic I think, yes. And also, we should
document (perhaps as a follow-up) what does and doesn't work in fastboot
today that depends on CMDLINE and parsing / executing commands as set in
environment variables. As with this patch yes, fastboot does not require
CMDLINE but it is also not as functional given that as I understand it
anyhow "fastboot_bootcmd" isn't just "bootm ..." but can handle various
state things and LED flashing and so on.