Correct, with the current implementation, I think that you have the only solution... (1) add code during the startup sequence to test that rebooting into fastboot is desired (as done in the misc_init_r functions that you described) (2) once this has been detected, then write an env variable (as done with "reboot-mode" variable) (3) ensure that the CONFIG_BOOTCOMMAND has a test for this env variable and that it launches the "fastboot" command (as done with: if test reboot-${reboot-mode} = reboot-b; then echo fastboot; fastboot 0; fi)
So, I wanted to: (1) simplify this to not depend on any env variable, and not depend on the CONFIG_BOOTCOMMAND (can this be accidentally wiped out in the environment?) (2) also allow for the "fastboot continue" command (although I think that the CONFIG_BOOTCOMMAND also handles this properly!) IMO - this series seems to be a much more straightforward approach.... perhaps if I changed the function name to: fb_handle_reboot_bootloader_flag() or handle_fastboot_reboot_bootloader_flag() because it is not trying to handle all possible reboot modes, only the "fastboot reboot-bootloader".... Would that help? Thanks, Steve On Wed, Aug 24, 2016 at 3:07 AM, Paul Kocialkowski <cont...@paulk.fr> wrote: > Hi, > > Le mardi 23 août 2016 à 16:38 -0700, Steve Rae a écrit : >> The "fastboot reboot-bootloader" command is defined to >> re-enter into fastboot mode after rebooting into the >> bootloader. >> >> There is current support for setting the reset flag >> via the __weak fb_set_reboot_flag() function. >> >> This commit adds a generic handler to implement code >> which could launch fastboot during the boot sequence >> via this __weak fb_handle_reboot_flag() function. >> The actual handling this reset flag should be implemented >> by board/SoC specific code. > > So far, we've been calling the fastboot command from CONFIG_BOOTCOMMAND (more > or > less directly) by setting an env variable (reboot-mode, dofastboot, etc), > which > I think is a good fit. Since fastboot is a standalone command, I think it > makes > sense to call it from the bootcommand instead of calling it from the function > you introduce. > > IMO the fb_handle_reboot_flag function you're introducing should only detect > that fastboot mode is requested and set an env variable (like it's done > in misc_init_r in sniper and kc1) so that the bootcommand can pick it up and > act > accordingly. This clearly separates the logic and puts each side of it where > it > belongs. > >> Signed-off-by: Steve Rae <steve....@raedomain.com> >> cc: Alexey Firago <alexey_fir...@mentor.com> >> cc: Paul Kocialkowski <cont...@paulk.fr> >> cc: Tom Rini <tr...@konsulko.com> >> cc: Angela Stegmaier <angelaba...@ti.com> >> cc: Dileep Katta <dileep.ka...@linaro.org> >> --- >> >> common/main.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/common/main.c b/common/main.c >> index 2116a9e..ea3fe42 100644 >> --- a/common/main.c >> +++ b/common/main.c >> @@ -20,6 +20,12 @@ DECLARE_GLOBAL_DATA_PTR; >> */ >> __weak void show_boot_progress(int val) {} >> >> +/* >> + * Board-specific Platform code must implement fb_handle_reboot_flag(), if >> + * this feature is desired >> + */ >> +__weak void fb_handle_reboot_flag(void) {} >> + >> static void run_preboot_environment_command(void) >> { >> #ifdef CONFIG_PREBOOT >> @@ -63,6 +69,8 @@ void main_loop(void) >> if (cli_process_fdt(&s)) >> cli_secure_boot_cmd(s); >> >> + fb_handle_reboot_flag(); >> + >> autoboot_command(s); >> >> cli_loop(); > -- > Paul Kocialkowski, developer of low-level free software for embedded devices > > Website: https://www.paulk.fr/ > Coding blog: https://code.paulk.fr/ > Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/ _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot