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.
}