[U-Boot] [PATCH 2/3] env_mmc: support env partition setup in runtime

[Pantelis Antoniou]

Hi Dmitry,

I took a good look at the patch and there's a problem.

On Apr 27, 2014, at 1:18 PM, Dmitry Lifshitz wrote:

> Add callback with __weak annotation to allow setup of environment
> partition number in runtime from a board file.
> 
> Signed-off-by: Dmitry Lifshitz <lifshitz at compulab.co.il>
> Signed-off-by: Igor Grinberg <grinberg at compulab.co.il>
> ---
> common/env_mmc.c |   35 ++++++++++++++++++++++++++---------
> 1 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/common/env_mmc.c b/common/env_mmc.c
> index 045428c..5d4b5f4 100644
> --- a/common/env_mmc.c
> +++ b/common/env_mmc.c
> @@ -62,6 +62,30 @@ int env_init(void)
> 	return 0;
> }
> 
> +
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> +__weak uint mmc_get_env_part(struct mmc *mmc)
> +{
> +	return CONFIG_SYS_MMC_ENV_PART;
> +}
> +
> +static int mmc_set_env_part(struct mmc *mmc)
> +{
> +	uint part = mmc_get_env_part(mmc);
> +
> +	if (part != mmc->part_num) {
> +		if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV, part)) {
> +			puts("MMC partition switch failed\n");
> +			return -1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +#else
> +static inline int mmc_set_env_part(struct mmc *mmc) {return 0; };
> +#endif
> +
> static int init_mmc_for_env(struct mmc *mmc)
> {
> 	if (!mmc) {
> @@ -74,15 +98,8 @@ static int init_mmc_for_env(struct mmc *mmc)
> 		return -1;
> 	}
> 

Just before this hunk, we have this:

> #ifdef CONFIG_SYS_MMC_ENV_PART
>         int dev = CONFIG_SYS_MMC_ENV_DEV;
> 
> #ifdef CONFIG_SPL_BUILD
>         dev = 0;
> #endif
> #endif
> 

This appears to be broken for SPL in case that CONFIG_SYS_MMC_ENV_DEV is not 0.

SPL hardcoded dev to 0, while mmc_switch_part is implicitly operating on
CONFIG_SYS_MMC_ENV_DEV.

Please rework it so that the SPL case is unaffected.

> -#ifdef CONFIG_SYS_MMC_ENV_PART
> -	if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) {
> -		if (mmc_switch_part(CONFIG_SYS_MMC_ENV_DEV,
> -				    CONFIG_SYS_MMC_ENV_PART)) {
> -			puts("MMC partition switch failed\n");
> -			return -1;
> -		}
> -	}
> -#endif
> +	if (mmc_set_env_part(mmc))
> +		return -1;
> 
> 	return 0;
> }
> -- 
> 1.7.5.4

Regards

-- Pantelis