[U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

Alex Deymo
2018-04-24T10:24:56Z

+Jocelyn.

Thanks Alex for taking the time to port this!! Some comments inline here
about your questions.

2018-04-24 11:37 GMT+02:00 Alex Kiernan <alex.kiernan@gmail.com>:

>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>
> Open questions:
>
> - what's the correct way of attributing the original authors? I've added
> Co-authored-by, is that right? checkpatch doesn't seem to know any
> of the co- tags
> - currently there's no NAND support and I've no way of testing that, so
> my inclination is towards leaving it like that until someone with that
> particular itch to scratch can look at it
>

Fastboot uses partition names, like "system" and "boot" which have a
meaning in the Android partition scheme. For GPT, we just use the partition
names as the android names; but if you are booting from some other storage
like NAND where you don't have that then you need some mapping glue
("system" --> device and partition number). I've seen U-Boot modifications
for several devices where they just stick a table hard-coded in the U-Boot
code; which works great for that device but it isn't really a generic
approach. Other than handling the names issue, I don't see any problem with
supporting NAND here, but a generic way to set global names / alias would
be needed for this.

> - you can select both USB and UDP fastboot, but the comments in the
> AOSP code suggest that needs fixing - again, I've no board I can test
> USB fastboot on
>

I thought we checked in the Kconfig that you couldn't enable both. I don't
remember the details now but yeah you can't wait for network or USB traffic
on the current code.

- the USB and UDP code want consolidating, with this series there would
> then be two separate implementations of the same protocol
> - the syntax for the USB fastboot command changes from `fastboot
> <controller>`
> to `fastboot usb <controller>`, that feels unacceptable and we probably
> want something like `fastboot <controller>` or `fastboot udp`?
> - unrelated to this series, but a show-stopper for me, there's no FIT image
> support, but that doesn't feel unsolveable - something like add an option
> to pass in the load address and/or use loadaddr, then something else to
> let you override the bootm invocation on the server side
>
> I've not tested all the code paths yet, but the obvious stuff works
> (basic interaction, download, boot) - every interaction elicits a
> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
> my local end is sending a getvar for that, but I've not investigated.
>

Yes, there's a bit of different handling of partitions for A/B android
devices. Instead of "system" you have two partitions: "system_a" and
"system_b" (potentially more although 2 is the most common case) so to know
whether you have an old device or a newer device the fastboot client checks
the "slot-count" variable. Undefined means of course that you have an
old-style device, but if you return something like "2" you would be able to
flash "system" on the "slot A" which is translated (again in the fastboot
client) to flash "system_a" partition. This is useful when using the higher
level operations like flashall/update and you want to specify to flash only
"slot A", "slot B" or even both of them. There are other fastboot variables
that require some plumbing, such as "has-slot:<partition>" to tell whether
"system" is a partition that indeed has the two versions _a and _b.
Typically some partitions are twice (like "system" and "boot") and some
other are not (like "misc" or "userdata"). Anyway, this as is should work
for either old-style partition schemes or by force flashing "system_a" with
the system.img.

> Without any way of testing any of the USB/NAND code I'm nervous of wading
> into that kind of refactoring, would that be a pre-requisite for merging?
>
>
> Alex Kiernan (5):
> dfu: Refactor fastboot_okay/fail to take response
> dfu: Extract fastboot_okay/fail to fb_common.c
> net: dfu: Merge AOSP UDP fastboot
> dfu: Resolve Kconfig dependency loops
> net: dfu: Support building without MMC
>
> cmd/fastboot.c | 32 ++-
> cmd/fastboot/Kconfig | 21 +-
> cmd/net.c | 6 +
> common/Makefile | 4 +
> common/fb_common.c | 44 ++++
> common/fb_mmc.c | 114 ++++++---
> common/fb_nand.c | 31 +--
> common/image-sparse.c | 41 ++-
> drivers/usb/gadget/f_fastboot.c | 36 +--
> include/fastboot.h | 17 +-
> 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 | 548 ++++++++++++++++++++++++++++++
> ++++++++++
> net/net.c | 9 +
> 18 files changed, 824 insertions(+), 123 deletions(-)
> create mode 100644 common/fb_common.c
> create mode 100644 include/net/fastboot.h
> create mode 100644 net/fastboot.c
>
> --
> 2.7.4
>
>