[U-Boot,RFC,v2,07/20] net: fastboot: Merge AOSP UDP fastboot

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>
---

Changes in v2:
- ensure fastboot syntax is backward compatible - 'fastboot 0' means
  'fastboot usb 0'

 cmd/fastboot.c               |  35 ++-
 cmd/net.c                    |   6 +
 drivers/fastboot/Kconfig     |  16 +-
 drivers/fastboot/fb_common.c |  18 ++
 drivers/fastboot/fb_mmc.c    |  34 ++-
 include/fastboot.h           |  13 ++
 include/net.h                |   6 +-
 include/net/fastboot.h       |  27 +++
 net/Makefile                 |   1 +
 net/fastboot.c               | 542 +++++++++++++++++++++++++++++++++++++++++++
 net/net.c                    |   9 +
 11 files changed, 695 insertions(+), 12 deletions(-)
 create mode 100644 include/net/fastboot.h
 create mode 100644 net/fastboot.c

On Mon, Apr 30, 2018 at 1:33 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>
> ---
>
> Changes in v2:
> - ensure fastboot syntax is backward compatible - 'fastboot 0' means
>   'fastboot usb 0'
>
>  cmd/fastboot.c               |  35 ++-
>  cmd/net.c                    |   6 +
>  drivers/fastboot/Kconfig     |  16 +-
>  drivers/fastboot/fb_common.c |  18 ++
>  drivers/fastboot/fb_mmc.c    |  34 ++-
>  include/fastboot.h           |  13 ++
>  include/net.h                |   6 +-
>  include/net/fastboot.h       |  27 +++
>  net/Makefile                 |   1 +
>  net/fastboot.c               | 542
> +++++++++++++++++++++++++++++++++++++++++++
>  net/net.c                    |   9 +
>  11 files changed, 695 insertions(+), 12 deletions(-)
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index 8adcca5..68a41de 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -11,17 +11,41 @@
>  #include <command.h>
>  #include <console.h>
>  #include <g_dnl.h>
> +#include <net.h>
>  #include <usb.h>
>
>  static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[])
>  {
> +#ifdef CONFIG_USB_FUNCTION_FASTBOOT
>         int controller_index;
>         char *usb_controller;
>         int ret;
> +#endif
>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
>
> +       if (!strcmp(argv[1], "udp")) {
> +#ifndef CONFIG_UDP_FUNCTION_FASTBOOT
> +               pr_err("Fastboot UDP not enabled\n");
> +               return -1;
> +#else
> +               return do_fastboot_udp(cmdtp, flag, argc, argv);
> +#endif
> +       }
> +
> +       if (!strcmp(argv[1], "usb")) {
> +               argv++;
> +               argc--;
> +       }
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +#ifndef CONFIG_USB_FUNCTION_FASTBOOT
> +       pr_err("Fastboot USB not enabled\n");
> +       return -1;
> +#else
>         usb_controller = argv[1];
>         controller_index = simple_strtoul(usb_controller, NULL, 0);
>
> @@ -59,11 +83,14 @@ exit:
>         board_usb_cleanup(controller_index, USB_INIT_DEVICE);
>
>         return ret;
> +#endif
>  }
>
>  U_BOOT_CMD(
> -       fastboot, 2, 1, do_fastboot,
> -       "use USB Fastboot protocol",
> -       "<USB_controller>\n"
> -       "    - run as a fastboot usb device"
> +       fastboot, 3, 1, do_fastboot,
> +       "use USB or UDP Fastboot protocol",
> +       "[usb,udp] <USB_controller>\n"
> +       " - run as a fastboot usb or udp device\n"
> +       "   usb: specify <USB_controller>\n"
> +       "   udp: requires ip_addr set and ethernet initialized\n"
>  );
> diff --git a/cmd/net.c b/cmd/net.c
> index 67888d4..668f344 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -74,6 +74,12 @@ U_BOOT_CMD(
>  );
>  #endif
>
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[])
> +{
> +       return netboot_common(FASTBOOT, cmdtp, argc, argv);
> +}
> +#endif
>
>  #ifdef CONFIG_CMD_RARP
>  int do_rarpb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 64b94f0..53c337f 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -2,6 +2,8 @@ menu "Fastboot support"
>
>  config FASTBOOT
>         bool
> +       imply ANDROID_BOOT_IMAGE
> +       imply CMD_FASTBOOT
>
>  config USB_FUNCTION_FASTBOOT
>         bool "Enable USB fastboot gadget"
> @@ -9,12 +11,17 @@ config USB_FUNCTION_FASTBOOT
>         default y if ARCH_SUNXI && USB_MUSB_GADGET
>         select FASTBOOT
>         select USB_GADGET_DOWNLOAD
> -       imply ANDROID_BOOT_IMAGE
> -       imply CMD_FASTBOOT
>         help
>           This enables the USB part of the fastboot gadget.
>
> -if USB_FUNCTION_FASTBOOT
> +config UDP_FUNCTION_FASTBOOT
> +       depends on NET
> +       select FASTBOOT
> +       bool "Enable fastboot protocol over UDP"
> +       help
> +         This enables the fastboot protocol over UDP.
> +
> +if USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
>
>  config FASTBOOT_BUF_ADDR
>         hex "Define FASTBOOT buffer address"
> @@ -46,6 +53,7 @@ config FASTBOOT_BUF_SIZE
>
>  config FASTBOOT_USB_DEV
>         int "USB controller number"
> +       depends on USB_FUNCTION_FASTBOOT
>         default 0
>         help
>           Some boards have USB OTG controller other than 0. Define this
> @@ -117,6 +125,6 @@ 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".
>
> -endif # USB_FUNCTION_FASTBOOT
> +endif # USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
>
>  endmenu
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index fe58803..7367fbb 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -12,6 +12,9 @@
>
>  #include <common.h>
>  #include <fastboot.h>
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +#include <net/fastboot.h>
> +#endif
>
>  /**
>   * Writes a response to response buffer of the form "$tag$reason".
> @@ -44,3 +47,18 @@ void fastboot_okay(const char *reason, char *response)
>  {
>         fastboot_response("OKAY", response, "%s", reason);
>  }
> +
> +void timed_send_info(ulong *start, const char *msg)
> +{
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +       /* Initialize timer */
> +       if (*start == 0)
> +               *start = get_timer(0);
> +       ulong time = get_timer(*start);
> +       /* Send INFO packet to host every 30 seconds */
> +       if (time >= 30000) {
> +               *start = get_timer(0);
> +               fastboot_send_info(msg);
> +       }
> +#endif
> +}
> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
> index 02864aa..304bda1 100644
> --- a/drivers/fastboot/fb_mmc.c
> +++ b/drivers/fastboot/fb_mmc.c
> @@ -29,6 +29,9 @@
>  #define CONFIG_FASTBOOT_MBR_NAME "mbr"
>  #endif
>
> +#define FASTBOOT_MAX_BLK_WRITE 16384
> +static ulong timer;
> +
>  #define BOOT_PARTITION_NAME "boot"
>
>  struct fb_mmc_sparse {
> @@ -57,13 +60,38 @@ static int part_get_info_by_name_or_alias(struct
> blk_desc *dev_desc,
>         return ret;
>  }
>
> +static lbaint_t fb_mmc_blk_write(struct blk_desc *block_dev, lbaint_t
> start,
> +                                lbaint_t blkcnt, const void *buffer)
> +{
> +       lbaint_t blk = start;
> +       lbaint_t blks_written;
> +       lbaint_t cur_blkcnt;
> +       lbaint_t blks = 0;
> +       int i;
> +
> +       for (i = 0; i < blkcnt; i += FASTBOOT_MAX_BLK_WRITE) {
> +               cur_blkcnt = min((int)blkcnt - i, FASTBOOT_MAX_BLK_WRITE);
> +               if (!buffer) {
> +                       timed_send_info(&timer, "writing");
> +                       blks_written = blk_dwrite(block_dev, blk,
> cur_blkcnt,
> +                                                 buffer + (i *
> block_dev->blksz));
> +               } else {
> +                       timed_send_info(&timer, "erasing");
> +                       blks_written = blk_derase(block_dev, blk,
> cur_blkcnt);
> +               }
> +               blk += blks_written;
> +               blks += blks_written;
> +       }
> +       return blks;
> +}
> +
>  static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info,
>                 lbaint_t blk, lbaint_t blkcnt, const void *buffer)
>  {
>         struct fb_mmc_sparse *sparse = info->priv;
>         struct blk_desc *dev_desc = sparse->dev_desc;
>
> -       return blk_dwrite(dev_desc, blk, blkcnt, buffer);
> +       return fb_mmc_blk_write(dev_desc, blk, blkcnt, buffer);
>  }
>
>  static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info,
> @@ -91,7 +119,7 @@ static void write_raw_image(struct blk_desc *dev_desc,
> disk_partition_t *info,
>
>         puts("Flashing Raw Image\n");
>
> -       blks = blk_dwrite(dev_desc, info->start, blkcnt, buffer);
> +       blks = fb_mmc_blk_write(dev_desc, info->start, blkcnt, buffer);
>         if (blks != blkcnt) {
>                 pr_err("failed writing to device %d\n", dev_desc->devnum);
>                 fastboot_fail("failed writing to device", response);
> @@ -399,7 +427,7 @@ void fb_mmc_erase(const char *cmd, char *response)
>         printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n",
>                blks_start, blks_start + blks_size);
>
> -       blks = blk_derase(dev_desc, blks_start, blks_size);
> +       blks = fb_mmc_blk_write(dev_desc, blks_start, blks_size, NULL);
>         if (blks != blks_size) {
>                 pr_err("failed erasing from device %d", dev_desc->devnum);
>                 fastboot_fail("failed erasing from device", response);
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 2140c94..6f69423 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -23,4 +23,17 @@ void fastboot_response(const char *tag, char *response,
>  void fastboot_fail(const char *reason, char *response);
>  void fastboot_okay(const char *reason, char *response);
>
> +/**
> + * Send an INFO packet during long commands based on timer. If
> + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
> + * if the time is 30 seconds after start. Else, noop.
> + *
> + * TODO: Handle the situation where both UDP and USB fastboot are
> + *       enabled.
> + *
> + * @param start:  Time since last INFO packet was sent.
> + * @param msg:    String describing the reason for waiting
> + */
> +void timed_send_info(ulong *start, const char *msg);
> +
>  #endif /* _FASTBOOT_H_ */
> diff --git a/include/net.h b/include/net.h
> index 3469811..890ae27 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -535,7 +535,7 @@ extern int          net_restart_wrap;       /* Tried
> all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS,
> SNTP,
> -       TFTPSRV, TFTPPUT, LINKLOCAL
> +       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT
>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> @@ -549,6 +549,10 @@ extern char *net_dns_resolve;              /* The
> host to resolve  */
>  extern char *net_dns_env_var;          /* the env var to put the ip into
> */
>  #endif
>
> +#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
> +int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const
> argv[]);
> +#endif
> +
>  #if defined(CONFIG_CMD_PING)
>  extern struct in_addr net_ping_ip;     /* the ip address to ping */
>  #endif
> diff --git a/include/net/fastboot.h b/include/net/fastboot.h
> new file mode 100644
> index 0000000..c0dd033
> --- /dev/null
> +++ b/include/net/fastboot.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2016 The Android Open Source Project
> + */
> +
> +#ifndef __NET_FASTBOOT_H__
> +#define __NET_FASTBOOT_H__
> +
> +/**********************************************************************/
> +/*
> + *     Global functions and variables.
> + */
> +
> +/**
> + * Wait for incoming fastboot comands.
> + */
> +void fastboot_start_server(void);
> +/**
> + * Send an INFO packet during long commands
> + *
> + * @param msg: String describing the reason for waiting
> + */
> +void fastboot_send_info(const char *msg);
> +
> +/**********************************************************************/
> +
> +#endif /* __NET_FASTBOOT_H__ */
> diff --git a/net/Makefile b/net/Makefile
> index ce6e5ad..3489ce5 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
> +obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/fastboot.c b/net/fastboot.c
> new file mode 100644
> index 0000000..32cb581
> --- /dev/null
> +++ b/net/fastboot.c
> @@ -0,0 +1,542 @@
> +// 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 <net.h>
> +#include <net/fastboot.h>
> +#include <part.h>
> +#include <stdlib.h>
> +#include <version.h>
> +
> +/* Fastboot port # defined in spec */
> +#define WELL_KNOWN_PORT 5554
> +
> +enum {
> +       FASTBOOT_ERROR = 0,
> +       FASTBOOT_QUERY = 1,
> +       FASTBOOT_INIT = 2,
> +       FASTBOOT_FASTBOOT = 3,
> +};
> +
> +struct __packed fastboot_header {
> +       uchar id;
> +       uchar flags;
> +       unsigned short seq;
> +};
> +
> +#define PACKET_SIZE 1024
> +#define FASTBOOT_HEADER_SIZE sizeof(struct fastboot_header)
> +#define DATA_SIZE (PACKET_SIZE - FASTBOOT_HEADER_SIZE)
> +#define FASTBOOT_VERSION "0.4"
> +
> +/* Sequence number sent for every packet */
> +static unsigned short fb_sequence_number = 1;
> +static const unsigned short fb_packet_size = PACKET_SIZE;
> +static const unsigned short fb_udp_version = 1;
> +
> +/* Keep track of last packet for resubmission */
> +static uchar last_packet[PACKET_SIZE];
> +static unsigned int last_packet_len;
> +
> +/* Parsed from first fastboot command packet */
> +static char *cmd_string;
> +static char *cmd_parameter;
> +
> +/* Fastboot download parameters */
> +static unsigned int bytes_received;
> +static unsigned int bytes_expected;
> +static unsigned int image_size;
> +
> +static struct in_addr fastboot_remote_ip;
> +/* The UDP port at their end */
> +static int fastboot_remote_port;
> +/* The UDP port at our end */
> +static int fastboot_our_port;
> +
> +static void fb_getvar(char *);
> +static void fb_download(char *, unsigned int, char *);
> +static void fb_flash(char *);
> +static void fb_erase(char *);
> +static void fb_continue(char *);
> +static void fb_reboot(char *);
> +static void boot_downloaded_image(void);
> +static void cleanup_command_data(void);
> +static void write_fb_response(const char *, const char *, char *);
> +
> +void fastboot_send_info(const char *msg)
> +{
> +       uchar *packet;
> +       uchar *packet_base;
> +       int len = 0;
> +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> +
> +       struct fastboot_header fb_response_header = {
> +               .id = FASTBOOT_FASTBOOT,
> +               .flags = 0,
> +               .seq = htons(fb_sequence_number)
> +       };
> +       ++fb_sequence_number;
> +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> +       packet_base = packet;
> +
> +       /* Write headers */
> +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
> +       packet += sizeof(fb_response_header);
> +       /* Write response */
> +       write_fb_response("INFO", msg, response);
> +       memcpy(packet, response, strlen(response));
> +       packet += strlen(response);
> +
> +       len = packet - packet_base;
> +
> +       /* Save packet for retransmitting */
> +       last_packet_len = len;
> +       memcpy(last_packet, packet_base, last_packet_len);
> +
> +       net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                           fastboot_remote_port, fastboot_our_port, len);
> +}
> +
> +/**
> + * Constructs and sends a packet in response to received fastboot packet
> + *
> + * @param fb_header            Header for response packet
> + * @param fastboot_data        Pointer to received fastboot data
> + * @param fastboot_data_len    Length of received fastboot data
> + * @param retransmit           Nonzero if sending last sent packet
> + */
> +static void fastboot_send(struct fastboot_header fb_header, char
> *fastboot_data,
> +                         unsigned int fastboot_data_len, uchar retransmit)
> +{
> +       uchar *packet;
> +       uchar *packet_base;
> +       int len = 0;
> +       const char *error_msg = "An error occurred.";
> +       short tmp;
> +       struct fastboot_header fb_response_header = fb_header;
> +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> +       /*
> +        *      We will always be sending some sort of packet, so
> +        *      cobble together the packet headers now.
> +        */
> +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> +       packet_base = packet;
> +
> +       /* Resend last packet */
> +       if (retransmit) {
> +               memcpy(packet, last_packet, last_packet_len);
> +               net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                                   fastboot_remote_port,
> fastboot_our_port,
> +                                   last_packet_len);
> +               return;
> +       }
> +
> +       fb_response_header.seq = htons(fb_response_header.seq);
> +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
> +       packet += sizeof(fb_response_header);
> +
> +       switch (fb_header.id) {
> +       case FASTBOOT_QUERY:
> +               tmp = htons(fb_sequence_number);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               break;
> +       case FASTBOOT_INIT:
> +               tmp = htons(fb_udp_version);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               tmp = htons(fb_packet_size);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               break;
> +       case FASTBOOT_ERROR:
> +               memcpy(packet, error_msg, strlen(error_msg));
> +               packet += strlen(error_msg);
> +               break;
> +       case FASTBOOT_FASTBOOT:
> +               if (!cmd_string) {
> +                       /* Parse command and send ack */
> +                       cmd_parameter = fastboot_data;
> +                       cmd_string = strsep(&cmd_parameter, ":");
> +                       cmd_string = strdup(cmd_string);
> +                       if (cmd_parameter)
> +                               cmd_parameter = strdup(cmd_parameter);
> +               } else if (!strcmp("getvar", cmd_string)) {
> +                       fb_getvar(response);
> +               } else if (!strcmp("download", cmd_string)) {
> +                       fb_download(fastboot_data, fastboot_data_len,
> response);
> +               } else if (!strcmp("flash", cmd_string)) {
> +                       fb_flash(response);
> +               } else if (!strcmp("erase", cmd_string)) {
> +                       fb_erase(response);
> +               } else if (!strcmp("boot", cmd_string)) {
> +                       write_fb_response("OKAY", "", response);
> +               } else if (!strcmp("continue", cmd_string)) {
> +                       fb_continue(response);
> +               } else if (!strncmp("reboot", cmd_string, 6)) {
> +                       fb_reboot(response);
> +               } else if (!strcmp("set_active", cmd_string)) {
> +                       /* A/B not implemented, for now do nothing */
> +                       write_fb_response("OKAY", "", response);
> +               } else {
> +                       pr_err("command %s not implemented.\n",
> cmd_string);
> +                       write_fb_response("FAIL", "unrecognized command",
> +                                         response);
> +               }
> +               /* Sent some INFO packets, need to update sequence number
> in
> +                * header
> +                */
> +               if (fb_header.seq != fb_sequence_number) {
> +                       fb_response_header.seq = htons(fb_sequence_number);
> +                       memcpy(packet_base, &fb_response_header,
> +                              sizeof(fb_response_header));
> +               }
> +               /* Write response to packet */
> +               memcpy(packet, response, strlen(response));
> +               packet += strlen(response);
> +               break;
> +       default:
> +               pr_err("ID %d not implemented.\n", fb_header.id);
> +               return;
> +       }
> +
> +       len = packet - packet_base;
> +
> +       /* Save packet for retransmitting */
> +       last_packet_len = len;
> +       memcpy(last_packet, packet_base, last_packet_len);
> +
> +       net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                           fastboot_remote_port, fastboot_our_port, len);
> +
> +       /* Continue boot process after sending response */
> +       if (!strncmp("OKAY", response, 4)) {
> +               if (!strcmp("boot", cmd_string)) {
> +                       boot_downloaded_image();
> +               } else if (!strcmp("continue", cmd_string)) {
> +                       run_command(env_get("bootcmd"), CMD_FLAG_ENV);
> +               } else if (!strncmp("reboot", cmd_string, 6)) {
> +                       /* Matches reboot or reboot-bootloader */
> +                       do_reset(NULL, 0, 0, NULL);
> +               }
> +       }
> +
> +       /* OKAY and FAIL indicate command is complete */
> +       if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4))
> +               cleanup_command_data();
> +}
> +
> +/**
> + * Writes ascii string specified by cmd_parameter to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_getvar(char *response)
> +{
> +       if (!cmd_parameter) {
> +               write_fb_response("FAIL", "missing var", response);
> +       } else if (!strcmp("version", cmd_parameter)) {
> +               write_fb_response("OKAY", FASTBOOT_VERSION, response);
> +       } else if (!strcmp("bootloader-version", cmd_parameter) ||
> +                  !strcmp("version-bootloader", cmd_parameter)) {
> +               write_fb_response("OKAY", U_BOOT_VERSION, response);
> +       } else if (!strcmp("downloadsize", cmd_parameter) ||
> +                  !strcmp("max-download-size", cmd_parameter)) {
> +               char buf_size_str[12];
> +
> +               sprintf(buf_size_str, "0x%08x", CONFIG_FASTBOOT_BUF_SIZE);
> +               write_fb_response("OKAY", buf_size_str, response);
> +       } else if (!strcmp("serialno", cmd_parameter)) {
> +               const char *tmp = env_get("serial#");
> +
> +               if (tmp)
> +                       write_fb_response("OKAY", tmp, response);
> +               else
> +                       write_fb_response("FAIL", "Value not set",
> response);
> +       } else if (!strcmp("version-baseband", cmd_parameter)) {
> +               write_fb_response("OKAY", "N/A", response);
> +       } else if (!strcmp("product", cmd_parameter)) {
> +               const char *board = env_get("board");
> +
> +               if (board)
> +                       write_fb_response("OKAY", board, response);
> +               else
> +                       write_fb_response("FAIL", "Board not set",
> response);
> +       } else if (!strcmp("current-slot", cmd_parameter)) {
> +               /* A/B not implemented, for now always return _a */
> +               write_fb_response("OKAY", "_a", response);
> +       } else if (!strcmp("slot-suffixes", cmd_parameter)) {
> +               write_fb_response("OKAY", "_a,_b", response);
> +       } else if (!strncmp("has-slot", cmd_parameter, 8)) {
> +               char *part_name = cmd_parameter;
> +
> +               cmd_parameter = strsep(&part_name, ":");
> +               if (!strcmp(part_name, "boot") || !strcmp(part_name,
> "system"))
> +                       write_fb_response("OKAY", "yes", response);
> +               else
> +                       write_fb_response("OKAY", "no", response);
> +       } else if (!strncmp("partition-type", cmd_parameter, 14) ||
> +                  !strncmp("partition-size", cmd_parameter, 14)) {
> +               disk_partition_t part_info;
> +               struct blk_desc *dev_desc;
> +               char *part_name = cmd_parameter;
> +               char part_size_str[20];
> +
> +               cmd_parameter = strsep(&part_name, ":");
> +               dev_desc = blk_get_dev("mmc", 0);
> +               if (!dev_desc) {
> +                       write_fb_response("FAIL", "block device not found",
> +                                         response);
> +               } else if (part_get_info_by_name(dev_desc, part_name,
> +                                                &part_info) < 0) {
> +                       write_fb_response("FAIL", "partition not found",
> +                                         response);
> +               } else if (!strncmp("partition-type", cmd_parameter, 14)) {
> +                       write_fb_response("OKAY", (char *)part_info.type,
> +                                         response);
> +               } else if (!strncmp("partition-size", cmd_parameter, 14)) {
> +                       sprintf(part_size_str, "0x%016x",
> (int)part_info.size);
> +                       write_fb_response("OKAY", part_size_str, response);
> +               }
> +       } else {
> +               printf("WARNING: unknown variable: %s\n", cmd_parameter);
> +               write_fb_response("FAIL", "Variable not implemented",
> +                                 response);
> +       }
> +}
> +
> +/**
> + * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
> + * Writes to response.
> + *
> + * @param fastboot_data        Pointer to received fastboot data
> + * @param fastboot_data_len    Length of received fastboot data
> + * @param repsonse             Pointer to fastboot response buffer
> + */
> +static void fb_download(char *fastboot_data, unsigned int
> fastboot_data_len,
> +                       char *response)
> +{
> +       char *tmp;
> +
> +       if (bytes_expected == 0) {
> +               if (!cmd_parameter) {
> +                       write_fb_response("FAIL", "Expected command
> parameter",
> +                                         response);
> +                       return;
> +               }
> +               bytes_expected = simple_strtoul(cmd_parameter, &tmp, 16);
> +               if (bytes_expected == 0) {
> +                       write_fb_response("FAIL", "Expected nonzero image
> size",
> +                                         response);
> +                       return;
> +               }
> +       }
> +       if (fastboot_data_len == 0 && bytes_received == 0) {
> +               /* Nothing to download yet. Response is of the form:
> +                * [DATA|FAIL]$cmd_parameter
> +                *
> +                * where cmd_parameter is an 8 digit hexadecimal number
> +                */
> +               if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
> +                       write_fb_response("FAIL", cmd_parameter, response);
> +               else
> +                       write_fb_response("DATA", cmd_parameter, response);
> +       } else if (fastboot_data_len == 0 &&
> +                  (bytes_received >= bytes_expected)) {
> +               /* Download complete. Respond with "OKAY" */
> +               write_fb_response("OKAY", "", response);
> +               image_size = bytes_received;
> +               bytes_expected = 0;
> +               bytes_received = 0;
> +       } else {
> +               if (fastboot_data_len == 0 ||
> +                   (bytes_received + fastboot_data_len) > bytes_expected)
> {
> +                       write_fb_response("FAIL",
> +                                         "Received invalid data length",
> +                                         response);
> +                       return;
> +               }
> +               /* Download data to CONFIG_FASTBOOT_BUF_ADDR */
> +               memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + bytes_received,
> +                      fastboot_data, fastboot_data_len);
> +               bytes_received += fastboot_data_len;
> +       }
> +}
> +
> +/**
> + * Writes the previously downloaded image to the partition indicated by
> + * cmd_parameter. Writes to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_flash(char *response)
> +{
> +       fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
> +                          image_size, response);
> +}
> +
> +/**
> + * Erases the partition indicated by cmd_parameter (clear to 0x00s).
> Writes
> + * to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_erase(char *response)
> +{
> +       fb_mmc_erase(cmd_parameter, response);
> +}
> +
> +/**
> + * Continues normal boot process by running "bootcmd". Writes
> + * to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_continue(char *response)
> +{
> +       char *bootcmd;
> +
> +       bootcmd = env_get("bootcmd");
> +       if (bootcmd)
> +               write_fb_response("OKAY", "", response);
> +       else
> +               write_fb_response("FAIL", "bootcmd not set", response);
> +}
> +
> +/**
> + * Sets reboot bootloader flag if requested. Writes to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_reboot(char *response)
> +{
> +       write_fb_response("OKAY", "", response);
> +       if (!strcmp("reboot-bootloader", cmd_string))
> +               strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR,
> "reboot-bootloader");
> +}
> +
> +/**
> + * Boots into downloaded image.
> + */
> +static void boot_downloaded_image(void)
> +{
> +       char kernel_addr[12];
> +       char *fdt_addr = env_get("fdt_addr_r");
> +       char *const bootm_args[] = {
> +               "bootm", kernel_addr, "-", fdt_addr, NULL
> +       };
> +
> +       sprintf(kernel_addr, "0x%lx", (long)CONFIG_FASTBOOT_BUF_ADDR);
> +
> +       printf("\nBooting kernel at %s with fdt at %s...\n\n\n",
> +              kernel_addr, fdt_addr);
> +       do_bootm(NULL, 0, 4, bootm_args);
> +
> +       /* This only happens if image is faulty so we start over. */
> +       do_reset(NULL, 0, 0, NULL);
> +}
> +
> +/**
> + * Writes a response to response buffer of the form "$tag$reason".
> + *
> + * @param tag         The first part of the response
> + * @param reason      The second part of the response
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void write_fb_response(const char *tag, const char *reason,
> +                             char *response)
> +{
> +       strncpy(response, tag, strlen(tag));
> +       strncat(response, reason, FASTBOOT_RESPONSE_LEN - strlen(tag) - 1);
> +}
> +
> +/**
> + * Frees any resources allocated during current fastboot command.
> + */
> +static void cleanup_command_data(void)
> +{
> +       /* cmd_parameter and cmd_string potentially point to memory
> allocated by
> +        * strdup
> +        */
> +       if (cmd_parameter)
> +               free(cmd_parameter);
> +       if (cmd_string)
> +               free(cmd_string);
> +       cmd_parameter = NULL;
> +       cmd_string = NULL;
> +}
> +
> +/**
> + * Incoming UDP packet handler.
> + *
> + * @param packet  Pointer to incoming UDP packet
> + * @param dport   Destination UDP port
> + * @param sip     Source IP address
> + * @param sport   Source UDP port
> + * @param len     Packet length
> + */
> +static void fastboot_handler(uchar *packet, unsigned int dport,
> +                            struct in_addr sip, unsigned int sport,
> +                            unsigned int len)
> +{
> +       struct fastboot_header fb_header;
> +       char fastboot_data[DATA_SIZE] = {0};
> +       unsigned int fastboot_data_len = 0;
> +
> +       if (dport != fastboot_our_port)
> +               return;
> +
> +       fastboot_remote_ip = sip;
> +       fastboot_remote_port = sport;
> +
> +       if (len < FASTBOOT_HEADER_SIZE || len > PACKET_SIZE)
> +               return;
> +       memcpy(&fb_header, packet, sizeof(fb_header));
> +       fb_header.flags = 0;
> +       fb_header.seq = ntohs(fb_header.seq);
> +       packet += sizeof(fb_header);
> +       len -= sizeof(fb_header);
> +
> +       switch (fb_header.id) {
> +       case FASTBOOT_QUERY:
> +               fastboot_send(fb_header, fastboot_data, 0, 0);
> +               break;
> +       case FASTBOOT_INIT:
> +       case FASTBOOT_FASTBOOT:
> +               fastboot_data_len = len;
> +               if (len > 0)
> +                       memcpy(fastboot_data, packet, len);
> +               if (fb_header.seq == fb_sequence_number) {
> +                       fastboot_send(fb_header, fastboot_data,
> +                                     fastboot_data_len, 0);
> +                       fb_sequence_number++;
> +               } else if (fb_header.seq == fb_sequence_number - 1) {
> +                       /* Retransmit last sent packet */
> +                       fastboot_send(fb_header, fastboot_data,
> +                                     fastboot_data_len, 1);
> +               }
> +               break;
> +       default:
> +               pr_err("ID %d not implemented.\n", fb_header.id);
> +               fb_header.id = FASTBOOT_ERROR;
> +               fastboot_send(fb_header, fastboot_data, 0, 0);
> +               break;
> +       }
> +}
> +
> +void fastboot_start_server(void)
> +{
> +       printf("Using %s device\n", eth_get_name());
> +       printf("Listening for fastboot command on %pI4\n", &net_ip);
> +
> +       fastboot_our_port = WELL_KNOWN_PORT;
> +
> +       net_set_udp_handler(fastboot_handler);
> +
> +       /* zero out server ether in case the server ip has changed */
> +       memset(net_server_ethaddr, 0, 6);
> +}
> diff --git a/net/net.c b/net/net.c
> index d222c1f..554df7a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -87,6 +87,9 @@
>  #include <environment.h>
>  #include <errno.h>
>  #include <net.h>
> +#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
> +#include <net/fastboot.h>
> +#endif
>  #include <net/tftp.h>
>  #if defined(CONFIG_LED_STATUS)
>  #include <miiphy.h>
> @@ -451,6 +454,11 @@ restart:
>                         tftp_start_server();
>                         break;
>  #endif
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +               case FASTBOOT:
> +                       fastboot_start_server();
> +                       break;
> +#endif
>  #if defined(CONFIG_CMD_DHCP)
>                 case DHCP:
>                         bootp_reset();
> @@ -1322,6 +1330,7 @@ common:
>                 /* Fall through */
>
>         case NETCONS:
> +       case FASTBOOT:
>         case TFTPSRV:
>                 if (net_ip.s_addr == 0) {
>                         puts("*** ERROR: `ipaddr' not set\n");
> --
> 2.7.4
>
>
 Signed-off-by: Jocelyn Bohr <bohr@google.com>

On Mon, Apr 30, 2018 at 3:32 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>
> ---
>
> Changes in v2:
> - ensure fastboot syntax is backward compatible - 'fastboot 0' means
>   'fastboot usb 0'
>
>  cmd/fastboot.c               |  35 ++-
>  cmd/net.c                    |   6 +
>  drivers/fastboot/Kconfig     |  16 +-
>  drivers/fastboot/fb_common.c |  18 ++
>  drivers/fastboot/fb_mmc.c    |  34 ++-
>  include/fastboot.h           |  13 ++
>  include/net.h                |   6 +-
>  include/net/fastboot.h       |  27 +++
>  net/Makefile                 |   1 +
>  net/fastboot.c               | 542 +++++++++++++++++++++++++++++++++++++++++++
>  net/net.c                    |   9 +
>  11 files changed, 695 insertions(+), 12 deletions(-)
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index 8adcca5..68a41de 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -11,17 +11,41 @@
>  #include <command.h>
>  #include <console.h>
>  #include <g_dnl.h>
> +#include <net.h>
>  #include <usb.h>
>
>  static int do_fastboot(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>  {
> +#ifdef CONFIG_USB_FUNCTION_FASTBOOT
>         int controller_index;
>         char *usb_controller;
>         int ret;
> +#endif
>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
>
> +       if (!strcmp(argv[1], "udp")) {
> +#ifndef CONFIG_UDP_FUNCTION_FASTBOOT
> +               pr_err("Fastboot UDP not enabled\n");
> +               return -1;

Commands should always return an enum command_ret_t.

> +#else
> +               return do_fastboot_udp(cmdtp, flag, argc, argv);
> +#endif
> +       }
> +
> +       if (!strcmp(argv[1], "usb")) {
> +               argv++;
> +               argc--;
> +       }
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +#ifndef CONFIG_USB_FUNCTION_FASTBOOT
> +       pr_err("Fastboot USB not enabled\n");
> +       return -1;

Commands should always return an enum command_ret_t.

> +#else
>         usb_controller = argv[1];
>         controller_index = simple_strtoul(usb_controller, NULL, 0);
>
> @@ -59,11 +83,14 @@ exit:
>         board_usb_cleanup(controller_index, USB_INIT_DEVICE);
>
>         return ret;
> +#endif
>  }
>
>  U_BOOT_CMD(
> -       fastboot, 2, 1, do_fastboot,
> -       "use USB Fastboot protocol",
> -       "<USB_controller>\n"
> -       "    - run as a fastboot usb device"
> +       fastboot, 3, 1, do_fastboot,
> +       "use USB or UDP Fastboot protocol",
> +       "[usb,udp] <USB_controller>\n"
> +       " - run as a fastboot usb or udp device\n"
> +       "   usb: specify <USB_controller>\n"
> +       "   udp: requires ip_addr set and ethernet initialized\n"
>  );
> diff --git a/cmd/net.c b/cmd/net.c
> index 67888d4..668f344 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -74,6 +74,12 @@ U_BOOT_CMD(
>  );
>  #endif
>
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])

