Skip to contentSkip to conversation

env_mmc: make board configurable the partition for the environment

PATCH Superseded Hector Palacios 3 participants 6 messages Jan 17, 2014

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 env_mmc: make board configurable the partition for the environment

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

---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)
common/env_mmc.c +16 -12
index 78c2bc7a1f08..570caf63aeae 100644
@@ -64,6 +64,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
 __weak int mmc_get_env_devno(void)
 {
 	return CONFIG_SYS_MMC_ENV_DEV;
+
+#if !defined(CONFIG_SYS_MMC_ENV_PART)
+#define CONFIG_SYS_MMC_ENV_PART 0
+#endif
+
+__weak int mmc_get_env_partno(void)
+{
+	return CONFIG_SYS_MMC_ENV_PART;
 }
 
 int env_init(void)
@@ -77,6 +85,9 @@ int env_init(void)
 
 static int init_mmc_for_env(struct mmc *mmc)
 {
+	int mmc_env_devno = mmc_get_env_devno();
+	int mmc_env_partno = mmc_get_env_partno();
+
 	if (!mmc) {
 		puts("No MMC card found\n");
 		return -1;
@@ -87,30 +98,23 @@ static int init_mmc_for_env(struct mmc *mmc)
 		return -1;
 	}
 
-#ifdef CONFIG_SYS_MMC_ENV_PART
-	if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num) {
-		int mmc_env_devno = mmc_get_env_devno();
-
-		if (mmc_switch_part(mmc_env_devno,
-				    CONFIG_SYS_MMC_ENV_PART)) {
+	if (mmc_env_partno != mmc->part_num) {
+		if (mmc_switch_part(mmc_env_devno, mmc_env_partno)) {
 			puts("MMC partition switch failed\n");
 			return -1;
 		}
 	}
-#endif
 
 	return 0;
 }
 
 static void fini_mmc_for_env(struct mmc *mmc)
 {
-#ifdef CONFIG_SYS_MMC_ENV_PART
 	int mmc_env_devno = mmc_get_env_devno();
+	int mmc_env_partno = mmc_get_env_partno();
 
-	if (CONFIG_SYS_MMC_ENV_PART != mmc->part_num)
-		mmc_switch_part(mmc_env_devno,
-				mmc->part_num);
-#endif
+	if (mmc_env_partno != mmc->part_num)
+		mmc_switch_part(mmc_env_devno, mmc->part_num);
 }
 
 #ifdef CONFIG_CMD_SAVEENV
#1 lore raw

PATCH v2 2/2 env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

Since function mmc_get_env_devno is __weak and can be overridden by board code, boards do not need to mandatory define CONFIG_SYS_MMC_ENV_DEV.
If the constant is not defined, define it to 0 by default.

---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 5 +++++
 1 file changed, 5 insertions(+)
common/env_mmc.c +5 -0
index 570caf63aeae..c9df4c49e2b8 100644
@@ -61,9 +61,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr)
 	return 0;
 }
 
+#if !defined(CONFIG_SYS_MMC_ENV_DEV)
+#define CONFIG_SYS_MMC_ENV_DEV 0
+#endif
+
 __weak int mmc_get_env_devno(void)
 {
 	return CONFIG_SYS_MMC_ENV_DEV;
+}
 
 #if !defined(CONFIG_SYS_MMC_ENV_PART)
 #define CONFIG_SYS_MMC_ENV_PART 0
#2 lore raw

PATCH v2 2/2 env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

Hello Hector,

Quoted 20 lines · Hector Palacios

Since function mmc_get_env_devno is __weak and can be overridden by board code, boards do not need to mandatory define CONFIG_SYS_MMC_ENV_DEV.
If the constant is not defined, define it to 0 by default.

---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 5 +++++
 1 file changed, 5 insertions(+)
common/env_mmc.c +0 -0
index 570caf63aeae..c9df4c49e2b8 100644

common/env_mmc.c __weak int mmc_get_env_addr(struct mmc *mmc, int copy, · line 69

Quoted 9 lines
common/env_mmc.c +4 -0
@@ -61,9 +61,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy,
 u32 *env_addr)
        return 0;
 }
 
+#if !defined(CONFIG_SYS_MMC_ENV_DEV)
+#define CONFIG_SYS_MMC_ENV_DEV 0
+#endif
+
 

Please move this to include/config_fallbacks.h

Quoted 8 lines
common/env_mmc.c +1 -0
 __weak int mmc_get_env_devno(void)
 {
        return CONFIG_SYS_MMC_ENV_DEV;
+}
 

#if !defined(CONFIG_SYS_MMC_ENV_PART)
#define CONFIG_SYS_MMC_ENV_PART 0

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
#3 lore raw

PATCH v2 2/2 env_mmc: default to 0 if CONFIG_SYS_MMC_ENV_DEV not defined

In reply to Hector Palacios
Quoted 6 lines · Hector Palacios

Since function mmc_get_env_devno is __weak and can be overridden by board code, boards do not need to mandatory define CONFIG_SYS_MMC_ENV_DEV.
If the constant is not defined, define it to 0 by default.

common/env_mmc.c __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr) · line 71

Quoted 9 lines
common/env_mmc.c +5 -0
 
+#if !defined(CONFIG_SYS_MMC_ENV_DEV)
+#define CONFIG_SYS_MMC_ENV_DEV 0
+#endif
+
 __weak int mmc_get_env_devno(void)
 {
 	return CONFIG_SYS_MMC_ENV_DEV;
+}

Oh, I guess your fixup got squashed into the wrong patch?

Aside from that issue, and those Otavio raised, the series,

#4 lore raw

PATCH v2 1/2 env_mmc: make board configurable the partition for the environment

In reply to Hector Palacios

Hello Hector,

Quoted 25 lines · Hector Palacios

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

---

Notes:
Changes since v1:
- Use default define if not set

 common/env_mmc.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)
common/env_mmc.c +0 -0
index 78c2bc7a1f08..570caf63aeae 100644

common/env_mmc.c __weak int mmc_get_env_addr(struct mmc *mmc, int copy, · line 72

Quoted 9 lines
common/env_mmc.c +4 -0
@@ -64,6 +64,14 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy,
 u32 *env_addr)
 __weak int mmc_get_env_devno(void)
 {
        return CONFIG_SYS_MMC_ENV_DEV;
+
+#if !defined(CONFIG_SYS_MMC_ENV_PART)
+#define CONFIG_SYS_MMC_ENV_PART 0
+#endif
 

This should be in include/config_fallbacks.h

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
#5 lore raw

PATCH v2 1/2 env_mmc: make board configurable the partition for the environment

In reply to Hector Palacios
Quoted 11 lines · Hector Palacios

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

common/env_mmc.c __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32 *env_addr) · line 68

Quoted 6 lines
common/env_mmc.c +2 -0
 
 __weak int mmc_get_env_devno(void)
 {
 	return CONFIG_SYS_MMC_ENV_DEV;
+
+#if !defined(CONFIG_SYS_MMC_ENV_PART)

Isn't the trailing } for that function missing?

Quoted 7 lines
common/env_mmc.c +6 -0
+#define CONFIG_SYS_MMC_ENV_PART 0
+#endif
+
+__weak int mmc_get_env_partno(void)
+{
+	return CONFIG_SYS_MMC_ENV_PART;
 }