[PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

[PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

[Marek Vasut]

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>
---
V2: Add documentation of this behavior into Kconfig help text
---
 env/Kconfig |  5 +++++
 env/mmc.c   | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index f75f2b13536..06d72bad1dc 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -200,6 +200,11 @@ config ENV_IS_IN_MMC
 	  This value may also be positive or negative; this is handled in the
 	  same way as CONFIG_ENV_OFFSET.
 
+	  In case CONFIG_SYS_MMC_ENV_PART is 1 (i.e. environment in eMMC boot
+	  partition) then setting CONFIG_ENV_OFFSET_REDUND to the same value
+	  as CONFIG_ENV_OFFSET makes use of the second eMMC boot partition for
+	  the redundant environment copy.
+
 	  This value is also in units of bytes, but must also be aligned to
 	  an MMC sector boundary.
 
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,
-- 
2.33.0

Re: [PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

[Jaehoon Chung]

Hi Marek,

On 10/18/21 2:23 AM, 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.

Is there any patch before apply this? When I had been trying to test this, I can't apply this from patchwork.

Best Regards,
Jaehoon Chung

> 
> 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>
> ---
> V2: Add documentation of this behavior into Kconfig help text
> ---
>  env/Kconfig |  5 +++++
>  env/mmc.c   | 47 ++++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 47 insertions(+), 5 deletions(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index f75f2b13536..06d72bad1dc 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -200,6 +200,11 @@ config ENV_IS_IN_MMC
>  	  This value may also be positive or negative; this is handled in the
>  	  same way as CONFIG_ENV_OFFSET.
>  
> +	  In case CONFIG_SYS_MMC_ENV_PART is 1 (i.e. environment in eMMC boot
> +	  partition) then setting CONFIG_ENV_OFFSET_REDUND to the same value
> +	  as CONFIG_ENV_OFFSET makes use of the second eMMC boot partition for
> +	  the redundant environment copy.
> +
>  	  This value is also in units of bytes, but must also be aligned to
>  	  an MMC sector boundary.
>  
> 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,
> 

Re: [PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

[Marek Vasut]

On 10/18/21 4:02 AM, Jaehoon Chung wrote:
> Hi Marek,

Hi,

> On 10/18/21 2:23 AM, 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.
> 
> Is there any patch before apply this? When I had been trying to test this, I can't apply this from patchwork.

Uh, pick this one fix, I missed it in this v2 resend:
https://patchwork.ozlabs.org/project/uboot/patch/20211006162954.63596-1-marex@denx.de/

Re: [PATCH v2] env: mmc: Add support for redundant env in both eMMC boot partitions

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On Sun, Oct 17, 2021 at 07:23:36PM +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>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]