Commit aa5ef3c0a75 ("bootretry: check for bootretry variable changes")
broke the feature where one can define different keys for "delaying"
versus "stopping" boot. The way the latter is implemented is by the code
in autoboot.c calling bootretry_dont_retry() when the stop sequence
has been detected, and that simply sets the retry_time variable in
bootretry.c to -1.However, with the mentioned commit, that is unconditionally overridden on every command, since it gets re-initialized from either the bootretry environment variable or CONFIG_BOOT_RETRY_TIME, thus making "delay" and "stop" effectively the same. To fix that, while still picking up changes to the bootretry environment variable, use the proper mechanism for C code to be notified about changes to environment variables. Since the callback is invoked before the change has actually been done to the environment (callbacks can reject the change from taking effect), we cannot simply call the existing bootretry_init_cmd_timeout() from the callback, as its env_get() would not see the new value. Instead, refactor most of it to an internal bootretry_parse(), and call that with the new value (which is NULL in the case bootretry is being deleted, so that works exactly as it should). Signed-off-by: Rasmus Villemoes <[email protected]> --- boot/bootretry.c | 28 +++++++++++++++++++++++----- include/env_callback.h | 7 +++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/boot/bootretry.c b/boot/bootretry.c index a60767eaa2e..52a253edd3e 100644 --- a/boot/bootretry.c +++ b/boot/bootretry.c @@ -8,6 +8,7 @@ #include <bootretry.h> #include <cli.h> #include <env.h> +#include <env_callback.h> #include <errno.h> #include <time.h> #include <vsprintf.h> @@ -19,10 +20,9 @@ static int retry_time = -1; /* -1 so can call readline before main_loop */ /*************************************************************************** * initialize command line timeout */ -void bootretry_init_cmd_timeout(void) -{ - char *s = env_get("bootretry"); +static void bootretry_parse(const char *s) +{ if (s != NULL) retry_time = (int)simple_strtol(s, NULL, 10); else @@ -32,13 +32,31 @@ void bootretry_init_cmd_timeout(void) retry_time = CONFIG_BOOT_RETRY_MIN; } +void bootretry_init_cmd_timeout(void) +{ + bootretry_parse(env_get("bootretry")); +} + +/* Parse changes to bootretry */ +static int on_bootretry(const char *name, const char *value, enum env_op op, + int flags) +{ + switch (op) { + case env_op_create: + case env_op_overwrite: + case env_op_delete: + bootretry_parse(value); + break; + } + return 0; +} +U_BOOT_ENV_CALLBACK(bootretry, on_bootretry); + /*************************************************************************** * reset command line timeout to retry_time seconds */ void bootretry_reset_cmd_timeout(void) { - /* Parse changes to bootretry */ - bootretry_init_cmd_timeout(); endtime = endtick(retry_time); } diff --git a/include/env_callback.h b/include/env_callback.h index 1181ab4a157..572cacde03e 100644 --- a/include/env_callback.h +++ b/include/env_callback.h @@ -70,6 +70,12 @@ #define DFU_CALLBACK #endif +#ifdef CONFIG_BOOT_RETRY +#define BOOTRETRY_CALLBACK "bootretry:bootretry," +#else +#define BOOTRETRY_CALLBACK +#endif + /* * This list of callback bindings is static, but may be overridden by defining * a new association in the ".callbacks" environment variable. @@ -81,6 +87,7 @@ NET6_CALLBACKS \ BOOTSTD_CALLBACK \ DFU_CALLBACK \ + BOOTRETRY_CALLBACK \ "loadaddr:loadaddr," \ SILENT_CALLBACK \ "stdin:console,stdout:console,stderr:console," \ -- 2.55.0