You should move this to cmd/fastboot.c and make it static.

> +{
> +       return netboot_common(FASTBOOT, cmdtp, argc, argv);

Is this really what you want? You are passing these command
parameters, but you don't really want them to be interpreted (as a
load address or file name), right?

I think you just want this:

        int err = net_loop(FASTBOOT);

        if (err < 0) {
                printf("fastboot udp error: %i\n", err);
                return CMD_RET_FAILURE;
        }

        return CMD_RET_SUCCESS;


> +}
> +#endif
>
>  #ifdef CONFIG_CMD_RARP
>  int do_rarpb(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig
> index 64b94f0..53c337f 100644
> --- a/drivers/fastboot/Kconfig
> +++ b/drivers/fastboot/Kconfig
> @@ -2,6 +2,8 @@ menu "Fastboot support"
>
>  config FASTBOOT
>         bool
> +       imply ANDROID_BOOT_IMAGE
> +       imply CMD_FASTBOOT
>
>  config USB_FUNCTION_FASTBOOT
>         bool "Enable USB fastboot gadget"
> @@ -9,12 +11,17 @@ config USB_FUNCTION_FASTBOOT
>         default y if ARCH_SUNXI && USB_MUSB_GADGET
>         select FASTBOOT
>         select USB_GADGET_DOWNLOAD
> -       imply ANDROID_BOOT_IMAGE
> -       imply CMD_FASTBOOT
>         help
>           This enables the USB part of the fastboot gadget.
>
> -if USB_FUNCTION_FASTBOOT
> +config UDP_FUNCTION_FASTBOOT
> +       depends on NET

This is the correct level of dependency once you start using net_loop
directly, but based on how you have it now, you would have to depend
on CMD_NET.

> +       select FASTBOOT
> +       bool "Enable fastboot protocol over UDP"
> +       help
> +         This enables the fastboot protocol over UDP.
> +
> +if USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
>
>  config FASTBOOT_BUF_ADDR
>         hex "Define FASTBOOT buffer address"
> @@ -46,6 +53,7 @@ config FASTBOOT_BUF_SIZE
>
>  config FASTBOOT_USB_DEV
>         int "USB controller number"
> +       depends on USB_FUNCTION_FASTBOOT
>         default 0
>         help
>           Some boards have USB OTG controller other than 0. Define this
> @@ -117,6 +125,6 @@ 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".
>
> -endif # USB_FUNCTION_FASTBOOT
> +endif # USB_FUNCTION_FASTBOOT || UDP_FUNCTION_FASTBOOT
>
>  endmenu
> diff --git a/drivers/fastboot/fb_common.c b/drivers/fastboot/fb_common.c
> index fe58803..7367fbb 100644
> --- a/drivers/fastboot/fb_common.c
> +++ b/drivers/fastboot/fb_common.c
> @@ -12,6 +12,9 @@
>
>  #include <common.h>
>  #include <fastboot.h>
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +#include <net/fastboot.h>
> +#endif
>
>  /**
>   * Writes a response to response buffer of the form "$tag$reason".
> @@ -44,3 +47,18 @@ void fastboot_okay(const char *reason, char *response)
>  {
>         fastboot_response("OKAY", response, "%s", reason);
>  }
> +
> +void timed_send_info(ulong *start, const char *msg)

This should be called fastboot_send_info()... it is in the global namespace!

> +{
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +       /* Initialize timer */
> +       if (*start == 0)

You should probably be a little more defensive and check for NULL
before dereferencing.

> +               *start = get_timer(0);
> +       ulong time = get_timer(*start);
> +       /* Send INFO packet to host every 30 seconds */
> +       if (time >= 30000) {
> +               *start = get_timer(0);
> +               fastboot_send_info(msg);

This should call fastboot_(udp_)send_info()

> +       }
> +#endif
> +}
> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
> index 02864aa..304bda1 100644
> --- a/drivers/fastboot/fb_mmc.c
> +++ b/drivers/fastboot/fb_mmc.c
> @@ -29,6 +29,9 @@
>  #define CONFIG_FASTBOOT_MBR_NAME "mbr"
>  #endif
>
> +#define FASTBOOT_MAX_BLK_WRITE 16384
> +static ulong timer;

