On 01/15/2014 03:53 AM, Hector Palacios wrote:
> This complements commit 9404a5fc7cb58 "env_mmc: allow environment to be
> in an eMMC partition" by allowing boards to accommodate the partition
> to use for the environment in different scenarios (similarly to what is
> done with the mmc dev number). Depending on the detected boot media,
> boards may decide to store the environment in a different partition.
>
> The __weak function also allows to remove some ifdefs from the code.
> If CONFIG_SYS_MMC_ENV_PART is not defined, partition 0 is assumed
> (default value for U-Boot when a partition is not provided).
One advantage of the ifdefs is that it means zero code size bloat for
people who haven't defined CONFIG_SYS_MMC_ENV_PART. I'm not that
concerned about the issue for this amount of code, so don't take this as
a nak from me, but I'm sure others will be.
Also, it seems like a good idea to explicitly force people to think
about the partition ID they want the environment stored in, rather than
assume some potentially incorrect default for it?
> diff --git a/common/env_mmc.c b/common/env_mmc.c
> +__weak int mmc_get_env_partno(void)
> +{
> +#ifdef CONFIG_SYS_MMC_ENV_PART
> + return CONFIG_SYS_MMC_ENV_PART;
> +#endif
With this implementation, you'd want that to be #else
> + return 0;
... and the #endif here, so you don't end up with two return statements
back-to-back in the function in one case.
> }
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
