[U-Boot] [RFC PATCH v2 00/20] Add fastboot UDP support

Jocelyn Bohr bohr at google.com
Wed May 2 06:33:39 UTC 2018


Hi Alex,

I think this approach looks really good so far, and will make maintaining
both
implementations easier going forward. I haven't looked at every change here
yet, but when going through the patches should I add a "Reviewed-by:" line
if
the change looks good to me?

Thanks,
Jocelyn

On Mon, Apr 30, 2018 at 1:33 AM Alex Kiernan <alex.kiernan at gmail.com> wrote:

>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>
> The underlying implementations of most commands are now merged between
> both code paths ('oem format' from the USB side is the only one remaining).
>
> Other changes to command handling so that UDP follows the existing USB
> behaviour:
>
> - 'boot' now follows the USB code and does 'fastboot
> CONFIG_FASTBOOT_BUF_ADDR'.
>   I've added 'fastbootcmd' which if set overrides the boot command and
>   allows the existing UDP behaviour to be preserved.
>

The UDP behavior more correct based on the specification.
"The previously downloaded data is a boot.img and should be booted
according to
the normal procedure for a boot.img."

https://android.googlesource.com/platform/system/core/+/master/fastboot/

I'm not sure who we will break if we change the USB code...also we probably
want to make sure image verification happens with "fastboot boot", making
sure
"fastboot boot" doesn't allow booting an unverified kernel on locked
devices. I
think we can add that flow later though, upstream U-boot doesn't support
lock/unlock commands anyway.


> - 'continue' in UDP now exits the fastboot server rather than executing
>   'run bootcmd'
> - 'reboot-bootloader' no longer writes 'reboot-bootloader' to
>   CONFIG_FASTBOOT_BUF_ADDR as its marker for the subsequent boot. The code
>   which is in AOSP common/android_bootloader.c expects this marker, but
>   we have prior art in the USB code using the weak function
>   fb_set_reboot_flag
> - 'getvar' in the UDP path now supports fetching 'fastboot.' prefixed
>   variables (inherited from the USB path)
> - 'getvar' in the USB path inherits all the variables from the UDP path
>
> Remaining issues:
>
> - whilst I've merged NAND support into the UDP code path, I've no way
>   of testing it. My expectation is it'll work, but will need
> timed_send_info
>   working into the NAND path to avoid timeouts on the network side.


I don't know of any devices that use NAND and the fastboot UDP code, so
I'm inclined to say this is okay.

- I still need to fix timed_send_info handling when going through the USB
>   path.
> - the protocol part of the fastboot UDP implementation is separated out
>   and would I expect form the basis of a consolidated implementation, but
>   I'm inclined to address that as a clearly separate patch so it can be
>   tested in isolation (I have no USB hardware I can try this on).
>

Should this set of patches be tested on USB hardware too? There are some
changes here to the USB implementation.


>
> Changes in v2:
> - ensure fastboot syntax is backward compatible - 'fastboot 0' means
>   'fastboot usb 0'
>
> Alex Kiernan (20):
>   fastboot: Move fastboot to drivers/fastboot
>   fastboot: Switch dependencies on FASTBOOT to USB_FUNCTION_FASTBOOT
>   fastboot: Refactor fastboot_okay/fail to take response
>   fastboot: Extract fastboot_okay/fail to fb_common.c
>   fastboot: Introduce fastboot_response and refactor fastboot_okay/fail
>   fastboot: Correct dependencies in FASTBOOT_FLASH
>   net: fastboot: Merge AOSP UDP fastboot
>   net: fastboot: Support building without MMC
>   fastboot: Refactor write_fb_response into fastboot_okay/fail/response
>   fastboot: Merge USB and UDP getvar implementation
>   fastboot: net: Change 'continue' so it matches USB fastboot
>   fastboot: net: Convert command lookup to a table
>   fastboot: Merge reboot-bootloader handling
>   fastboot: Avoid re-parsing cmd_string for boot/reboot
>   fastboot: Merge boot common across USB and UDP
>   fastboot: net: Add NAND support
>   fastboot: Guard getvar:partition-type/size with MMC
>   fastboot: Check if part_name is NULL before using it
>   fastboot: Add missing newlines
>   fastboot: net: Split fastboot protocol out from net
>
>  arch/arm/Kconfig                                 |   2 -
>  arch/arm/mach-omap2/boot-common.c                |   2 +-
>  arch/arm/mach-omap2/utils.c                      |   4 +-
>  arch/arm/mach-rockchip/rk3128-board.c            |   2 +-
>  arch/arm/mach-rockchip/rk322x-board.c            |   2 +-
>  board/ti/common/Kconfig                          |   1 -
>  cmd/Kconfig                                      |  14 +-
>  cmd/fastboot.c                                   |  35 ++-
>  cmd/net.c                                        |   6 +
>  common/Makefile                                  |  13 --
>  configs/am335x_boneblack_defconfig               |   2 +-
>  configs/am335x_boneblack_vboot_defconfig         |   2 +-
>  configs/am335x_evm_defconfig                     |   2 +-
>  configs/am335x_evm_nor_defconfig                 |   2 +-
>  configs/am335x_evm_norboot_defconfig             |   2 +-
>  configs/am335x_evm_spiboot_defconfig             |   2 +-
>  configs/am335x_evm_usbspl_defconfig              |   2 +-
>  configs/am57xx_evm_defconfig                     |   2 +-
>  configs/am57xx_hs_evm_defconfig                  |   2 +-
>  configs/bcm23550_w1d_defconfig                   |   2 +-
>  configs/bcm28155_ap_defconfig                    |   2 +-
>  configs/birdland_bav335a_defconfig               |   2 +-
>  configs/birdland_bav335b_defconfig               |   2 +-
>  configs/cgtqmx6eval_defconfig                    |   2 +-
>  configs/dra7xx_evm_defconfig                     |   2 +-
>  configs/dra7xx_hs_evm_defconfig                  |   2 +-
>  configs/kc1_defconfig                            |   2 +-
>  configs/mx6qsabrelite_defconfig                  |   2 +-
>  configs/mx6sabresd_defconfig                     |   2 +-
>  configs/nitrogen6dl2g_defconfig                  |   2 +-
>  configs/nitrogen6dl_defconfig                    |   2 +-
>  configs/nitrogen6q2g_defconfig                   |   2 +-
>  configs/nitrogen6q_defconfig                     |   2 +-
>  configs/nitrogen6s1g_defconfig                   |   2 +-
>  configs/nitrogen6s_defconfig                     |   2 +-
>  configs/omap3_beagle_defconfig                   |   2 +-
>  configs/omap3_evm_defconfig                      |   2 +-
>  configs/omap3_logic_defconfig                    |   2 +-
>  configs/sniper_defconfig                         |   2 +-
>  configs/stih410-b2260_defconfig                  |   2 +-
>  configs/xilinx_zynqmp_zc1751_xm015_dc1_defconfig |   2 +-
>  configs/xilinx_zynqmp_zc1751_xm016_dc2_defconfig |   2 +-
>  configs/xilinx_zynqmp_zcu102_rev1_0_defconfig    |   2 +-
>  configs/xilinx_zynqmp_zcu102_revA_defconfig      |   2 +-
>  configs/xilinx_zynqmp_zcu102_revB_defconfig      |   2 +-
>  configs/xilinx_zynqmp_zcu106_revA_defconfig      |   2 +-
>  drivers/Kconfig                                  |   2 +
>  drivers/Makefile                                 |   1 +
>  {cmd => drivers}/fastboot/Kconfig                |  46 ++--
>  drivers/fastboot/Makefile                        |   9 +
>  drivers/fastboot/fb_common.c                     | 136 ++++++++++++
>  drivers/fastboot/fb_getvar.c                     |  96 +++++++++
>  {common => drivers/fastboot}/fb_mmc.c            | 142 +++++++-----
>  {common => drivers/fastboot}/fb_nand.c           |  31 +--
>  drivers/fastboot/fb_packet.c                     | 249
> +++++++++++++++++++++
>  {common => drivers/fastboot}/image-sparse.c      |  41 ++--
>  drivers/usb/gadget/f_fastboot.c                  | 133 ++----------
>  include/fastboot.h                               |  65 +++++-
>  include/fb_mmc.h                                 |   4 +-
>  include/fb_nand.h                                |   4 +-
>  include/image-sparse.h                           |   2 +-
>  include/net.h                                    |   6 +-
>  include/net/fastboot.h                           |  27 +++
>  net/Makefile                                     |   1 +
>  net/fastboot.c                                   | 262
> +++++++++++++++++++++++
>  net/net.c                                        |   9 +
>  66 files changed, 1123 insertions(+), 296 deletions(-)
>  rename {cmd => drivers}/fastboot/Kconfig (86%)
>  create mode 100644 drivers/fastboot/Makefile
>  create mode 100644 drivers/fastboot/fb_common.c
>  create mode 100644 drivers/fastboot/fb_getvar.c
>  rename {common => drivers/fastboot}/fb_mmc.c (71%)
>  rename {common => drivers/fastboot}/fb_nand.c (86%)
>  create mode 100644 drivers/fastboot/fb_packet.c
>  rename {common => drivers/fastboot}/image-sparse.c (89%)
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> --
> 2.7.4
>
>


More information about the U-Boot mailing list