From 4b17845529f5f0c94fe68c2daba1948d0ee41bb2 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 11 Sep 2024 15:39:00 +0200 Subject: [PATCH] dfu: define a callback function for the dfu_alt_info environment variable I'm trying to use dfu-util for bootstrapping an stm32mp board. It mostly works fine, but something goes horribly wrong as soon as I make use of the ability to run arbitrary u-boot shell commands. The shell commands themselves work fine, but the heuristic "dfu_alt_info may have changed, we have to reinit" seems to cause the board and/or my host machine to go into some bad state, and further dfu-util commands fail. U-Boot already has a mechanism whereby C code can be told about changes to specific environment variables. So instead of always doing re-init, add a hook to the dfu_alt_info variable so that we only do set dfu_reinit_needed if the commands actually did modify that variable. Signed-off-by: Rasmus Villemoes Reviewed-by: Mattijs Korpershoek Link: https://lore.kernel.org/r/20240911133900.1444083-1-rasmus.villemoes@prevas.dk Signed-off-by: Mattijs Korpershoek --- drivers/dfu/dfu.c | 16 ++++++++++++++++ drivers/dfu/dfu_mmc.c | 3 ++- include/dfu.h | 1 + include/env_callback.h | 7 +++++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 540d48fab7..7a4d7ba2a7 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -27,6 +27,21 @@ static unsigned long dfu_timeout = 0; #endif bool dfu_reinit_needed = false; +bool dfu_alt_info_changed = false; + +static int on_dfu_alt_info(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: + dfu_alt_info_changed = true; + break; + } + return 0; +} +U_BOOT_ENV_CALLBACK(dfu_alt_info, on_dfu_alt_info); /* * The purpose of the dfu_flush_callback() function is to @@ -152,6 +167,7 @@ int dfu_init_env_entities(char *interface, char *devstr) int ret = 0; dfu_reinit_needed = false; + dfu_alt_info_changed = false; #ifdef CONFIG_SET_DFU_ALT_INFO set_dfu_alt_info(interface, devstr); diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 8f7ecfa8fc..c19eb91938 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -232,7 +232,8 @@ int dfu_flush_medium_mmc(struct dfu_entity *dfu) break; case DFU_SCRIPT: /* script may have changed the dfu_alt_info */ - dfu_reinit_needed = true; + if (dfu_alt_info_changed) + dfu_reinit_needed = true; break; case DFU_RAW_ADDR: case DFU_SKIP: diff --git a/include/dfu.h b/include/dfu.h index 6c5431b394..e25588c33c 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -517,6 +517,7 @@ static inline int dfu_fill_entity_virt(struct dfu_entity *dfu, char *devstr, #endif extern bool dfu_reinit_needed; +extern bool dfu_alt_info_changed; #if CONFIG_IS_ENABLED(DFU_WRITE_ALT) /** diff --git a/include/env_callback.h b/include/env_callback.h index 8e500aaaf8..66cc8309e7 100644 --- a/include/env_callback.h +++ b/include/env_callback.h @@ -69,6 +69,12 @@ #define BOOTSTD_CALLBACK #endif +#ifdef CONFIG_DFU +#define DFU_CALLBACK "dfu_alt_info:dfu_alt_info," +#else +#define DFU_CALLBACK +#endif + /* * This list of callback bindings is static, but may be overridden by defining * a new association in the ".callbacks" environment variable. @@ -79,6 +85,7 @@ NET_CALLBACKS \ NET6_CALLBACKS \ BOOTSTD_CALLBACK \ + DFU_CALLBACK \ "loadaddr:loadaddr," \ SILENT_CALLBACK \ "stdin:console,stdout:console,stderr:console," \ -- 2.39.5