[PATCH 1/2] env: mmc: Add missing eMMC bootpart restoration to env erase
If the environment is stored in eMMC hardware boot partition, the environment driver first stores the currently selected eMMC boot partition, then does the requested operation, and then restores the original boot partition settings. In case the environment operation fails, the boot partition settings are also restored.
The 'env erase' implementation in the MMC environment driver lacks the path which restores the boot partition. This could lead to various failure modes, like the system boots the wrong copy of bootloader etc. Fix this by filling in the missing restoration path.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de --- env/mmc.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c index c4cb1639914..e111d8e5881 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -263,20 +263,26 @@ static int env_mmc_erase(void) return 1; }
- if (mmc_get_env_addr(mmc, copy, &offset)) - return CMD_RET_FAILURE; + if (mmc_get_env_addr(mmc, copy, &offset)) { + ret = CMD_RET_FAILURE; + goto fini; + }
ret = erase_env(mmc, CONFIG_ENV_SIZE, offset);
#ifdef CONFIG_ENV_OFFSET_REDUND copy = 1;
- if (mmc_get_env_addr(mmc, copy, &offset)) - return CMD_RET_FAILURE; + if (mmc_get_env_addr(mmc, copy, &offset)) { + ret = CMD_RET_FAILURE; + goto fini; + }
ret |= erase_env(mmc, CONFIG_ENV_SIZE, offset); #endif
+fini: + fini_mmc_for_env(mmc); return ret; } #endif /* CONFIG_CMD_SAVEENV && !CONFIG_SPL_BUILD */
Currently the MMC environment driver supports storing redundant environment only in one eMMC partition at different offsets. This is sub-optimal, since if this one boot partition is erased, both copies of environment are lost. Since the eMMC has two boot partitions, add support for storing one copy of environment in each of the two boot partitions.
To enable this functionality, select CONFIG_SYS_REDUNDAND_ENVIRONMENT to indicate redundant environment should be used. Set CONFIG_SYS_MMC_ENV_PART to 1 to indicate environment should be stored in eMMC boot partition. Set CONFIG_ENV_OFFSET equal to CONFIG_ENV_OFFSET_REDUND, and both to the offset from start of eMMC boot partition where the environment should be located.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de --- env/mmc.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c index e111d8e5881..465b104559b 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -26,6 +26,18 @@
DECLARE_GLOBAL_DATA_PTR;
+/* + * In case the environment is redundant, stored in eMMC hardware boot + * partition and the environment and redundant environment offsets are + * identical, store the environment and redundant environment in both + * eMMC boot partitions, one copy in each. + * */ +#if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \ + (CONFIG_SYS_MMC_ENV_PART == 1) && \ + (CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND)) +#define ENV_MMC_HWPART_REDUND +#endif + #if CONFIG_IS_ENABLED(OF_CONTROL) static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val) { @@ -126,13 +138,11 @@ __weak uint mmc_get_env_part(struct mmc *mmc)
static unsigned char env_mmc_orig_hwpart;
-static int mmc_set_env_part(struct mmc *mmc) +static int mmc_set_env_part(struct mmc *mmc, uint part) { - uint part = mmc_get_env_part(mmc); int dev = mmc_get_env_dev(); int ret = 0;
- env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart; ret = blk_select_hwpart_devnum(IF_TYPE_MMC, dev, part); if (ret) puts("MMC partition switch failed\n"); @@ -140,7 +150,7 @@ static int mmc_set_env_part(struct mmc *mmc) return ret; } #else -static inline int mmc_set_env_part(struct mmc *mmc) {return 0; }; +static inline int mmc_set_env_part(struct mmc *mmc, uint part) {return 0; }; #endif
static const char *init_mmc_for_env(struct mmc *mmc) @@ -157,7 +167,8 @@ static const char *init_mmc_for_env(struct mmc *mmc) if (mmc_init(mmc)) return "MMC init failed"; #endif - if (mmc_set_env_part(mmc)) + env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart; + if (mmc_set_env_part(mmc, mmc_get_env_part(mmc))) return "MMC partition switch failed";
return NULL; @@ -209,6 +220,13 @@ static int env_mmc_save(void) #ifdef CONFIG_ENV_OFFSET_REDUND if (gd->env_valid == ENV_VALID) copy = 1; + +#ifdef ENV_MMC_HWPART_REDUND + ret = mmc_set_env_part(mmc, copy + 1); + if (ret) + goto fini; +#endif + #endif
if (mmc_get_env_addr(mmc, copy, &offset)) { @@ -273,6 +291,12 @@ static int env_mmc_erase(void) #ifdef CONFIG_ENV_OFFSET_REDUND copy = 1;
+#ifdef ENV_MMC_HWPART_REDUND + ret = mmc_set_env_part(mmc, copy + 1); + if (ret) + goto fini; +#endif + if (mmc_get_env_addr(mmc, copy, &offset)) { ret = CMD_RET_FAILURE; goto fini; @@ -331,7 +355,20 @@ static int env_mmc_load(void) goto fini; }
+#ifdef ENV_MMC_HWPART_REDUND + ret = mmc_set_env_part(mmc, 1); + if (ret) + goto fini; +#endif + read1_fail = read_env(mmc, CONFIG_ENV_SIZE, offset1, tmp_env1); + +#ifdef ENV_MMC_HWPART_REDUND + ret = mmc_set_env_part(mmc, 2); + if (ret) + goto fini; +#endif + read2_fail = read_env(mmc, CONFIG_ENV_SIZE, offset2, tmp_env2);
ret = env_import_redund((char *)tmp_env1, read1_fail, (char *)tmp_env2,
On Wed, Oct 06, 2021 at 06:29:54PM +0200, Marek Vasut wrote:
Currently the MMC environment driver supports storing redundant environment only in one eMMC partition at different offsets. This is sub-optimal, since if this one boot partition is erased, both copies of environment are lost. Since the eMMC has two boot partitions, add support for storing one copy of environment in each of the two boot partitions.
To enable this functionality, select CONFIG_SYS_REDUNDAND_ENVIRONMENT to indicate redundant environment should be used. Set CONFIG_SYS_MMC_ENV_PART to 1 to indicate environment should be stored in eMMC boot partition. Set CONFIG_ENV_OFFSET equal to CONFIG_ENV_OFFSET_REDUND, and both to the offset from start of eMMC boot partition where the environment should be located.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de
env/mmc.c | 47 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c index e111d8e5881..465b104559b 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -26,6 +26,18 @@
DECLARE_GLOBAL_DATA_PTR;
+/*
- In case the environment is redundant, stored in eMMC hardware boot
- partition and the environment and redundant environment offsets are
- identical, store the environment and redundant environment in both
- eMMC boot partitions, one copy in each.
- */
+#if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \
(CONFIG_SYS_MMC_ENV_PART == 1) && \(CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND))+#define ENV_MMC_HWPART_REDUND +#endif
This needs to be documented in more than just the header, it should be in Kconfig at least. And if we could enable this directly via a Kconfig option instead, that would be even better.
On Wed, Oct 06, 2021 at 06:29:53PM +0200, Marek Vasut wrote:
If the environment is stored in eMMC hardware boot partition, the environment driver first stores the currently selected eMMC boot partition, then does the requested operation, and then restores the original boot partition settings. In case the environment operation fails, the boot partition settings are also restored.
The 'env erase' implementation in the MMC environment driver lacks the path which restores the boot partition. This could lead to various failure modes, like the system boots the wrong copy of bootloader etc. Fix this by filling in the missing restoration path.
Signed-off-by: Marek Vasut marex@denx.de Cc: Fabio Estevam festevam@gmail.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de
Applied to u-boot/master, thanks!
participants (2)
-
Marek Vasut -
Tom Rini