On 11/10/22 11:48, Patrick Delaunay wrote:
> This file has a lot of conditional code and much of it is unnecessary.
> Clean this up to reduce the number of build combinations.
>
> This patch replaces the test on CONFIG_ENV_OFFSET_REDUND for the
> more coherent CONFIG_SYS_REDUNDAND_ENVIRONMENT.
>
> This patch also corrects a compilation issue in init_mmc_for_env()
> when CONFIG_SYS_MMC_ENV_PART is not activated, env_mmc_orig_hwpart is
> not defined.
>
> Signed-off-by: Patrick Delaunay <patrick.delau...@foss.st.com>
> ---
>
> env/mmc.c | 120 +++++++++++++++++++++++++++++-------------------------
> 1 file changed, 64 insertions(+), 56 deletions(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index 42bcf7e775cc..b36bd9ad77ee 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -46,7 +46,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \
> (CONFIG_SYS_MMC_ENV_PART == 1) && \
> (CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND))
> -#define ENV_MMC_HWPART_REDUND
> +#define ENV_MMC_HWPART_REDUND 1
> #endif
>
> #if CONFIG_IS_ENABLED(OF_CONTROL)
> @@ -108,12 +108,11 @@ static inline s64 mmc_offset(int copy)
> defvalue = ENV_MMC_OFFSET;
> propname = dt_prop.offset;
>
> -#if defined(CONFIG_ENV_OFFSET_REDUND)
> - if (copy) {
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy) {
> defvalue = ENV_MMC_OFFSET_REDUND;
> propname = dt_prop.offset_redund;
> }
> -#endif
> +
> return ofnode_conf_read_int(propname, defvalue);
> }
> #else
> @@ -121,10 +120,9 @@ static inline s64 mmc_offset(int copy)
> {
> s64 offset = ENV_MMC_OFFSET;
>
> -#if defined(CONFIG_ENV_OFFSET_REDUND)
> - if (copy)
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy)
> offset = ENV_MMC_OFFSET_REDUND;
> -#endif
> +
> return offset;
> }
> #endif
> @@ -165,8 +163,24 @@ static int mmc_set_env_part(struct mmc *mmc, uint part)
>
> return ret;
> }
> +
> +static bool mmc_set_env_part_init(struct mmc *mmc)
> +{
> + env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
> + if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
> + return false;
> +
> + return true;
> +}
> +
> +static int mmc_set_env_part_restore(struct mmc *mmc)
> +{
> + return mmc_set_env_part(mmc, env_mmc_orig_hwpart);
> +}
> #else
> static inline int mmc_set_env_part(struct mmc *mmc, uint part) {return 0; };
> +static bool mmc_set_env_part_init(struct mmc *mmc) {return true; }
> +static inline int mmc_set_env_part_restore(struct mmc *mmc) {return 0; };
> #endif
>
> static const char *init_mmc_for_env(struct mmc *mmc)
> @@ -183,8 +197,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
> if (mmc_init(mmc))
> return "MMC init failed";
> #endif
> - env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
> - if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
> + if (!mmc_set_env_part_init(mmc))
> return "MMC partition switch failed";
>
> return NULL;
> @@ -192,11 +205,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
>
> static void fini_mmc_for_env(struct mmc *mmc)
> {
> -#ifdef CONFIG_SYS_MMC_ENV_PART
> - int dev = mmc_get_env_dev();
> -
> - blk_select_hwpart_devnum(UCLASS_MMC, dev, env_mmc_orig_hwpart);
> -#endif
> + mmc_set_env_part_restore(mmc);
> }
>
> #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
> @@ -233,21 +242,20 @@ static int env_mmc_save(void)
> if (ret)
> goto fini;
>
> -#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
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) {
> + if (gd->env_valid == ENV_VALID)
> + copy = 1;
>
> -#endif
> + if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
> + ret = mmc_set_env_part(mmc, copy + 1);
> + if (ret)
> + goto fini;
> + }
>
> - if (mmc_get_env_addr(mmc, copy, &offset)) {
> - ret = 1;
> - goto fini;
> + if (mmc_get_env_addr(mmc, copy, &offset)) {
> + ret = 1;
> + goto fini;
> + }
> }
>
> printf("Writing to %sMMC(%d)... ", copy ? "redundant " : "", dev);
> @@ -259,12 +267,12 @@ static int env_mmc_save(void)
>
> ret = 0;
>
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> - gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
> + gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID :
> ENV_REDUND;
>
> fini:
> fini_mmc_for_env(mmc);
> +
> return ret;
> }
>
> @@ -308,22 +316,22 @@ static int env_mmc_erase(void)
> printf("\n");
> ret = erase_env(mmc, CONFIG_ENV_SIZE, offset);
>
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> - copy = 1;
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) {
> + copy = 1;
>
> -#ifdef ENV_MMC_HWPART_REDUND
> - ret = mmc_set_env_part(mmc, copy + 1);
> - if (ret)
> - goto fini;
> -#endif
> + if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
> + ret = mmc_set_env_part(mmc, copy + 1);
> + if (ret)
> + goto fini;
> + }
>
> - if (mmc_get_env_addr(mmc, copy, &offset)) {
> - ret = CMD_RET_FAILURE;
> - goto fini;
> - }
> + if (mmc_get_env_addr(mmc, copy, &offset)) {
> + ret = CMD_RET_FAILURE;
> + goto fini;
> + }
>
> - ret |= erase_env(mmc, CONFIG_ENV_SIZE, offset);
> -#endif
> + ret |= erase_env(mmc, CONFIG_ENV_SIZE, offset);
> + }
>
> fini:
> fini_mmc_for_env(mmc);
> @@ -345,7 +353,7 @@ static inline int read_env(struct mmc *mmc, unsigned long
> size,
> return (n == blk_cnt) ? 0 : -1;
> }
>
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> +#if defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
> static int env_mmc_load(void)
> {
> #if !defined(ENV_IS_EMBEDDED)
> @@ -375,19 +383,19 @@ 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
> + if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
> + ret = mmc_set_env_part(mmc, 1);
> + if (ret)
> + goto fini;
> + }
>
> 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
> + if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
> + ret = mmc_set_env_part(mmc, 2);
> + if (ret)
> + goto fini;
> + }
>
> read2_fail = read_env(mmc, CONFIG_ENV_SIZE, offset2, tmp_env2);
>
> @@ -403,7 +411,7 @@ err:
> #endif
> return ret;
> }
> -#else /* ! CONFIG_ENV_OFFSET_REDUND */
> +#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
> static int env_mmc_load(void)
> {
> #if !defined(ENV_IS_EMBEDDED)
> @@ -448,7 +456,7 @@ err:
> #endif
> return ret;
> }
> -#endif /* CONFIG_ENV_OFFSET_REDUND */
> +#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
>
> U_BOOT_ENV_LOCATION(mmc) = {
> .location = ENVL_MMC,
Reviewed-by: Patrice Chotard <patrice.chot...@foss.st.com>
Thanks
Patrice
