On Tue, Dec 17, 2024 at 02:48:35PM -0600, Jonathan Humphreys wrote: Hello Jon,
Thank you for posting this patch. I will drop the equivalent of this patch when I post the v2 series for: https://patchwork.ozlabs.org/project/uboot/cover/20241217131658.2920799-1-s-vadapa...@ti.com/ > If CONFIG_SET_DFU_ALT_INFO is enabled, the dfu_alt_info environment > variable is dynamically set when initializing the DFU entities, which is > done as part of normal flow of a DFU operation. > > The USB DFU boot support will set it's own specific value for dfu_alt_info nitpick: s/it's/its > before performing the DFU operation. This means that if > CONFIG_SET_DFU_ALT_INFO is enabled, the dfu_alt_info environment variable > that the USB DFU boot path had set is overwritten, causing USB DFU boot to > fail. > > Likewise, if the user sets their own value for dfu_alt_info, say at the > U-Boot prompt, it get's overwritten if CONFIG_SET_DFU_ALT_INFO is enabled. nitpick: s/get's/gets > > This patch will first check that dfu_alt_info isn't already set before > calling set_dfu_alt_info(), when CONFIG_SET_DFU_ALT_INFO is enabled. Rather than referring to "patch", we could rephrase the above as: Check that dfu_alt_info isn't already set before calling set_dfu_alt_info() when CONFIG_SET_DFU_ALT_INFO is enabled, in order to avoid overwriting it. > > Signed-off-by: Jonathan Humphreys <j-humphr...@ti.com> > --- > drivers/dfu/dfu.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c > index 756569217bb..ab8abae1d89 100644 > --- a/drivers/dfu/dfu.c > +++ b/drivers/dfu/dfu.c > @@ -169,10 +169,13 @@ int dfu_init_env_entities(char *interface, char *devstr) > dfu_reinit_needed = false; > dfu_alt_info_changed = false; > > + str_env = env_get("dfu_alt_info"); > #ifdef CONFIG_SET_DFU_ALT_INFO > - set_dfu_alt_info(interface, devstr); > + if (!str_env) { > + set_dfu_alt_info(interface, devstr); > + str_env = env_get("dfu_alt_info"); > + } > #endif > - str_env = env_get("dfu_alt_info"); > if (!str_env) { > pr_err("\"dfu_alt_info\" env variable not defined!\n"); > return -EINVAL; Regards, Siddharth.