This timer seems like it should be a static variable inside of
fastboot_send_info() (the function currently called timed_send_info).
Then you don't have to pass it as a parameter and fb_mmc.c doesn't
need to know what fb_common is doing with the info update.

> +
>  #define BOOT_PARTITION_NAME "boot"
>
>  struct fb_mmc_sparse {
> @@ -57,13 +60,38 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc,
>         return ret;
>  }
>
> +static lbaint_t fb_mmc_blk_write(struct blk_desc *block_dev, lbaint_t start,
> +                                lbaint_t blkcnt, const void *buffer)
> +{
> +       lbaint_t blk = start;
> +       lbaint_t blks_written;
> +       lbaint_t cur_blkcnt;
> +       lbaint_t blks = 0;
> +       int i;
> +
> +       for (i = 0; i < blkcnt; i += FASTBOOT_MAX_BLK_WRITE) {
> +               cur_blkcnt = min((int)blkcnt - i, FASTBOOT_MAX_BLK_WRITE);
> +               if (!buffer) {
> +                       timed_send_info(&timer, "writing");
> +                       blks_written = blk_dwrite(block_dev, blk, cur_blkcnt,
> +                                                 buffer + (i * block_dev->blksz));
> +               } else {
> +                       timed_send_info(&timer, "erasing");
> +                       blks_written = blk_derase(block_dev, blk, cur_blkcnt);
> +               }
> +               blk += blks_written;
> +               blks += blks_written;
> +       }
> +       return blks;
> +}
> +
>  static lbaint_t fb_mmc_sparse_write(struct sparse_storage *info,
>                 lbaint_t blk, lbaint_t blkcnt, const void *buffer)
>  {
>         struct fb_mmc_sparse *sparse = info->priv;
>         struct blk_desc *dev_desc = sparse->dev_desc;
>
> -       return blk_dwrite(dev_desc, blk, blkcnt, buffer);
> +       return fb_mmc_blk_write(dev_desc, blk, blkcnt, buffer);
>  }
>
>  static lbaint_t fb_mmc_sparse_reserve(struct sparse_storage *info,
> @@ -91,7 +119,7 @@ static void write_raw_image(struct blk_desc *dev_desc, disk_partition_t *info,
>
>         puts("Flashing Raw Image\n");
>
> -       blks = blk_dwrite(dev_desc, info->start, blkcnt, buffer);
> +       blks = fb_mmc_blk_write(dev_desc, info->start, blkcnt, buffer);
>         if (blks != blkcnt) {
>                 pr_err("failed writing to device %d\n", dev_desc->devnum);
>                 fastboot_fail("failed writing to device", response);
> @@ -399,7 +427,7 @@ void fb_mmc_erase(const char *cmd, char *response)
>         printf("Erasing blocks " LBAFU " to " LBAFU " due to alignment\n",
>                blks_start, blks_start + blks_size);
>
> -       blks = blk_derase(dev_desc, blks_start, blks_size);
> +       blks = fb_mmc_blk_write(dev_desc, blks_start, blks_size, NULL);
>         if (blks != blks_size) {
>                 pr_err("failed erasing from device %d", dev_desc->devnum);
>                 fastboot_fail("failed erasing from device", response);
> diff --git a/include/fastboot.h b/include/fastboot.h
> index 2140c94..6f69423 100644
> --- a/include/fastboot.h
> +++ b/include/fastboot.h
> @@ -23,4 +23,17 @@ void fastboot_response(const char *tag, char *response,
>  void fastboot_fail(const char *reason, char *response);
>  void fastboot_okay(const char *reason, char *response);
>
> +/**
> + * Send an INFO packet during long commands based on timer. If
> + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
> + * if the time is 30 seconds after start. Else, noop.
> + *
> + * TODO: Handle the situation where both UDP and USB fastboot are
> + *       enabled.
> + *
> + * @param start:  Time since last INFO packet was sent.
> + * @param msg:    String describing the reason for waiting
> + */
> +void timed_send_info(ulong *start, const char *msg);
> +
>  #endif /* _FASTBOOT_H_ */
> diff --git a/include/net.h b/include/net.h
> index 3469811..890ae27 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -535,7 +535,7 @@ extern int          net_restart_wrap;       /* Tried all network devices */
>
>  enum proto_t {
>         BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
> -       TFTPSRV, TFTPPUT, LINKLOCAL
> +       TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT
>  };
>
>  extern char    net_boot_file_name[1024];/* Boot File name */
> @@ -549,6 +549,10 @@ extern char *net_dns_resolve;              /* The host to resolve  */
>  extern char *net_dns_env_var;          /* the env var to put the ip into */
>  #endif
>
> +#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)

Probably not necessary to guard - the linker would complain if it's not there.

> +int do_fastboot_udp(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);

This would be better as a static function in cmd/fastboot.c

> +#endif
> +
>  #if defined(CONFIG_CMD_PING)
>  extern struct in_addr net_ping_ip;     /* the ip address to ping */
>  #endif
> diff --git a/include/net/fastboot.h b/include/net/fastboot.h
> new file mode 100644
> index 0000000..c0dd033
> --- /dev/null
> +++ b/include/net/fastboot.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: BSD-2-Clause
> + *
> + * Copyright (C) 2016 The Android Open Source Project
> + */
> +
> +#ifndef __NET_FASTBOOT_H__
> +#define __NET_FASTBOOT_H__
> +
> +/**********************************************************************/
> +/*
> + *     Global functions and variables.
> + */
> +
> +/**
> + * Wait for incoming fastboot comands.
> + */
> +void fastboot_start_server(void);

Since this is in the global namespace, it should probably be
qualified... fastboot_udp_start_server()

> +/**
> + * Send an INFO packet during long commands
> + *
> + * @param msg: String describing the reason for waiting
> + */
> +void fastboot_send_info(const char *msg);

Same here... fastboot_udp_send_info()

> +
> +/**********************************************************************/
> +
> +#endif /* __NET_FASTBOOT_H__ */
> diff --git a/net/Makefile b/net/Makefile
> index ce6e5ad..3489ce5 100644
> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_CMD_PING) += ping.o
>  obj-$(CONFIG_CMD_RARP) += rarp.o
>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>  obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
> +obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot.o
>
>  # Disable this warning as it is triggered by:
>  # sprintf(buf, index ? "foo%d" : "foo", index)
> diff --git a/net/fastboot.c b/net/fastboot.c
> new file mode 100644
> index 0000000..32cb581
> --- /dev/null
> +++ b/net/fastboot.c
> @@ -0,0 +1,542 @@
> +// 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 <net.h>
> +#include <net/fastboot.h>
> +#include <part.h>
> +#include <stdlib.h>
> +#include <version.h>
> +
> +/* Fastboot port # defined in spec */
> +#define WELL_KNOWN_PORT 5554
> +
> +enum {
> +       FASTBOOT_ERROR = 0,
> +       FASTBOOT_QUERY = 1,
> +       FASTBOOT_INIT = 2,
> +       FASTBOOT_FASTBOOT = 3,
> +};
> +
> +struct __packed fastboot_header {
> +       uchar id;
> +       uchar flags;
> +       unsigned short seq;
> +};
> +
> +#define PACKET_SIZE 1024
> +#define FASTBOOT_HEADER_SIZE sizeof(struct fastboot_header)

This define doesn't seem any better than just using the sizeof() directly.

> +#define DATA_SIZE (PACKET_SIZE - FASTBOOT_HEADER_SIZE)
> +#define FASTBOOT_VERSION "0.4"
> +
> +/* Sequence number sent for every packet */
> +static unsigned short fb_sequence_number = 1;
> +static const unsigned short fb_packet_size = PACKET_SIZE;
> +static const unsigned short fb_udp_version = 1;
> +
> +/* Keep track of last packet for resubmission */
> +static uchar last_packet[PACKET_SIZE];
> +static unsigned int last_packet_len;
> +
> +/* Parsed from first fastboot command packet */
> +static char *cmd_string;
> +static char *cmd_parameter;
> +
> +/* Fastboot download parameters */
> +static unsigned int bytes_received;
> +static unsigned int bytes_expected;

These two are only ever used in fb_download(). They should be static
variables in that function.

> +static unsigned int image_size;
> +
> +static struct in_addr fastboot_remote_ip;
> +/* The UDP port at their end */
> +static int fastboot_remote_port;
> +/* The UDP port at our end */
> +static int fastboot_our_port;
> +
> +static void fb_getvar(char *);
> +static void fb_download(char *, unsigned int, char *);
> +static void fb_flash(char *);
> +static void fb_erase(char *);
> +static void fb_continue(char *);
> +static void fb_reboot(char *);
> +static void boot_downloaded_image(void);
> +static void cleanup_command_data(void);
> +static void write_fb_response(const char *, const char *, char *);
> +
> +void fastboot_send_info(const char *msg)
> +{
> +       uchar *packet;
> +       uchar *packet_base;
> +       int len = 0;
> +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> +
> +       struct fastboot_header fb_response_header = {
> +               .id = FASTBOOT_FASTBOOT,
> +               .flags = 0,
> +               .seq = htons(fb_sequence_number)
> +       };
> +       ++fb_sequence_number;
> +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> +       packet_base = packet;
> +
> +       /* Write headers */
> +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
> +       packet += sizeof(fb_response_header);
> +       /* Write response */
> +       write_fb_response("INFO", msg, response);
> +       memcpy(packet, response, strlen(response));
> +       packet += strlen(response);
> +
> +       len = packet - packet_base;
> +
> +       /* Save packet for retransmitting */
> +       last_packet_len = len;
> +       memcpy(last_packet, packet_base, last_packet_len);
> +
> +       net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                           fastboot_remote_port, fastboot_our_port, len);
> +}
> +
> +/**
> + * Constructs and sends a packet in response to received fastboot packet
> + *
> + * @param fb_header            Header for response packet
> + * @param fastboot_data        Pointer to received fastboot data
> + * @param fastboot_data_len    Length of received fastboot data
> + * @param retransmit           Nonzero if sending last sent packet
> + */
> +static void fastboot_send(struct fastboot_header fb_header, char *fastboot_data,
> +                         unsigned int fastboot_data_len, uchar retransmit)
> +{
> +       uchar *packet;
> +       uchar *packet_base;
> +       int len = 0;
> +       const char *error_msg = "An error occurred.";
> +       short tmp;
> +       struct fastboot_header fb_response_header = fb_header;
> +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> +       /*
> +        *      We will always be sending some sort of packet, so
> +        *      cobble together the packet headers now.
> +        */
> +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> +       packet_base = packet;
> +
> +       /* Resend last packet */
> +       if (retransmit) {
> +               memcpy(packet, last_packet, last_packet_len);
> +               net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                                   fastboot_remote_port, fastboot_our_port,
> +                                   last_packet_len);
> +               return;
> +       }
> +
> +       fb_response_header.seq = htons(fb_response_header.seq);
> +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
> +       packet += sizeof(fb_response_header);
> +
> +       switch (fb_header.id) {
> +       case FASTBOOT_QUERY:
> +               tmp = htons(fb_sequence_number);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               break;
> +       case FASTBOOT_INIT:
> +               tmp = htons(fb_udp_version);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               tmp = htons(fb_packet_size);
> +               memcpy(packet, &tmp, sizeof(tmp));
> +               packet += sizeof(tmp);
> +               break;
> +       case FASTBOOT_ERROR:
> +               memcpy(packet, error_msg, strlen(error_msg));
> +               packet += strlen(error_msg);
> +               break;
> +       case FASTBOOT_FASTBOOT:
> +               if (!cmd_string) {
> +                       /* Parse command and send ack */
> +                       cmd_parameter = fastboot_data;

This seems unnecessary. There are only 4 cases handled, and of them
only download seems to be a command that happens more than once. And
in download, the first past through this parameter is saved internally
as bytes_expected.

> +                       cmd_string = strsep(&cmd_parameter, ":");
> +                       cmd_string = strdup(cmd_string);
> +                       if (cmd_parameter)
> +                               cmd_parameter = strdup(cmd_parameter);
> +               } else if (!strcmp("getvar", cmd_string)) {
> +                       fb_getvar(response);

Seems like you should simply pass the "fastboot_data" as a parameter
here rather than using a global.

> +               } else if (!strcmp("download", cmd_string)) {
> +                       fb_download(fastboot_data, fastboot_data_len, response);
> +               } else if (!strcmp("flash", cmd_string)) {
> +                       fb_flash(response);
> +               } else if (!strcmp("erase", cmd_string)) {
> +                       fb_erase(response);
> +               } else if (!strcmp("boot", cmd_string)) {
> +                       write_fb_response("OKAY", "", response);
> +               } else if (!strcmp("continue", cmd_string)) {
> +                       fb_continue(response);
> +               } else if (!strncmp("reboot", cmd_string, 6)) {
> +                       fb_reboot(response);
> +               } else if (!strcmp("set_active", cmd_string)) {
> +                       /* A/B not implemented, for now do nothing */
> +                       write_fb_response("OKAY", "", response);
> +               } else {
> +                       pr_err("command %s not implemented.\n", cmd_string);
> +                       write_fb_response("FAIL", "unrecognized command",
> +                                         response);
> +               }
> +               /* Sent some INFO packets, need to update sequence number in
> +                * header
> +                */
> +               if (fb_header.seq != fb_sequence_number) {
> +                       fb_response_header.seq = htons(fb_sequence_number);
> +                       memcpy(packet_base, &fb_response_header,
> +                              sizeof(fb_response_header));
> +               }
> +               /* Write response to packet */
> +               memcpy(packet, response, strlen(response));
> +               packet += strlen(response);
> +               break;
> +       default:
> +               pr_err("ID %d not implemented.\n", fb_header.id);
> +               return;
> +       }
> +
> +       len = packet - packet_base;
> +
> +       /* Save packet for retransmitting */
> +       last_packet_len = len;
> +       memcpy(last_packet, packet_base, last_packet_len);
> +
> +       net_send_udp_packet(net_server_ethaddr, fastboot_remote_ip,
> +                           fastboot_remote_port, fastboot_our_port, len);
> +
> +       /* Continue boot process after sending response */
> +       if (!strncmp("OKAY", response, 4)) {
> +               if (!strcmp("boot", cmd_string)) {
> +                       boot_downloaded_image();
> +               } else if (!strcmp("continue", cmd_string)) {
> +                       run_command(env_get("bootcmd"), CMD_FLAG_ENV);
> +               } else if (!strncmp("reboot", cmd_string, 6)) {
> +                       /* Matches reboot or reboot-bootloader */
> +                       do_reset(NULL, 0, 0, NULL);
> +               }
> +       }
> +
> +       /* OKAY and FAIL indicate command is complete */
> +       if (!strncmp("OKAY", response, 4) || !strncmp("FAIL", response, 4))
> +               cleanup_command_data();
> +}
> +
> +/**
> + * Writes ascii string specified by cmd_parameter to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_getvar(char *response)
> +{
> +       if (!cmd_parameter) {
> +               write_fb_response("FAIL", "missing var", response);
> +       } else if (!strcmp("version", cmd_parameter)) {
> +               write_fb_response("OKAY", FASTBOOT_VERSION, response);
> +       } else if (!strcmp("bootloader-version", cmd_parameter) ||
> +                  !strcmp("version-bootloader", cmd_parameter)) {
> +               write_fb_response("OKAY", U_BOOT_VERSION, response);
> +       } else if (!strcmp("downloadsize", cmd_parameter) ||
> +                  !strcmp("max-download-size", cmd_parameter)) {
> +               char buf_size_str[12];
> +
> +               sprintf(buf_size_str, "0x%08x", CONFIG_FASTBOOT_BUF_SIZE);
> +               write_fb_response("OKAY", buf_size_str, response);
> +       } else if (!strcmp("serialno", cmd_parameter)) {
> +               const char *tmp = env_get("serial#");
> +
> +               if (tmp)
> +                       write_fb_response("OKAY", tmp, response);
> +               else
> +                       write_fb_response("FAIL", "Value not set", response);
> +       } else if (!strcmp("version-baseband", cmd_parameter)) {
> +               write_fb_response("OKAY", "N/A", response);
> +       } else if (!strcmp("product", cmd_parameter)) {
> +               const char *board = env_get("board");
> +
> +               if (board)
> +                       write_fb_response("OKAY", board, response);
> +               else
> +                       write_fb_response("FAIL", "Board not set", response);
> +       } else if (!strcmp("current-slot", cmd_parameter)) {
> +               /* A/B not implemented, for now always return _a */
> +               write_fb_response("OKAY", "_a", response);
> +       } else if (!strcmp("slot-suffixes", cmd_parameter)) {
> +               write_fb_response("OKAY", "_a,_b", response);
> +       } else if (!strncmp("has-slot", cmd_parameter, 8)) {
> +               char *part_name = cmd_parameter;
> +
> +               cmd_parameter = strsep(&part_name, ":");
> +               if (!strcmp(part_name, "boot") || !strcmp(part_name, "system"))
> +                       write_fb_response("OKAY", "yes", response);
> +               else
> +                       write_fb_response("OKAY", "no", response);
> +       } else if (!strncmp("partition-type", cmd_parameter, 14) ||
> +                  !strncmp("partition-size", cmd_parameter, 14)) {
> +               disk_partition_t part_info;
> +               struct blk_desc *dev_desc;
> +               char *part_name = cmd_parameter;
> +               char part_size_str[20];
> +
> +               cmd_parameter = strsep(&part_name, ":");
> +               dev_desc = blk_get_dev("mmc", 0);
> +               if (!dev_desc) {
> +                       write_fb_response("FAIL", "block device not found",
> +                                         response);
> +               } else if (part_get_info_by_name(dev_desc, part_name,
> +                                                &part_info) < 0) {
> +                       write_fb_response("FAIL", "partition not found",
> +                                         response);
> +               } else if (!strncmp("partition-type", cmd_parameter, 14)) {
> +                       write_fb_response("OKAY", (char *)part_info.type,
> +                                         response);
> +               } else if (!strncmp("partition-size", cmd_parameter, 14)) {
> +                       sprintf(part_size_str, "0x%016x", (int)part_info.size);
> +                       write_fb_response("OKAY", part_size_str, response);
> +               }
> +       } else {
> +               printf("WARNING: unknown variable: %s\n", cmd_parameter);
> +               write_fb_response("FAIL", "Variable not implemented",
> +                                 response);
> +       }
> +}
> +
> +/**
> + * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
> + * Writes to response.
> + *
> + * @param fastboot_data        Pointer to received fastboot data
> + * @param fastboot_data_len    Length of received fastboot data
> + * @param repsonse             Pointer to fastboot response buffer
> + */
> +static void fb_download(char *fastboot_data, unsigned int fastboot_data_len,
> +                       char *response)
> +{
> +       char *tmp;
> +
> +       if (bytes_expected == 0) {
> +               if (!cmd_parameter) {
> +                       write_fb_response("FAIL", "Expected command parameter",
> +                                         response);
> +                       return;
> +               }
> +               bytes_expected = simple_strtoul(cmd_parameter, &tmp, 16);
> +               if (bytes_expected == 0) {
> +                       write_fb_response("FAIL", "Expected nonzero image size",
> +                                         response);
> +                       return;
> +               }
> +       }
> +       if (fastboot_data_len == 0 && bytes_received == 0) {
> +               /* Nothing to download yet. Response is of the form:
> +                * [DATA|FAIL]$cmd_parameter
> +                *
> +                * where cmd_parameter is an 8 digit hexadecimal number
> +                */
> +               if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
> +                       write_fb_response("FAIL", cmd_parameter, response);
> +               else
> +                       write_fb_response("DATA", cmd_parameter, response);
> +       } else if (fastboot_data_len == 0 &&
> +                  (bytes_received >= bytes_expected)) {
> +               /* Download complete. Respond with "OKAY" */
> +               write_fb_response("OKAY", "", response);
> +               image_size = bytes_received;
> +               bytes_expected = 0;
> +               bytes_received = 0;
> +       } else {
> +               if (fastboot_data_len == 0 ||
> +                   (bytes_received + fastboot_data_len) > bytes_expected) {
> +                       write_fb_response("FAIL",
> +                                         "Received invalid data length",
> +                                         response);
> +                       return;
> +               }
> +               /* Download data to CONFIG_FASTBOOT_BUF_ADDR */
> +               memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR + bytes_received,
> +                      fastboot_data, fastboot_data_len);

This is different than all the other approaches, which take the load
address as a command parameter. Is there a good reason to have it
baked in like this?

> +               bytes_received += fastboot_data_len;
> +       }
> +}
> +
> +/**
> + * Writes the previously downloaded image to the partition indicated by
> + * cmd_parameter. Writes to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_flash(char *response)
> +{
> +       fb_mmc_flash_write(cmd_parameter, (void *)CONFIG_FASTBOOT_BUF_ADDR,
> +                          image_size, response);

It's peculiar that this hard-codes mmc.

> +}
> +
> +/**
> + * Erases the partition indicated by cmd_parameter (clear to 0x00s). Writes
> + * to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_erase(char *response)
> +{
> +       fb_mmc_erase(cmd_parameter, response);
> +}
> +
> +/**
> + * Continues normal boot process by running "bootcmd". Writes
> + * to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_continue(char *response)
> +{
> +       char *bootcmd;
> +
> +       bootcmd = env_get("bootcmd");
> +       if (bootcmd)
> +               write_fb_response("OKAY", "", response);
> +       else
> +               write_fb_response("FAIL", "bootcmd not set", response);
> +}
> +
> +/**
> + * Sets reboot bootloader flag if requested. Writes to response.
> + *
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void fb_reboot(char *response)
> +{
> +       write_fb_response("OKAY", "", response);
> +       if (!strcmp("reboot-bootloader", cmd_string))
> +               strcpy((char *)CONFIG_FASTBOOT_BUF_ADDR, "reboot-bootloader");
> +}
> +
> +/**
> + * Boots into downloaded image.
> + */
> +static void boot_downloaded_image(void)
> +{
> +       char kernel_addr[12];
> +       char *fdt_addr = env_get("fdt_addr_r");
> +       char *const bootm_args[] = {
> +               "bootm", kernel_addr, "-", fdt_addr, NULL
> +       };

It seems like this should be able to affected from the host side...
for instance choosing a config in an ITB.

How is the fdt supposed to be populated in the scenario at all?

> +
> +       sprintf(kernel_addr, "0x%lx", (long)CONFIG_FASTBOOT_BUF_ADDR);
> +
> +       printf("\nBooting kernel at %s with fdt at %s...\n\n\n",
> +              kernel_addr, fdt_addr);
> +       do_bootm(NULL, 0, 4, bootm_args);
> +
> +       /* This only happens if image is faulty so we start over. */
> +       do_reset(NULL, 0, 0, NULL);
> +}
> +
> +/**
> + * Writes a response to response buffer of the form "$tag$reason".
> + *
> + * @param tag         The first part of the response
> + * @param reason      The second part of the response
> + * @param repsonse    Pointer to fastboot response buffer
> + */
> +static void write_fb_response(const char *tag, const char *reason,
> +                             char *response)
> +{
> +       strncpy(response, tag, strlen(tag));
> +       strncat(response, reason, FASTBOOT_RESPONSE_LEN - strlen(tag) - 1);
> +}
> +
> +/**
> + * Frees any resources allocated during current fastboot command.
> + */
> +static void cleanup_command_data(void)
> +{
> +       /* cmd_parameter and cmd_string potentially point to memory allocated by

Use correct multi-line comment format. "/*" on its own line.

> +        * strdup
> +        */
> +       if (cmd_parameter)
> +               free(cmd_parameter);
> +       if (cmd_string)
> +               free(cmd_string);
> +       cmd_parameter = NULL;
> +       cmd_string = NULL;
> +}
> +
> +/**
> + * Incoming UDP packet handler.
> + *
> + * @param packet  Pointer to incoming UDP packet
> + * @param dport   Destination UDP port
> + * @param sip     Source IP address
> + * @param sport   Source UDP port
> + * @param len     Packet length
> + */
> +static void fastboot_handler(uchar *packet, unsigned int dport,
> +                            struct in_addr sip, unsigned int sport,
> +                            unsigned int len)
> +{
> +       struct fastboot_header fb_header;
> +       char fastboot_data[DATA_SIZE] = {0};
> +       unsigned int fastboot_data_len = 0;
> +
> +       if (dport != fastboot_our_port)
> +               return;
> +
> +       fastboot_remote_ip = sip;
> +       fastboot_remote_port = sport;
> +
> +       if (len < FASTBOOT_HEADER_SIZE || len > PACKET_SIZE)
> +               return;
> +       memcpy(&fb_header, packet, sizeof(fb_header));
> +       fb_header.flags = 0;
> +       fb_header.seq = ntohs(fb_header.seq);
> +       packet += sizeof(fb_header);
> +       len -= sizeof(fb_header);
> +
> +       switch (fb_header.id) {
> +       case FASTBOOT_QUERY:
> +               fastboot_send(fb_header, fastboot_data, 0, 0);
> +               break;
> +       case FASTBOOT_INIT:
> +       case FASTBOOT_FASTBOOT:
> +               fastboot_data_len = len;
> +               if (len > 0)
> +                       memcpy(fastboot_data, packet, len);
> +               if (fb_header.seq == fb_sequence_number) {
> +                       fastboot_send(fb_header, fastboot_data,
> +                                     fastboot_data_len, 0);
> +                       fb_sequence_number++;
> +               } else if (fb_header.seq == fb_sequence_number - 1) {
> +                       /* Retransmit last sent packet */
> +                       fastboot_send(fb_header, fastboot_data,
> +                                     fastboot_data_len, 1);
> +               }
> +               break;
> +       default:
> +               pr_err("ID %d not implemented.\n", fb_header.id);
> +               fb_header.id = FASTBOOT_ERROR;
> +               fastboot_send(fb_header, fastboot_data, 0, 0);
> +               break;
> +       }
> +}
> +
> +void fastboot_start_server(void)
> +{
> +       printf("Using %s device\n", eth_get_name());
> +       printf("Listening for fastboot command on %pI4\n", &net_ip);
> +
> +       fastboot_our_port = WELL_KNOWN_PORT;
> +
> +       net_set_udp_handler(fastboot_handler);
> +
> +       /* zero out server ether in case the server ip has changed */
> +       memset(net_server_ethaddr, 0, 6);
> +}
> diff --git a/net/net.c b/net/net.c
> index d222c1f..554df7a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -87,6 +87,9 @@
>  #include <environment.h>
>  #include <errno.h>
>  #include <net.h>
> +#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)

