From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Wed, 15 Jan 2014 10:37:27 -0700 Subject: [U-Boot] [PATCH 1/2] env_mmc: make board configurable the partition for the environment In-Reply-To: <1389783236-8464-1-git-send-email-hector.palacios@digi.com> References: <1389783236-8464-1-git-send-email-hector.palacios@digi.com> Message-ID: <52D6C757.1050400@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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. > }