On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr bohr@verily.com wrote:
Thanks so much for porting this, Alex!
On Tue, Apr 24, 2018 at 4:58 AM Alex Kiernan alex.kiernan@gmail.com wrote:
On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo deymo+@google.com wrote:
+Jocelyn.
Thanks Alex for taking the time to port this!!
It turned out to be a great opportunity to play with coccinelle on something trivial, which I'd been meaning to do for ages... the refactor to add response into the fastboot_okay/fail call chain was a breeze with it.
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.
With the fastboot NAND support in mainline it looks like we end up in mtdparts to deliver that mapping through environment variables. No actual idea what that looks like when you're driving it.
- 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 version I picked from (o-mr1-iot-preview-8) has them as independent symbols, but making them a choice would resolve the issue for now.
You can select both network and USB fastboot to be enabled with the Kconfig, but at runtime you have to select to wait on either USB or network by running "fastboot udp" or "fastboot usb <USB_controller>". When the Android bootloader gets the command to reboot back to fastboot, it will read the "fastbootcmd" env variable and run that as a command (common/android_bootloader.c:151).
Thanks for that (especially the detail on android_bootloader which I'd not read through). The bit that concerns me is in timed_send_info:
+/** + * 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);
The code in timed_send_info is guarded with an ifdef, rather than based on the transport that's been selected at runtime. Do we need to make timed_send_info work based on the runtime state, rather than the compile time, or can we drop the ifdef guard and remove the TODO? I guess I'm assuming the former, but with no way to test USB I don't want head off down the wrong road!