Re: [PATCH] dfu: define a callback function for the dfu_alt_info environment variable

Hi Rasmus,

Thank you for the patch.

On mer., sept. 11, 2024 at 15:39, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:

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 rasmus.villemoes@prevas.dk

Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

---

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 540d48fab77..7a4d7ba2a7f 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 8f7ecfa8fc7..c19eb919388 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 6c5431b3948..e25588c33cb 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 8e500aaaf80..66cc8309e71 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.46.0