[Uboot-stm32] [PATCH 2/8] env: mcc: Drop unnecessary #ifdefs

Patrice CHOTARD patrice.chotard at foss.st.com
Tue Dec 6 09:11:14 CET 2022



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.delaunay at 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.chotard at foss.st.com>

Thanks
Patrice


More information about the U-Boot mailing list