diff mbox

[U-Boot,2/2] env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

Message ID 1389783236-8464-2-git-send-email-hector.palacios@digi.com
State Superseded
Delegated to: Pantelis Antoniou
Headers show

Commit Message

Hector Palacios Jan. 15, 2014, 10:53 a.m. UTC
Since function mmc_get_env_devno is __weak and can be overridden by
board code, boards do not necessarily need to define
CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
default.

Signed-off-by: Hector Palacios <hector.palacios@digi.com>
---
 common/env_mmc.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Otavio Salvador Jan. 15, 2014, 11:10 a.m. UTC | #1
On Wed, Jan 15, 2014 at 8:53 AM, Hector Palacios
<hector.palacios@digi.com>wrote:

> Since function mmc_get_env_devno is __weak and can be overridden by
> board code, boards do not necessarily need to define
> CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
> default.
>
> Signed-off-by: Hector Palacios <hector.palacios@digi.com>
>

Maybe use same default define, if not set?
Stephen Warren Jan. 15, 2014, 5:40 p.m. UTC | #2
On 01/15/2014 03:53 AM, Hector Palacios wrote:
> Since function mmc_get_env_devno is __weak and can be overridden by
> board code, boards do not necessarily need to define
> CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
> default.

Ah, I see that's a better justification for allowing the define not to
be set.


> diff --git a/common/env_mmc.c b/common/env_mmc.c

>  __weak int mmc_get_env_devno(void)
>  {
> +#ifdef CONFIG_SYS_MMC_ENV_DEV
>  	return CONFIG_SYS_MMC_ENV_DEV;
> +#endif

Same comment here; need a #else here, and #endif below.
> +	return 0;
> +}

Re: Hector's comments: Perhaps the following in
include/config_fallbacks.h would be appropriate?

#ifndef CONFIG_SYS_MMC_ENV_DEV
#define CONFIG_SYS_MMC_ENV_DEV 0
#endif

?
Hector Palacios Jan. 16, 2014, 6:13 p.m. UTC | #3
Hi Stephen,

On 01/15/2014 06:40 PM, Stephen Warren wrote:
> On 01/15/2014 03:53 AM, Hector Palacios wrote:
>> Since function mmc_get_env_devno is __weak and can be overridden by
>> board code, boards do not necessarily need to define
>> CONFIG_SYS_MMC_ENV_DEV. If the constant is not defined, return 0 by
>> default.
>
> Ah, I see that's a better justification for allowing the define not to
> be set.
>
>
>> diff --git a/common/env_mmc.c b/common/env_mmc.c
>
>>   __weak int mmc_get_env_devno(void)
>>   {
>> +#ifdef CONFIG_SYS_MMC_ENV_DEV
>>   	return CONFIG_SYS_MMC_ENV_DEV;
>> +#endif
>
> Same comment here; need a #else here, and #endif below.
>> +	return 0;
>> +}
>
> Re: Hector's comments: Perhaps the following in
> include/config_fallbacks.h would be appropriate?
>
> #ifndef CONFIG_SYS_MMC_ENV_DEV
> #define CONFIG_SYS_MMC_ENV_DEV 0
> #endif

This define is only used in common/env_mmc.c so I think the default definition should 
stay on that file, rather than in include/config_fallbacks.h.

Best regards,
--
Hector Palacios
diff mbox

Patch

diff --git a/common/env_mmc.c b/common/env_mmc.c
index d569b070e005..bd3bc1d50b15 100644
--- a/common/env_mmc.c
+++ b/common/env_mmc.c
@@ -63,7 +63,11 @@  __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
 
 __weak int mmc_get_env_devno(void)
 {
+#ifdef CONFIG_SYS_MMC_ENV_DEV
 	return CONFIG_SYS_MMC_ENV_DEV;
+#endif
+	return 0;
+}
 
 __weak int mmc_get_env_partno(void)
 {