[U-Boot] [PATCH] Save environment data to mmc.

[Lv Terry-R65388]

Hi Babic,

	Pls check my comments with keyword [Ty].

	Thanks for reviewing the patch.

	Thanks~~

Yours
Terry

-----Original Message-----
From: Stefano Babic [mailto:sbabic at denx.de] 
Sent: 2010年4月29日 22:34
To: Lv Terry-R65388
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] Save environment data to mmc.

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 at 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.
[Ty]: OK, I will do that, thanks, :-)

> 
> 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.
[Ty]: I have moved mmc initialization in boards that uses generic mmc. I will add the comment, thanks.

> 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)	&& \