[U-Boot] [PATCH v4 14/14] net: fastboot: Merge AOSP UDP fastboot

[Alex Kiernan <alex.kiernan@gmail.com>]

On Wed, May 16, 2018 at 8:15 PM Joe Hershberger <joe.hershberger@ni.com>
wrote:

> On Tue, May 15, 2018 at 9:48 AM, Alex Kiernan <alex.kiernan@gmail.com>
wrote:
> > Merge UDP fastboot support from AOSP:
> >
> >
https://android.googlesource.com/platform/external/u-boot/+/android-o-mr1-iot-preview-8
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > Signed-off-by: Alex Deymo <deymo@google.com>
> > Signed-off-by: Jocelyn Bohr <bohr@google.com>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v4:
> > - guard fb_getvar/fb_command with UDP_FUNCTION_FASTBOOT in Makefile
> > - add docbook comments
> > - remove parameter from fastboot_boot() since we always want
> >   fastboot_buf_addr (and if we're using fastboot_bootcmd then it's
> >   ignored)
> >
> > Changes in v3:
> > - use FASTBOOT as our guard in Kconfig not a list of USB || UDP
> > - correct mis-translation from AOSP introduced when cleaning up for
> >   checkpatch - we should write when buffer is not NULL, rather than
> >   erasing, and erase when buffer is NULL
> > - use CMD_RET_USAGE from do_fastboot
> > - remove do_fastboot_udp from cmd/net.c and rewrite using net_loop()
> > - rename timed_send_info to fastboot_send_info, rename
fastboot_send_info to
> >   fastboot_udp_send_info
> > - replace FASTBOOT_HEADER_SIZE with sizeof(struct fastboot_header)
> > - move start time into timed_send_info() rather than passing it in
> > - make calls to fastboot_udp_send_info a runtime dependency, not a
compile
> >   time one
> > - set ${filesize} to size of downloaded image
> > - add progress meter from USB path during download
> > - add support for 'oem format' command from the USB path
> > - rename 'fastbootcmd' to 'fastboot_bootcmd' to make clear that this is
the
> >   fastboot boot command
> > - make getvar implementation table driven
> > - add fastboot_buf_addr, fastboot_buf_size to override buffer address
and
> >   size
> > - return correct filesystem type in getvar partition-type on MMC
> > - process "fastboot." prefixed env variables in getvar first so you
> >   can override the normal values (this also lets you set a fs type for
> >   NAND devices)
> > - make global variables static and add accessors for the things which
> >   the transport layers need
> > - squash subsequent patches which change this code into this one:
> >   - If the fastboot flash/erase commands are disabled, remove that
support
> >     so we still build correctly.
> >   - Add NAND support to fastboot UDP flash/erase commands
> >   - If we don't have a partition name passed, report it as not found.
> >   - Change the behaviour of the fastboot net code such that
> >     "reboot-bootloader" is no longer written to
CONFIG_FASTBOOT_BUF_ADDR for
> >     use as a marker on reboot (the AOSP code in
common/android-bootloader.c
> >     uses this marker - this code could be reinstated there if that gets
> >     merged).
> >   - Merge USB and UDP boot code. The USB implementation stays the same,
but
> >     UDP no longer passes an fdt. We introduce a new environment variable
> >     'fastboot_bootcmd' which if set overrides the hardcoded boot
command,
> >     setting this then allows the UDP implementation to remain the same.
If
> >     after running 'fastboot_bootcmd' the board has not booted, control
is
> >     returned to U-Boot and the fastboot process ends.
> >   - Separate the fastboot protocol handling from the fastboot UDP code
in
> >     preparation for reusing it in the USB code.
> >
> > Changes in v2:
> > - ensure fastboot syntax is backward compatible - 'fastboot 0' means
> >   'fastboot usb 0'
> >
> >  cmd/fastboot.c                |  91 +++++++++++-
> >  drivers/fastboot/Kconfig      |  15 ++
> >  drivers/fastboot/Makefile     |   3 +-
> >  drivers/fastboot/fb_command.c | 327
++++++++++++++++++++++++++++++++++++++++++
> >  drivers/fastboot/fb_common.c  | 113 +++++++++++++++
> >  drivers/fastboot/fb_getvar.c  | 229 +++++++++++++++++++++++++++++
> >  drivers/fastboot/fb_mmc.c     |  76 +++++++++-
> >  drivers/fastboot/fb_nand.c    |  12 +-
> >  include/fastboot.h            | 157 ++++++++++++++++++++
> >  include/fb_mmc.h              |   8 +-
> >  include/fb_nand.h             |  10 +-
> >  include/net.h                 |   2 +-
> >  include/net/fastboot.h        |  21 +++
> >  net/Makefile                  |   1 +
> >  net/fastboot.c                | 312
++++++++++++++++++++++++++++++++++++++++
> >  net/net.c                     |   7 +
> >  16 files changed, 1367 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/fastboot/fb_command.c
> >  create mode 100644 drivers/fastboot/fb_getvar.c
> >  create mode 100644 include/net/fastboot.h
> >  create mode 100644 net/fastboot.c
> >
> > diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> > index a5ec5f4..557257a 100644
> > --- a/cmd/fastboot.c
> > +++ b/cmd/fastboot.c
> > @@ -10,10 +10,32 @@
> >  #include <command.h>
> >  #include <console.h>
> >  #include <g_dnl.h>
> > +#include <fastboot.h>
> > +#include <net.h>
> >  #include <usb.h>
> >
> > -static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char
*const argv[])
> > +static int do_fastboot_udp(int argc, char *const argv[],
> > +                          uintptr_t buf_addr, size_t buf_size)
> >  {
> > +#if CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)
> > +       int err = net_loop(FASTBOOT);
> > +
> > +       if (err < 0) {
> > +               printf("fastboot udp error: %d\n", err);
> > +               return CMD_RET_FAILURE;
> > +       }
> > +
> > +       return CMD_RET_SUCCESS;
> > +#else
> > +       pr_err("Fastboot UDP not enabled\n");
> > +       return CMD_RET_FAILURE;
> > +#endif
> > +}
> > +
> > +static int do_fastboot_usb(int argc, char *const argv[],
> > +                          uintptr_t buf_addr, size_t buf_size)
> > +{
> > +#if CONFIG_IS_ENABLED(USB_FUNCTION_FASTBOOT)
> >         int controller_index;
> >         char *usb_controller;
> >         int ret;
> > @@ -58,11 +80,70 @@ exit:
> >         board_usb_cleanup(controller_index, USB_INIT_DEVICE);
> >
> >         return ret;
> > +#else
> > +       pr_err("Fastboot USB not enabled\n");
> > +       return CMD_RET_FAILURE;
> > +#endif
> > +}
> > +
> > +static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char
*const argv[])
> > +{
> > +       uintptr_t buf_addr = (uintptr_t)NULL;
> > +       size_t buf_size = 0;
> > +
> > +       if (argc < 2)
> > +               return CMD_RET_USAGE;
> > +
> > +       while (argc > 1 && **(argv + 1) == '-') {
> > +               char *arg = *++argv;
> > +
> > +               --argc;
> > +               while (*++arg) {
> > +                       switch (*arg) {
> > +                       case 'l':
> > +                               if (--argc <= 0)
> > +                                       return CMD_RET_USAGE;
> > +                               buf_addr = simple_strtoul(*++argv,
NULL, 16);
> > +                               goto NXTARG;
> > +
> > +                       case 's':
> > +                               if (--argc <= 0)
> > +                                       return CMD_RET_USAGE;
> > +                               buf_size = simple_strtoul(*++argv,
NULL, 16);
> > +                               goto NXTARG;
> > +
> > +                       default:
> > +                               return CMD_RET_USAGE;
> > +                       }
> > +               }
> > +NXTARG:
> > +               ;
> > +       }
> > +
> > +       fastboot_init((void *)buf_addr, buf_size);
> > +
> > +       if (!strcmp(argv[1], "udp"))
> > +               return do_fastboot_udp(argc, argv, buf_addr, buf_size);
> > +
> > +       if (!strcmp(argv[1], "usb")) {
> > +               argv++;
> > +               argc--;
> > +       }
> > +
> > +       return do_fastboot_usb(argc, argv, buf_addr, buf_size);
> >  }
> >
> > +#ifdef CONFIG_SYS_LONGHELP
> > +static char fastboot_help_text[] =
> > +       "[-l addr] [-s size] usb <controller> | udp\n"
> > +       "\taddr - address of buffer used during data transfers ("
> > +       __stringify(CONFIG_FASTBOOT_BUF_ADDR) ")\n"
> > +       "\tsize - size of buffer used during data transfers ("
> > +       __stringify(CONFIG_FASTBOOT_BUF_SIZE) ")"
> > +       ;
> > +#endif
> > +
> >  U_BOOT_CMD(
> > -       fastboot, 2, 1, do_fastboot,
> > -       "use USB Fastboot protocol",
> > -       "<USB_controller>\n"
> > -       "    - run as a fastboot usb device"
> > +       fastboot, CONFIG_SYS_MAXARGS, 1, do_fastboot,
> > +       "run as a fastboot usb or udp device", fastboot_help_text
> >  );
> > diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> > index 82e1420..5844f3f 100644
> > --- a/drivers/fastboot/Kconfig
> > +++ b/drivers/fastboot/Kconfig
> > @@ -14,6 +14,13 @@ config USB_FUNCTION_FASTBOOT
> >         help
> >           This enables the USB part of the fastboot gadget.
> >
> > +config UDP_FUNCTION_FASTBOOT
> > +       depends on NET
> > +       select FASTBOOT
> > +       bool "Enable fastboot protocol over UDP"
> > +       help
> > +         This enables the fastboot protocol over UDP.
> > +
> >  if FASTBOOT
> >
> >  config FASTBOOT_BUF_ADDR
> > @@ -119,6 +126,14 @@ config FASTBOOT_MBR_NAME
> >           specified on the "fastboot flash" command line matches the
value
> >           defined here. The default target name for updating MBR is
"mbr".
> >
> > +config FASTBOOT_CMD_OEM_FORMAT
> > +       bool "Enable the 'oem format' command"
> > +       depends on FASTBOOT_FLASH_MMC && CMD_GPT
> > +       help
> > +         Add support for the "oem format" command from a client. This
> > +         relies on the env variable partitions to contain the list of
> > +         partitions as required by the gpt command.

