Terry Lv wrote:
diff --git a/common/env_mmc.c b/common/env_mmc.c
+#include <linux/stddef.h> +#include <malloc.h> +#include <mmc.h>
+#if defined(CONFIG_CMD_ENV) && defined(CONFIG_CMD_MMC)
This seems not correct. If not explicitely set, we get a compiler error. Assuming you has taken this check from env_nand.c, this should be:
#if defined(CONFIG_CMD_SAVEENV) && defined(CONFIG_CMD_MMC)
+#define CMD_SAVEENV +#elif defined(CONFIG_ENV_OFFSET_REDUND) +#error Cannot use CONFIG_ENV_OFFSET_REDUND without CONFIG_CMD_ENV & CONFIG_CMD_MMC
Line too long.
+#endif
+#if defined(CONFIG_ENV_SIZE_REDUND) && (CONFIG_ENV_SIZE_REDUND < CONFIG_ENV_SIZE)
Ditto.
+#ifdef CMD_SAVEENV
+inline int write_env(struct mmc *mmc, unsigned long size,
unsigned long offset, const void *buffer)
Line too long.
+{
- uint blk_start = 0, blk_cnt = 0, n = 0;
- blk_start = (offset % 512) ? ((offset / 512) + 1) : (offset / 512);
- blk_cnt = (size % 512) ? ((size / 512) + 1) : (size / 512);
The alignment to block size is repeated here and in the read function. Should not better to set a macro (something like BLOCK_ALIGN) providing the required alignment ?
+int saveenv(void) +{
- struct mmc *mmc = find_mmc_device(0);
Why is the MMC device hard-coded ? At least should be configurable with a CONFIG_ option. There are boards with more than one MMC controller and you constraint to use always the first one.
- blk_start = (offset % 512) ? ((offset / 512) + 1) : (offset / 512);
Already said, a macro is much more readable to perform alignment.
diff --git a/include/environment.h b/include/environment.h
+#if defined(CONFIG_ENV_IS_IN_MMC) +# ifndef CONFIG_ENV_OFFSET +# error "Need to define CONFIG_ENV_OFFSET when using CONFIG_ENV_IS_IN_MMC" +# endif +# ifndef CONFIG_ENV_ADDR +# define CONFIG_ENV_ADDR (CONFIG_ENV_OFFSET) +# endif +# ifndef CONFIG_ENV_OFFSET +# define CONFIG_ENV_OFFSET (CONFIG_ENV_ADDR) +# endif +# ifdef CONFIG_ENV_OFFSET_REDUND +# define CONFIG_SYS_REDUNDAND_ENVIRONMENT +# endif +# ifdef CONFIG_ENV_IS_EMBEDDED +# define ENV_IS_EMBEDDED 1 +# endif +#endif /* CONFIG_ENV_IS_IN_MMC */
You missed Wolfgang's comment. I think also that there is no reason to set offset for the MMC and block numbers makes more sense.
/* Embedded env is only supported for some flash types */ #ifdef CONFIG_ENV_IS_EMBEDDED # if !defined(CONFIG_ENV_IS_IN_FLASH) && \ diff --git a/lib_arm/board.c b/lib_arm/board.c index 5e3d7f6..f846d0d 100644
+#ifdef CONFIG_GENERIC_MMC
- puts ("MMC: ");
- mmc_initialize (gd->bd);
+#endif
diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 765f97a..9b3f84c 100644 --- a/lib_ppc/board.c +++ b/lib_ppc/board.c @@ -776,6 +776,12 @@ void board_init_r (gd_t *id, ulong dest_addr) nand_init(); /* go init the NAND */ #endif
+#ifdef CONFIG_GENERIC_MMC
- WATCHDOG_RESET ();
- puts ("MMC: ");
- mmc_initialize (bd);
+#endif
I am quite confused. You add the initialization only for ARM and PPC. What about the other architectures ?
I tested your patch on mx51evk, environment is correctly read/written on the SD situated on the back.
Regards, Stefano