From 6731bef6966ea2b26cdcfe0109ff5a950003fd03 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Fri, 19 Jun 2020 23:07:17 +0100 Subject: [PATCH 01/10] env/fat.c: allow loading from a FAT partition on the MMC boot device I don't want to have to specify the device; only the partition. This allows me to use the same image on internal eMMC or SD card for Banana Pi R2, and it finds its own environment either way. Signed-off-by: David Woodhouse [trini: Add #if/#else/#endif logic around CONFIG_SYS_MMC_ENV_DEV usage, whitespace changes] Signed-off-by: Tom Rini --- env/Kconfig | 4 ++++ env/fat.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/env/Kconfig b/env/Kconfig index 38e7fadbb93..57841366742 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -434,6 +434,10 @@ config ENV_FAT_DEVICE_AND_PART If none, first valid partition in device D. If no partition table then means device D. + If ENV_FAT_INTERFACE is set to "mmc" then device 'D' can be omitted, + leaving the string starting with a colon, and the boot device will + be used. + config ENV_FAT_FILE string "Name of the FAT file to use for the environment" depends on ENV_IS_IN_FAT diff --git a/env/fat.c b/env/fat.c index 35a1955e634..63aced93179 100644 --- a/env/fat.c +++ b/env/fat.c @@ -29,6 +29,34 @@ # define LOADENV #endif +__weak int mmc_get_env_dev(void) +{ +#ifdef CONFIG_SYS_MMC_ENV_DEV + return CONFIG_SYS_MMC_ENV_DEV; +#else + return 0; +#endif +} + +static char *env_fat_device_and_part(void) +{ +#ifdef CONFIG_MMC + static char *part_str; + + if (!part_str) { + part_str = CONFIG_ENV_FAT_DEVICE_AND_PART; + if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc") && part_str[0] == ':') { + part_str = "0" CONFIG_ENV_FAT_DEVICE_AND_PART; + part_str[0] += mmc_get_env_dev(); + } + } + + return part_str; +#else + return CONFIG_ENV_FAT_DEVICE_AND_PART; +#endif +} + static int env_fat_save(void) { env_t __aligned(ARCH_DMA_MINALIGN) env_new; @@ -43,7 +71,7 @@ static int env_fat_save(void) return err; part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE, - CONFIG_ENV_FAT_DEVICE_AND_PART, + env_fat_device_and_part(), &dev_desc, &info, 1); if (part < 0) return 1; @@ -89,7 +117,7 @@ static int env_fat_load(void) #endif part = blk_get_device_part_str(CONFIG_ENV_FAT_INTERFACE, - CONFIG_ENV_FAT_DEVICE_AND_PART, + env_fat_device_and_part(), &dev_desc, &info, 1); if (part < 0) goto err_env_relocate; From d5a6a5a9271bd93d0c8337788ddaac708c9123ee Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Wed, 24 Jun 2020 09:27:25 +0200 Subject: [PATCH 02/10] env: correct overflow check of env_has_init size Correct the overflow check of the bit-field env_has_init with the max value of env_location= ENVL_COUNT and no more with the size of env_locations. This bit-field is indexed by this enumerate and not by the position in the env_locations (only used in env_get_location) and the 2 values are different, depending of thea ctivated CONFIG_ENV_IS_ options. Signed-off-by: Patrick Delaunay --- env/env.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/env.c b/env/env.c index dcc25c030b2..49545a8d9cd 100644 --- a/env/env.c +++ b/env/env.c @@ -103,7 +103,7 @@ static void env_set_inited(enum env_location location) * using the above enum value as the bit index. We need to * make sure that we're not overflowing it. */ - BUILD_BUG_ON(ARRAY_SIZE(env_locations) > BITS_PER_LONG); + BUILD_BUG_ON(ENVL_COUNT > BITS_PER_LONG); gd->env_has_init |= BIT(location); } From 8968288cb477ba69b002db01a6407cf78e3a6e06 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Wed, 24 Jun 2020 10:17:50 +0200 Subject: [PATCH 03/10] env: add failing trace in env_save Add trace in env save to indicate any errors to end user and avoid silent output when the command 'env save' is not executed. Signed-off-by: Patrick Delaunay --- env/env.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/env/env.c b/env/env.c index 49545a8d9cd..2e64346438e 100644 --- a/env/env.c +++ b/env/env.c @@ -240,13 +240,17 @@ int env_save(void) if (drv) { int ret; - if (!drv->save) - return -ENODEV; - - if (!env_has_inited(drv->location)) - return -ENODEV; - printf("Saving Environment to %s... ", drv->name); + if (!drv->save) { + printf("not possible\n"); + return -ENODEV; + } + + if (!env_has_inited(drv->location)) { + printf("not initialized\n"); + return -ENODEV; + } + ret = drv->save(); if (ret) printf("Failed (%d)\n", ret); From 6718ebd0327a255c6e3e840c3105ee397c8c0326 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Fri, 19 Jun 2020 14:03:34 +0200 Subject: [PATCH 04/10] cmd: env: add option for quiet output on env info The "env info" can be use for test with -d and -p parameter, in scripting case the output of the command is not needed. This patch allows to deactivate this output with a new option "-q". For example, we can save the environment if default environment is used and persistent storage is managed with: if env info -p -d -q; then env save; fi Without the quiet option, I have the unnecessary traces First boot: Default environment is used Environment can be persisted Saving Environment to EXT4... File System is consistent Next boot: Environment was loaded from persistent storage Environment can be persisted Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass Reviewed-by: Tom Rini --- cmd/Kconfig | 1 + cmd/nvedit.c | 26 +++++++++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index bfe6c163dc3..e2b0a4fbc01 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -601,6 +601,7 @@ config CMD_NVEDIT_INFO This command can be optionally used for evaluation in scripts: [-d] : evaluate whether default environment is used [-p] : evaluate whether environment can be persisted + [-q] : quiet output The result of multiple evaluations will be combined with AND. endmenu diff --git a/cmd/nvedit.c b/cmd/nvedit.c index ca0be92fc39..6d7da097900 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1224,12 +1224,15 @@ static int print_env_info(void) * env info - display environment information * env info [-d] - evaluate whether default environment is used * env info [-p] - evaluate whether environment can be persisted + * Add [-q] - quiet mode, use only for command result, for test by example: + * test env info -p -d -q */ static int do_env_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { int eval_flags = 0; int eval_results = 0; + bool quiet = false; /* display environment information */ if (argc <= 1) @@ -1247,6 +1250,9 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, case 'p': eval_flags |= ENV_INFO_IS_PERSISTED; break; + case 'q': + quiet = true; + break; default: return CMD_RET_USAGE; } @@ -1256,20 +1262,24 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, /* evaluate whether default environment is used */ if (eval_flags & ENV_INFO_IS_DEFAULT) { if (gd->flags & GD_FLG_ENV_DEFAULT) { - printf("Default environment is used\n"); + if (!quiet) + printf("Default environment is used\n"); eval_results |= ENV_INFO_IS_DEFAULT; } else { - printf("Environment was loaded from persistent storage\n"); + if (!quiet) + printf("Environment was loaded from persistent storage\n"); } } /* evaluate whether environment can be persisted */ if (eval_flags & ENV_INFO_IS_PERSISTED) { #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) - printf("Environment can be persisted\n"); + if (!quiet) + printf("Environment can be persisted\n"); eval_results |= ENV_INFO_IS_PERSISTED; #else - printf("Environment cannot be persisted\n"); + if (!quiet) + printf("Environment cannot be persisted\n"); #endif } @@ -1326,7 +1336,7 @@ static struct cmd_tbl cmd_env_sub[] = { U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""), #endif #if defined(CONFIG_CMD_NVEDIT_INFO) - U_BOOT_CMD_MKENT(info, 2, 0, do_env_info, "", ""), + U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""), #endif U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""), #if defined(CONFIG_CMD_RUN) @@ -1405,8 +1415,10 @@ static char env_help_text[] = #endif #if defined(CONFIG_CMD_NVEDIT_INFO) "env info - display environment information\n" - "env info [-d] - whether default environment is used\n" - "env info [-p] - whether environment can be persisted\n" + "env info [-d] [-p] [-q] - evaluate environment information\n" + " \"-d\": default environment is used\n" + " \"-p\": environment can be persisted\n" + " \"-q\": quiet output\n" #endif "env print [-a | name ...] - print environment\n" #if defined(CONFIG_CMD_NVEDIT_EFI) From 2f96b3238ca330897b8eb5bde82db2da2c07effb Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Fri, 19 Jun 2020 14:03:35 +0200 Subject: [PATCH 05/10] cmd: env: check real location for env info command Check the current ENV location, dynamically provided by the weak function env_get_location to be sure that the environment can be persistent. The compilation flag ENV_IS_IN_DEVICE is not enough when the board dynamically select the available storage location (according boot device for example). This patch solves issue for stm32mp1 platform, when the boot device is USB. Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass Reviewed-by: Tom Rini --- cmd/nvedit.c | 15 ++++++++++++--- include/env_internal.h | 11 +++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 6d7da097900..acd9f826676 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1233,6 +1233,9 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, int eval_flags = 0; int eval_results = 0; bool quiet = false; +#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) + enum env_location loc; +#endif /* display environment information */ if (argc <= 1) @@ -1274,9 +1277,15 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, /* evaluate whether environment can be persisted */ if (eval_flags & ENV_INFO_IS_PERSISTED) { #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) - if (!quiet) - printf("Environment can be persisted\n"); - eval_results |= ENV_INFO_IS_PERSISTED; + loc = env_get_location(ENVOP_SAVE, gd->env_load_prio); + if (ENVL_NOWHERE != loc && ENVL_UNKNOWN != loc) { + if (!quiet) + printf("Environment can be persisted\n"); + eval_results |= ENV_INFO_IS_PERSISTED; + } else { + if (!quiet) + printf("Environment cannot be persisted\n"); + } #else if (!quiet) printf("Environment cannot be persisted\n"); diff --git a/include/env_internal.h b/include/env_internal.h index e89fbdb1b75..66550434c3f 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -211,6 +211,17 @@ struct env_driver { extern struct hsearch_data env_htab; +/** + * env_get_location()- Provide the best location for the U-Boot environment + * + * It is a weak function allowing board to overidde the environment location + * + * @op: operations performed on the environment + * @prio: priority between the multiple environments, 0 being the + * highest priority + * @return an enum env_location value on success, or -ve error code. + */ +enum env_location env_get_location(enum env_operation op, int prio); #endif /* DO_DEPS_ONLY */ #endif /* _ENV_INTERNAL_H_ */ From 7d813449b8f266bb9e2261e3f175748f5319fda2 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Fri, 19 Jun 2020 14:03:36 +0200 Subject: [PATCH 06/10] configs: sandbox: Enable sub command 'env info' Enable support for sub command 'env info' in sandbox with CONFIG_CMD_NVEDIT_INFO. This is aimed primarily at adding unit test. Signed-off-by: Patrick Delaunay Reviewed-by: Simon Glass --- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 4 files changed, 4 insertions(+) diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index dcf2f44b584..45edd3bd959 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -30,6 +30,7 @@ CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 6059d668af7..9f3ac9018fd 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 4158b9b86d4..efdacefc25d 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -24,6 +24,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index b3274a93b41..c2f873287e1 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y From acbf93b52695431b683d2665120707a3f52d6b4b Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Fri, 19 Jun 2020 14:03:37 +0200 Subject: [PATCH 07/10] test: env: add test for env info sub-command Add a pytest for testing the env info sub-command: test_env_info: test command with several option that can be executed on real hardware device without assumption test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi Signed-off-by: Patrick Delaunay Acked-by: Stephen Warren --- test/py/tests/test_env.py | 63 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index 6ff38f1020b..a64aaa9bc5a 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -336,3 +336,66 @@ def test_env_import_whitelist_delete(state_test_env): unset_var(state_test_env, 'foo2') unset_var(state_test_env, 'foo3') unset_var(state_test_env, 'foo4') + +@pytest.mark.buildconfigspec('cmd_nvedit_info') +def test_env_info(state_test_env): + + """Test 'env info' command with all possible options. + """ + c = state_test_env.u_boot_console + + response = c.run_command('env info') + nb_line = 0 + for l in response.split('\n'): + if 'env_valid = ' in l: + assert '= invalid' in l or '= valid' in l or '= redundant' in l + nb_line += 1 + elif 'env_ready =' in l or 'env_use_default =' in l: + assert '= true' in l or '= false' in l + nb_line += 1 + else: + assert true + assert nb_line == 3 + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response or "Environment was loaded from persistent storage" in response + assert 'Environment can be persisted' in response or "Environment cannot be persisted" in response + + response = c.run_command('env info -p -d -q') + assert response == "" + + response = c.run_command('env info -p -q') + assert response == "" + + response = c.run_command('env info -d -q') + assert response == "" + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_nvedit_info') +@pytest.mark.buildconfigspec('cmd_echo') +def test_env_info_sandbox(state_test_env): + + """Test 'env info' command result with several options on sandbox + with a known ENV configuration: ready & default & persistent + """ + c = state_test_env.u_boot_console + + response = c.run_command('env info') + assert 'env_ready = true' in response + assert 'env_use_default = true' in response + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response + assert 'Environment cannot be persisted' in response + + response = c.run_command('env info -d -q') + response = c.run_command('echo $?') + assert response == "0" + + response = c.run_command('env info -p -q') + response = c.run_command('echo $?') + assert response == "1" + + response = c.run_command('env info -d -p -q') + response = c.run_command('echo $?') + assert response == "1" From 2b2f727500dc934ce201f1445c94c540ecae2798 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 15 Jun 2020 10:38:55 +0200 Subject: [PATCH 08/10] env: mmc: allow support of mmc_get_env_dev with OF_CONTROL Use the weak function mmc_get_env_dev in mmc_offset_try_partition function to allow dynamic selection of mmc device to use and no more use directly the define CONFIG_SYS_MMC_ENV_DEV. Signed-off-by: Patrick Delaunay --- env/mmc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/env/mmc.c b/env/mmc.c index a8b661db80a..e67ef90bcec 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -24,14 +24,25 @@ DECLARE_GLOBAL_DATA_PTR; +#if !defined(CONFIG_SYS_MMC_ENV_DEV) +#define CONFIG_SYS_MMC_ENV_DEV 0 +#endif + +__weak int mmc_get_env_dev(void) +{ + return CONFIG_SYS_MMC_ENV_DEV; +} + #if CONFIG_IS_ENABLED(OF_CONTROL) static inline int mmc_offset_try_partition(const char *str, s64 *val) { struct disk_partition info; struct blk_desc *desc; int len, i, ret; + char dev_str[4]; - ret = blk_get_device_by_str("mmc", STR(CONFIG_SYS_MMC_ENV_DEV), &desc); + snprintf(dev_str, sizeof(dev_str), "%d", mmc_get_env_dev()); + ret = blk_get_device_by_str("mmc", dev_str, &desc); if (ret < 0) return (ret); @@ -114,11 +125,6 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr) return 0; } -__weak int mmc_get_env_dev(void) -{ - return CONFIG_SYS_MMC_ENV_DEV; -} - #ifdef CONFIG_SYS_MMC_ENV_PART __weak uint mmc_get_env_part(struct mmc *mmc) { From 76b640c3f2c7299acc5f1e89f8788862667cc5a0 Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 15 Jun 2020 10:38:56 +0200 Subject: [PATCH 09/10] env: mmc: correct the offset returned by mmc_offset_try_partition The output of the function mmc_offset_try_partition must be a byte offset in mmc and not a multiple of blksz. This function is used in mmc_offset(), called by mmc_get_env_addr() and the offset is used in write_env(), erase_env() and read_env(). In these function, blk_start = offset / mmc->read_bl_len or /write_bl_len so this offset is not a multiple of blksz. Signed-off-by: Patrick Delaunay --- env/mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/env/mmc.c b/env/mmc.c index e67ef90bcec..5de4a458171 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -56,10 +56,10 @@ static inline int mmc_offset_try_partition(const char *str, s64 *val) } /* round up to info.blksz */ - len = (CONFIG_ENV_SIZE + info.blksz - 1) & ~(info.blksz - 1); + len = DIV_ROUND_UP(CONFIG_ENV_SIZE, info.blksz); /* use the top of the partion for the environment */ - *val = (info.start + info.size - 1) - len / info.blksz; + *val = (info.start + info.size - len) * info.blksz; return 0; } From 5d4f7b4e2a1a2df459172ec95cbcdd6373dd707a Mon Sep 17 00:00:00 2001 From: Patrick Delaunay Date: Mon, 15 Jun 2020 10:38:57 +0200 Subject: [PATCH 10/10] env: mmc: add redundancy support in mmc_offset_try_partition Manage 2 copy at the end of the partition selected by config "u-boot,mmc-env-partition" to save the U-Boot environment, with CONFIG_ENV_SIZE and 2*CONFIG_ENV_SIZE offset. This patch allows to support redundancy (CONFIG_ENV_OFFSET_REDUND). Signed-off-by: Patrick Delaunay --- env/mmc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/env/mmc.c b/env/mmc.c index 5de4a458171..aca61b75e99 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -34,7 +34,7 @@ __weak int mmc_get_env_dev(void) } #if CONFIG_IS_ENABLED(OF_CONTROL) -static inline int mmc_offset_try_partition(const char *str, s64 *val) +static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val) { struct disk_partition info; struct blk_desc *desc; @@ -59,7 +59,7 @@ static inline int mmc_offset_try_partition(const char *str, s64 *val) len = DIV_ROUND_UP(CONFIG_ENV_SIZE, info.blksz); /* use the top of the partion for the environment */ - *val = (info.start + info.size - len) * info.blksz; + *val = (info.start + info.size - (1 + copy) * len) * info.blksz; return 0; } @@ -84,7 +84,7 @@ static inline s64 mmc_offset(int copy) str = fdtdec_get_config_string(gd->fdt_blob, dt_prop.partition); if (str) { /* try to place the environment at end of the partition */ - err = mmc_offset_try_partition(str, &val); + err = mmc_offset_try_partition(str, copy, &val); if (!err) return val; }