> I think this (feature) deserves its own patch.


Not sure why I squashed that as I did have have its own... I'll split it
out.

> > +
> >  endif # FASTBOOT
> >
> >  endmenu
> > diff --git a/drivers/fastboot/Makefile b/drivers/fastboot/Makefile
> > index e4bd389..8831096 100644
> > --- a/drivers/fastboot/Makefile
> > +++ b/drivers/fastboot/Makefile
> > @@ -1,6 +1,7 @@
> >  # SPDX-License-Identifier:      GPL-2.0+
> >
> >  obj-y += fb_common.o
> > -
> > +obj-$(CONFIG_UDP_FUNCTION_FASTBOOT) += fb_getvar.o
> > +obj-$(CONFIG_UDP_FUNCTION_FASTBOOT) += fb_command.o
> >  obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fb_mmc.o
> >  obj-$(CONFIG_FASTBOOT_FLASH_NAND) += fb_nand.o
> > diff --git a/drivers/fastboot/fb_command.c
b/drivers/fastboot/fb_command.c
> > new file mode 100644
> > index 0000000..0ca05ec
> > --- /dev/null
> > +++ b/drivers/fastboot/fb_command.c
> > @@ -0,0 +1,327 @@
> > +// SPDX-License-Identifier: BSD-2-Clause
> > +/*
> > + * Copyright (C) 2016 The Android Open Source Project
> > + */
> > +
> > +#include <common.h>
> > +#include <fastboot.h>
> > +#include <fb_mmc.h>
> > +#include <fb_nand.h>
> > +#include <part.h>
> > +#include <stdlib.h>
> > +
> > +/**
> > + * image_size - final fastboot image size
> > + */
> > +static u32 image_size;
> > +
> > +/**
> > + * fastboot_bytes_received - number of bytes received in the current
download
> > + */
> > +static u32 fastboot_bytes_received;
> > +
> > +/**
> > + * fastboot_bytes_expected - number of bytes expected in the current
download
> > + */
> > +static u32 fastboot_bytes_expected;
> > +
> > +static void fb_okay(char *, char *);
> > +static void fb_getvar(char *, char *);
> > +static void fb_download(char *, char *);
> > +#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> > +static void fb_flash(char *, char *);
> > +static void fb_erase(char *, char *);
> > +#endif
> > +static void fb_reboot_bootloader(char *, char *);
> > +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
> > +static void fb_oem_format(char *, char *);
> > +#endif
> > +
> > +static const struct {
> > +       const char *command;
> > +       void (*dispatch)(char *cmd_parameter, char *response);
> > +} fb_commands[FASTBOOT_COMMAND_COUNT] = {
> > +       [FASTBOOT_COMMAND_GETVAR] = {
> > +               .command = "getvar",
> > +               .dispatch = fb_getvar
> > +       },
> > +       [FASTBOOT_COMMAND_DOWNLOAD] = {
> > +               .command = "download",
> > +               .dispatch = fb_download
> > +       },
> > +#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
> > +       [FASTBOOT_COMMAND_FLASH] =  {
> > +               .command = "flash",
> > +               .dispatch = fb_flash
> > +       },
> > +       [FASTBOOT_COMMAND_ERASE] =  {
> > +               .command = "erase",
> > +               .dispatch = fb_erase
> > +       },
> > +#endif
> > +       [FASTBOOT_COMMAND_BOOT] =  {
> > +               .command = "boot",
> > +               .dispatch = fb_okay
> > +       },
> > +       [FASTBOOT_COMMAND_CONTINUE] =  {
> > +               .command = "continue",
> > +               .dispatch = fb_okay
> > +       },
> > +       [FASTBOOT_COMMAND_REBOOT] =  {
> > +               .command = "reboot",
> > +               .dispatch = fb_okay
> > +       },
> > +       [FASTBOOT_COMMAND_REBOOT_BOOTLOADER] =  {
> > +               .command = "reboot-bootloader",
> > +               .dispatch = fb_reboot_bootloader
> > +       },
> > +       [FASTBOOT_COMMAND_SET_ACTIVE] =  {
> > +               .command = "set_active",
> > +               .dispatch = fb_okay
> > +       },
> > +#if CONFIG_IS_ENABLED(FASTBOOT_CMD_OEM_FORMAT)
> > +       [FASTBOOT_COMMAND_OEM_FORMAT] = {
> > +               .command = "oem format",
> > +               .dispatch = fb_oem_format,
> > +       },
> > +#endif
> > +};
> > +
> > +/**
> > + * fastboot_get_bytes_remaining() - return bytes remaining in current
transfer
> > + *
> > + * Return: Number of bytes left in the current download
> > + */
> > +u32 fastboot_get_bytes_remaining(void)
> > +{
> > +       return fastboot_bytes_expected - fastboot_bytes_received;
> > +}
> > +
> > +/**
> > + * fastboot_handle_command - Handle fastboot command
> > + *
> > + * @cmd_string: Pointer to command string
> > + * @response: Pointer to fastboot response buffer
> > + *
> > + * Return: Executed command, or -1 if not recognized
> > + */
> > +int fastboot_handle_command(char *cmd_string, char *response)
> > +{
> > +       int i;
> > +       char *cmd_parameter;
> > +
> > +       cmd_parameter = cmd_string;
> > +       strsep(&cmd_parameter, ":");
> > +
> > +       for (i = 0; i < FASTBOOT_COMMAND_COUNT; i++) {
> > +               if (!strcmp(fb_commands[i].command, cmd_string)) {
> > +                       if (fb_commands[i].dispatch) {
> > +                               fb_commands[i].dispatch(cmd_parameter,
> > +                                                       response);
> > +                               return i;
> > +                       } else {
> > +                               break;
> > +                       }
> > +               }
> > +       }
> > +
> > +       pr_err("command %s not recognized.\n", cmd_string);
> > +       fastboot_fail("unrecognized command", response);
> > +       return -1;
> > +}
> > +
> > +/**
> > + * fb_okay() - Send bare OKAY response

> It would be good to settle on "fastboot" or "fb" and not use both. It
> seems many of the "fb" are static, but some are not. Static really
> don't need any scoping like "fb_".

I'm taking fb_ to be frame buffer, fastboot_ is fastboot.

I'll fix them.