On Fri, Oct 8, 2010 at 6:39 AM, Steve Sakoman <[email protected]> wrote: > On Fri, Oct 8, 2010 at 1:03 AM, Stefano Babic <[email protected]> wrote: >> mmc_initialize is not called at the startup if the >> relocation takes place and the environment is stored >> into a MMC card. >> >> Signed-off-by: Stefano Babic <[email protected]> >> --- >> arch/arm/lib/board.c | 10 +++++----- >> common/env_mmc.c | 11 +++++------ >> 2 files changed, 10 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c >> index 5f2dfd0..704ddcb 100644 >> --- a/arch/arm/lib/board.c >> +++ b/arch/arm/lib/board.c >> @@ -775,6 +775,11 @@ void board_init_r (gd_t *id, ulong dest_addr) >> nand_init(); /* go init the NAND */ >> #endif >> >> +#ifdef CONFIG_GENERIC_MMC >> + puts ("MMC: "); >> + mmc_initialize (bd); >> +#endif >> + >> #if defined(CONFIG_CMD_ONENAND) >> onenand_init(); >> #endif >> @@ -854,11 +859,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 > > I think it might make more sense to put the MMC ifdef after the > onenand code so that it doesn't split the nand/onenand grouping. > Otherwise it matches what I did. > >> diff --git a/common/env_mmc.c b/common/env_mmc.c >> index 14203b6..cb7c448 100644 >> --- a/common/env_mmc.c >> +++ b/common/env_mmc.c >> @@ -130,24 +130,23 @@ void env_relocate_spec(void) >> { >> #if !defined(ENV_IS_EMBEDDED) >> struct mmc *mmc = find_mmc_device(CONFIG_SYS_MMC_ENV_DEV); >> + char buf[CONFIG_ENV_SIZE]; >> >> if (init_mmc_for_env(mmc)) >> return; >> >> - if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, env_ptr)) >> - return use_default(); >> - >> - if (crc32(0, env_ptr->data, ENV_SIZE) != env_ptr->crc) >> + if (read_env(mmc, CONFIG_ENV_SIZE, CONFIG_ENV_OFFSET, buf)) >> return use_default(); > > This is a void function and shouldn't have a return value. I fixed > this in my version. > >> >> gd->env_valid = 1; > > I removed this in my version, probably an error to do that though :-) > >> + >> + env_import(buf, 1); >> #endif >> } >> >> #if !defined(ENV_IS_EMBEDDED) >> static void use_default() >> { >> - puts ("*** Warning - bad CRC or MMC, using default environment\n\n"); >> - set_default_env(); >> + set_default_env("*** Warning - bad CRC or MMC, using default >> environment\n\n"); > > I previously submitted a patch to fix this and Wolfgang sent an email > saying that it had been applied. > >> } >> #endif >> -- >> 1.6.3.3 >> >> > > I'll revise my patch to add the missing gd->env_valid = 1; and re-test.
Hmm . . . looking at env_nand.c I see that it doesn't do gd->env_valid = 1 either, which is probably why I removed it last night. Any idea whether it is actually required? Seems to work fine without it! Steve _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