I'd prefer we not proliferate the guards around includes.

> +#include <net/fastboot.h>
> +#endif
>  #include <net/tftp.h>
>  #if defined(CONFIG_LED_STATUS)
>  #include <miiphy.h>
> @@ -451,6 +454,11 @@ restart:
>                         tftp_start_server();
>                         break;
>  #endif
> +#ifdef CONFIG_UDP_FUNCTION_FASTBOOT
> +               case FASTBOOT:
> +                       fastboot_start_server();
> +                       break;
> +#endif
>  #if defined(CONFIG_CMD_DHCP)
>                 case DHCP:
>                         bootp_reset();
> @@ -1322,6 +1330,7 @@ common:
>                 /* Fall through */
>
>         case NETCONS:
> +       case FASTBOOT:
>         case TFTPSRV:
>                 if (net_ip.s_addr == 0) {
>                         puts("*** ERROR: `ipaddr' not set\n");
> --
> 2.7.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

On Thu, May 3, 2018 at 9:39 PM Joe Hershberger <joe.hershberger@ni.com>
wrote:

> On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com>
wrote:

<snip>

> > +/**
> > + * Constructs and sends a packet in response to received fastboot
packet
> > + *
> > + * @param fb_header            Header for response packet
> > + * @param fastboot_data        Pointer to received fastboot data
> > + * @param fastboot_data_len    Length of received fastboot data
> > + * @param retransmit           Nonzero if sending last sent packet
> > + */
> > +static void fastboot_send(struct fastboot_header fb_header, char
*fastboot_data,
> > +                         unsigned int fastboot_data_len, uchar
retransmit)
> > +{
> > +       uchar *packet;
> > +       uchar *packet_base;
> > +       int len = 0;
> > +       const char *error_msg = "An error occurred.";
> > +       short tmp;
> > +       struct fastboot_header fb_response_header = fb_header;
> > +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> > +       /*
> > +        *      We will always be sending some sort of packet, so
> > +        *      cobble together the packet headers now.
> > +        */
> > +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
> > +       packet_base = packet;
> > +
> > +       /* Resend last packet */
> > +       if (retransmit) {
> > +               memcpy(packet, last_packet, last_packet_len);
> > +               net_send_udp_packet(net_server_ethaddr,
fastboot_remote_ip,
> > +                                   fastboot_remote_port,
fastboot_our_port,
> > +                                   last_packet_len);
> > +               return;
> > +       }
> > +
> > +       fb_response_header.seq = htons(fb_response_header.seq);
> > +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
> > +       packet += sizeof(fb_response_header);
> > +
> > +       switch (fb_header.id) {
> > +       case FASTBOOT_QUERY:
> > +               tmp = htons(fb_sequence_number);
> > +               memcpy(packet, &tmp, sizeof(tmp));
> > +               packet += sizeof(tmp);
> > +               break;
> > +       case FASTBOOT_INIT:
> > +               tmp = htons(fb_udp_version);
> > +               memcpy(packet, &tmp, sizeof(tmp));
> > +               packet += sizeof(tmp);
> > +               tmp = htons(fb_packet_size);
> > +               memcpy(packet, &tmp, sizeof(tmp));
> > +               packet += sizeof(tmp);
> > +               break;
> > +       case FASTBOOT_ERROR:
> > +               memcpy(packet, error_msg, strlen(error_msg));
> > +               packet += strlen(error_msg);
> > +               break;
> > +       case FASTBOOT_FASTBOOT:
> > +               if (!cmd_string) {
> > +                       /* Parse command and send ack */
> > +                       cmd_parameter = fastboot_data;

> This seems unnecessary. There are only 4 cases handled, and of them
> only download seems to be a command that happens more than once. And
> in download, the first past through this parameter is saved internally
> as bytes_expected.

> > +                       cmd_string = strsep(&cmd_parameter, ":");
> > +                       cmd_string = strdup(cmd_string);
> > +                       if (cmd_parameter)
> > +                               cmd_parameter = strdup(cmd_parameter);
> > +               } else if (!strcmp("getvar", cmd_string)) {
> > +                       fb_getvar(response);

> Seems like you should simply pass the "fastboot_data" as a parameter
> here rather than using a global.

I'm completely pulling this apart in a later patch. I wonder if I should
really be folding some of these back into this - I was trying to maintain a
clear line to the AOSP code which this came from.

> > +/**
> > + * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
> > + * Writes to response.
> > + *
> > + * @param fastboot_data        Pointer to received fastboot data
> > + * @param fastboot_data_len    Length of received fastboot data
> > + * @param repsonse             Pointer to fastboot response buffer
> > + */
> > +static void fb_download(char *fastboot_data, unsigned int
fastboot_data_len,
> > +                       char *response)
> > +{
> > +       char *tmp;
> > +
> > +       if (bytes_expected == 0) {
> > +               if (!cmd_parameter) {
> > +                       write_fb_response("FAIL", "Expected command
parameter",
> > +                                         response);
> > +                       return;
> > +               }
> > +               bytes_expected = simple_strtoul(cmd_parameter, &tmp,
16);
> > +               if (bytes_expected == 0) {
> > +                       write_fb_response("FAIL", "Expected nonzero
image size",
> > +                                         response);
> > +                       return;
> > +               }
> > +       }
> > +       if (fastboot_data_len == 0 && bytes_received == 0) {
> > +               /* Nothing to download yet. Response is of the form:
> > +                * [DATA|FAIL]$cmd_parameter
> > +                *
> > +                * where cmd_parameter is an 8 digit hexadecimal number
> > +                */
> > +               if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
> > +                       write_fb_response("FAIL", cmd_parameter,
response);
> > +               else
> > +                       write_fb_response("DATA", cmd_parameter,
response);
> > +       } else if (fastboot_data_len == 0 &&
> > +                  (bytes_received >= bytes_expected)) {
> > +               /* Download complete. Respond with "OKAY" */
> > +               write_fb_response("OKAY", "", response);
> > +               image_size = bytes_received;
> > +               bytes_expected = 0;
> > +               bytes_received = 0;
> > +       } else {
> > +               if (fastboot_data_len == 0 ||
> > +                   (bytes_received + fastboot_data_len) >
bytes_expected) {
> > +                       write_fb_response("FAIL",
> > +                                         "Received invalid data
length",
> > +                                         response);
> > +                       return;
> > +               }
> > +               /* Download data to CONFIG_FASTBOOT_BUF_ADDR */
> > +               memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR +
bytes_received,
> > +                      fastboot_data, fastboot_data_len);

> This is different than all the other approaches, which take the load
> address as a command parameter. Is there a good reason to have it
> baked in like this?

Feels odd to me too, but this is just following the USB code. I'm thinking
we should change the command so it's:

fastboot ... [<load-address> [<size>]]

And then default to the current values is they're not overridden?

> > +               bytes_received += fastboot_data_len;
> > +       }
> > +}
> > +
> > +/**
> > + * Writes the previously downloaded image to the partition indicated by
> > + * cmd_parameter. Writes to response.
> > + *
> > + * @param repsonse    Pointer to fastboot response buffer
> > + */
> > +static void fb_flash(char *response)
> > +{
> > +       fb_mmc_flash_write(cmd_parameter, (void
*)CONFIG_FASTBOOT_BUF_ADDR,
> > +                          image_size, response);

> It's peculiar that this hard-codes mmc.

Fixed up in a later patch.

> > +/**
> > + * Boots into downloaded image.
> > + */
> > +static void boot_downloaded_image(void)
> > +{
> > +       char kernel_addr[12];
> > +       char *fdt_addr = env_get("fdt_addr_r");
> > +       char *const bootm_args[] = {
> > +               "bootm", kernel_addr, "-", fdt_addr, NULL
> > +       };

> It seems like this should be able to affected from the host side...
> for instance choosing a config in an ITB.

> How is the fdt supposed to be populated in the scenario at all?

I strip this bit out in a later patch and replace it with either a simple
bootm (which follows the USB code), or run fastbootcmd.

Again, this is part of trying to land this patch as close as possible to
the code which comes from AOSP.

--
Alex Kiernan

On Tue, May 8, 2018 at 4:11 AM, Alex Kiernan <alex.kiernan@gmail.com> wrote:
> On Thu, May 3, 2018 at 9:39 PM Joe Hershberger <joe.hershberger@ni.com>
> wrote:
>
>> On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com>
> wrote:
>
> <snip>
>
>> > +/**
>> > + * Constructs and sends a packet in response to received fastboot
> packet
>> > + *
>> > + * @param fb_header            Header for response packet
>> > + * @param fastboot_data        Pointer to received fastboot data
>> > + * @param fastboot_data_len    Length of received fastboot data
>> > + * @param retransmit           Nonzero if sending last sent packet
>> > + */
>> > +static void fastboot_send(struct fastboot_header fb_header, char
> *fastboot_data,
>> > +                         unsigned int fastboot_data_len, uchar
> retransmit)
>> > +{
>> > +       uchar *packet;
>> > +       uchar *packet_base;
>> > +       int len = 0;
>> > +       const char *error_msg = "An error occurred.";
>> > +       short tmp;
>> > +       struct fastboot_header fb_response_header = fb_header;
>> > +       char response[FASTBOOT_RESPONSE_LEN] = {0};
>> > +       /*
>> > +        *      We will always be sending some sort of packet, so
>> > +        *      cobble together the packet headers now.
>> > +        */
>> > +       packet = net_tx_packet + net_eth_hdr_size() + IP_UDP_HDR_SIZE;
>> > +       packet_base = packet;
>> > +
>> > +       /* Resend last packet */
>> > +       if (retransmit) {
>> > +               memcpy(packet, last_packet, last_packet_len);
>> > +               net_send_udp_packet(net_server_ethaddr,
> fastboot_remote_ip,
>> > +                                   fastboot_remote_port,
> fastboot_our_port,
>> > +                                   last_packet_len);
>> > +               return;
>> > +       }
>> > +
>> > +       fb_response_header.seq = htons(fb_response_header.seq);
>> > +       memcpy(packet, &fb_response_header, sizeof(fb_response_header));
>> > +       packet += sizeof(fb_response_header);
>> > +
>> > +       switch (fb_header.id) {
>> > +       case FASTBOOT_QUERY:
>> > +               tmp = htons(fb_sequence_number);
>> > +               memcpy(packet, &tmp, sizeof(tmp));
>> > +               packet += sizeof(tmp);
>> > +               break;
>> > +       case FASTBOOT_INIT:
>> > +               tmp = htons(fb_udp_version);
>> > +               memcpy(packet, &tmp, sizeof(tmp));
>> > +               packet += sizeof(tmp);
>> > +               tmp = htons(fb_packet_size);
>> > +               memcpy(packet, &tmp, sizeof(tmp));
>> > +               packet += sizeof(tmp);
>> > +               break;
>> > +       case FASTBOOT_ERROR:
>> > +               memcpy(packet, error_msg, strlen(error_msg));
>> > +               packet += strlen(error_msg);
>> > +               break;
>> > +       case FASTBOOT_FASTBOOT:
>> > +               if (!cmd_string) {
>> > +                       /* Parse command and send ack */
>> > +                       cmd_parameter = fastboot_data;
>
>> This seems unnecessary. There are only 4 cases handled, and of them
>> only download seems to be a command that happens more than once. And
>> in download, the first past through this parameter is saved internally
>> as bytes_expected.
>
>> > +                       cmd_string = strsep(&cmd_parameter, ":");
>> > +                       cmd_string = strdup(cmd_string);
>> > +                       if (cmd_parameter)
>> > +                               cmd_parameter = strdup(cmd_parameter);
>> > +               } else if (!strcmp("getvar", cmd_string)) {
>> > +                       fb_getvar(response);
>
>> Seems like you should simply pass the "fastboot_data" as a parameter
>> here rather than using a global.
>
> I'm completely pulling this apart in a later patch. I wonder if I should
> really be folding some of these back into this - I was trying to maintain a
> clear line to the AOSP code which this came from.

What value do we get from that? Is there a merge technique you are
trying to enable to make pulling in future AOSP changes easier? Are
you trying to isolate your changes from AOSP for attribution purposes?
So far I'm of the opinion that the fixes to the existing code should
simply be there in the originating patch. Feel free to convince me
otherwise.

>> > +/**
>> > + * Copies image data from fastboot_data to CONFIG_FASTBOOT_BUF_ADDR.
>> > + * Writes to response.
>> > + *
>> > + * @param fastboot_data        Pointer to received fastboot data
>> > + * @param fastboot_data_len    Length of received fastboot data
>> > + * @param repsonse             Pointer to fastboot response buffer
>> > + */
>> > +static void fb_download(char *fastboot_data, unsigned int
> fastboot_data_len,
>> > +                       char *response)
>> > +{
>> > +       char *tmp;
>> > +
>> > +       if (bytes_expected == 0) {
>> > +               if (!cmd_parameter) {
>> > +                       write_fb_response("FAIL", "Expected command
> parameter",
>> > +                                         response);
>> > +                       return;
>> > +               }
>> > +               bytes_expected = simple_strtoul(cmd_parameter, &tmp,
> 16);
>> > +               if (bytes_expected == 0) {
>> > +                       write_fb_response("FAIL", "Expected nonzero
> image size",
>> > +                                         response);
>> > +                       return;
>> > +               }
>> > +       }
>> > +       if (fastboot_data_len == 0 && bytes_received == 0) {
>> > +               /* Nothing to download yet. Response is of the form:
>> > +                * [DATA|FAIL]$cmd_parameter
>> > +                *
>> > +                * where cmd_parameter is an 8 digit hexadecimal number
>> > +                */
>> > +               if (bytes_expected > CONFIG_FASTBOOT_BUF_SIZE)
>> > +                       write_fb_response("FAIL", cmd_parameter,
> response);
>> > +               else
>> > +                       write_fb_response("DATA", cmd_parameter,
> response);
>> > +       } else if (fastboot_data_len == 0 &&
>> > +                  (bytes_received >= bytes_expected)) {
>> > +               /* Download complete. Respond with "OKAY" */
>> > +               write_fb_response("OKAY", "", response);
>> > +               image_size = bytes_received;
>> > +               bytes_expected = 0;
>> > +               bytes_received = 0;
>> > +       } else {
>> > +               if (fastboot_data_len == 0 ||
>> > +                   (bytes_received + fastboot_data_len) >
> bytes_expected) {
>> > +                       write_fb_response("FAIL",
>> > +                                         "Received invalid data
> length",
>> > +                                         response);
>> > +                       return;
>> > +               }
>> > +               /* Download data to CONFIG_FASTBOOT_BUF_ADDR */
>> > +               memcpy((void *)CONFIG_FASTBOOT_BUF_ADDR +
> bytes_received,
>> > +                      fastboot_data, fastboot_data_len);
>
>> This is different than all the other approaches, which take the load
>> address as a command parameter. Is there a good reason to have it
>> baked in like this?
>
> Feels odd to me too, but this is just following the USB code. I'm thinking
> we should change the command so it's:
>
> fastboot ... [<load-address> [<size>]]
>
> And then default to the current values is they're not overridden?
>
>> > +               bytes_received += fastboot_data_len;
>> > +       }
>> > +}
>> > +
>> > +/**
>> > + * Writes the previously downloaded image to the partition indicated by
>> > + * cmd_parameter. Writes to response.
>> > + *
>> > + * @param repsonse    Pointer to fastboot response buffer
>> > + */
>> > +static void fb_flash(char *response)
>> > +{
>> > +       fb_mmc_flash_write(cmd_parameter, (void
> *)CONFIG_FASTBOOT_BUF_ADDR,
>> > +                          image_size, response);
>
>> It's peculiar that this hard-codes mmc.
>
> Fixed up in a later patch.
>
>> > +/**
>> > + * Boots into downloaded image.
>> > + */
>> > +static void boot_downloaded_image(void)
>> > +{
>> > +       char kernel_addr[12];
>> > +       char *fdt_addr = env_get("fdt_addr_r");
>> > +       char *const bootm_args[] = {
>> > +               "bootm", kernel_addr, "-", fdt_addr, NULL
>> > +       };
>
>> It seems like this should be able to affected from the host side...
>> for instance choosing a config in an ITB.
>
>> How is the fdt supposed to be populated in the scenario at all?
>
> I strip this bit out in a later patch and replace it with either a simple
> bootm (which follows the USB code), or run fastbootcmd.
>
> Again, this is part of trying to land this patch as close as possible to
> the code which comes from AOSP.
>
> --
> Alex Kiernan
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

On Tue, May 8, 2018 at 4:24 PM Joe Hershberger <joe.hershberger@ni.com>
wrote:

> On Tue, May 8, 2018 at 4:11 AM, Alex Kiernan <alex.kiernan@gmail.com>
wrote:
> > On Thu, May 3, 2018 at 9:39 PM Joe Hershberger <joe.hershberger@ni.com>
> > wrote:
> >
> >> On Mon, Apr 30, 2018 at 3:32 AM, Alex Kiernan <alex.kiernan@gmail.com>
> > wrote:
> >
> > <snip>
> >
> >> > +/**
> >> > + * Constructs and sends a packet in response to received fastboot
> > packet
> >> > + *
> >> > + * @param fb_header            Header for response packet
> >> > + * @param fastboot_data        Pointer to received fastboot data
> >> > + * @param fastboot_data_len    Length of received fastboot data
> >> > + * @param retransmit           Nonzero if sending last sent packet
> >> > + */
> >> > +static void fastboot_send(struct fastboot_header fb_header, char
> > *fastboot_data,
> >> > +                         unsigned int fastboot_data_len, uchar
> > retransmit)
> >> > +{
> >> > +       uchar *packet;
> >> > +       uchar *packet_base;
> >> > +       int len = 0;
> >> > +       const char *error_msg = "An error occurred.";
> >> > +       short tmp;
> >> > +       struct fastboot_header fb_response_header = fb_header;
> >> > +       char response[FASTBOOT_RESPONSE_LEN] = {0};
> >> > +       /*
> >> > +        *      We will always be sending some sort of packet, so
> >> > +        *      cobble together the packet headers now.
> >> > +        */
> >> > +       packet = net_tx_packet + net_eth_hdr_size() +
IP_UDP_HDR_SIZE;
> >> > +       packet_base = packet;
> >> > +
> >> > +       /* Resend last packet */
> >> > +       if (retransmit) {
> >> > +               memcpy(packet, last_packet, last_packet_len);
> >> > +               net_send_udp_packet(net_server_ethaddr,
> > fastboot_remote_ip,
> >> > +                                   fastboot_remote_port,
> > fastboot_our_port,
> >> > +                                   last_packet_len);
> >> > +               return;
> >> > +       }
> >> > +
> >> > +       fb_response_header.seq = htons(fb_response_header.seq);
> >> > +       memcpy(packet, &fb_response_header,
sizeof(fb_response_header));
> >> > +       packet += sizeof(fb_response_header);
> >> > +
> >> > +       switch (fb_header.id) {
> >> > +       case FASTBOOT_QUERY:
> >> > +               tmp = htons(fb_sequence_number);
> >> > +               memcpy(packet, &tmp, sizeof(tmp));
> >> > +               packet += sizeof(tmp);
> >> > +               break;
> >> > +       case FASTBOOT_INIT:
> >> > +               tmp = htons(fb_udp_version);
> >> > +               memcpy(packet, &tmp, sizeof(tmp));
> >> > +               packet += sizeof(tmp);
> >> > +               tmp = htons(fb_packet_size);
> >> > +               memcpy(packet, &tmp, sizeof(tmp));
> >> > +               packet += sizeof(tmp);
> >> > +               break;
> >> > +       case FASTBOOT_ERROR:
> >> > +               memcpy(packet, error_msg, strlen(error_msg));
> >> > +               packet += strlen(error_msg);
> >> > +               break;
> >> > +       case FASTBOOT_FASTBOOT:
> >> > +               if (!cmd_string) {
> >> > +                       /* Parse command and send ack */
> >> > +                       cmd_parameter = fastboot_data;
> >
> >> This seems unnecessary. There are only 4 cases handled, and of them
> >> only download seems to be a command that happens more than once. And
> >> in download, the first past through this parameter is saved internally
> >> as bytes_expected.
> >
> >> > +                       cmd_string = strsep(&cmd_parameter, ":");
> >> > +                       cmd_string = strdup(cmd_string);
> >> > +                       if (cmd_parameter)
> >> > +                               cmd_parameter =
strdup(cmd_parameter);
> >> > +               } else if (!strcmp("getvar", cmd_string)) {
> >> > +                       fb_getvar(response);
> >
> >> Seems like you should simply pass the "fastboot_data" as a parameter
> >> here rather than using a global.
> >
> > I'm completely pulling this apart in a later patch. I wonder if I should
> > really be folding some of these back into this - I was trying to
maintain a
> > clear line to the AOSP code which this came from.

> What value do we get from that? Is there a merge technique you are
> trying to enable to make pulling in future AOSP changes easier? Are
> you trying to isolate your changes from AOSP for attribution purposes?

I was trying to maintain the attribution line, but given the level of
change I'm struggling to achieve that in what's an increasingly complex
chain of patches which end up interacting too much.

Given Alex and Jocelyn are engaged, I'm not sure I see the value in
continuing trying to do this.

> So far I'm of the opinion that the fixes to the existing code should
> simply be there in the originating patch. Feel free to convince me
> otherwise.

I'll fail to convince you as I'm increasingly unconvinced myself... I'm
spending more time rebasing patches than actually separating the code into
something which can handle both UDP and USB transports.

--
Alex Kiernan