Terry Lv wrote:
This patch is to save environment data to mmc card. It uses interfaces defined in generic mmc.
Hi Terry,
Signed-off-by: Terry Lv r65388@freescale.com
arch/arm/lib/board.c | 10 ++-- arch/powerpc/lib/board.c | 12 ++-- common/Makefile | 1 + common/cmd_nvedit.c | 1 + common/env_mmc.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 167 insertions(+), 11 deletions(-) create mode 100644 common/env_mmc.c
Could you set a version of your patch (something like [PATCH V*] in the subject, so it is easier to track changes ? This is the third version, but it is difficult to get it without searching in archive.
diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c index f5660a9..f62e0eb 100644 --- a/arch/arm/lib/board.c +++ b/arch/arm/lib/board.c @@ -347,6 +347,11 @@ void start_armboot (void) dataflash_print_info(); #endif
+#ifdef CONFIG_GENERIC_MMC
- puts ("MMC: ");
- mmc_initialize (gd->bd);
+#endif
- /* initialize environment */ env_relocate ();
@@ -419,11 +424,6 @@ extern void davinci_eth_set_mac_addr (const u_int8_t *addr); board_late_init (); #endif
-#ifdef CONFIG_GENERIC_MMC
- puts ("MMC: ");
- mmc_initialize (gd->bd);
-#endif
#ifdef CONFIG_BITBANGMII bb_miiphy_init(); #endif
Because it is required to move the initialization of the mmc before env_relocate(), we need probably to advise that there are some consequences. If someone implements the mmc support in board_late_init() (setting a pin multiplexer or something like that, for example), it does not work. I think we should at least write a comment to advise that the mmc/sd controller should work after board_init() is called. However, after a quick check in the arm boards, I have not found a board that is initializing the mmc controller in board_late_init(). Not sure for powerpc.
diff --git a/common/cmd_nvedit.c b/common/cmd_nvedit.c index eb89e9e..78f75fb 100644 --- a/common/cmd_nvedit.c +++ b/common/cmd_nvedit.c @@ -59,6 +59,7 @@ DECLARE_GLOBAL_DATA_PTR; !defined(CONFIG_ENV_IS_IN_FLASH) && \ !defined(CONFIG_ENV_IS_IN_DATAFLASH) && \ !defined(CONFIG_ENV_IS_IN_MG_DISK) && \
- !defined(CONFIG_ENV_IS_IN_MMC) && \ !defined(CONFIG_ENV_IS_IN_NAND) && \ !defined(CONFIG_ENV_IS_IN_NVRAM) && \ !defined(CONFIG_ENV_IS_IN_ONENAND) && \
From the first version you remove the changes in the error string:
# error Define one of CONFIG_ENV_IS_IN_{EEPROM|FLASH|DATAFLASH|ONENAND|\ -SPI_FLASH|MG_DISK|NVRAM|NOWHERE} +SPI_FLASH|MG_DISK|NVRAM|MMC|NOWHERE}
I know it is only an error message, but the change seems correct. I have not found a comment against it ;)
+#else /* ! ENV_IS_EMBEDDED */ +env_t *env_ptr; +#endif /* ENV_IS_EMBEDDED */
You missed Andy's comment. You have to initialize env_ptr.
- if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr))
return use_default();
This is still broken, as found by Andy. I cannot find a line where env_ptr is initialized and then you pass the pointer to the mmc read function.
Best regards, Stefano