[U-Boot] [PATCH] ADS5121 Add mem config for current rev4 boards

[Wolfgang Denk]

Dear John Rigby,

In message <1234908276-26381-1-git-send-email-jrigby at freescale.com> you wrote:
> From: Martha Marx <mmarx at silicontkx.com>
> 
> The following configurations are now supported:
> 
> ads5121_config
> 	rev 4.1 boards with Elpida memory
> 
> ads5121_rev4m_config
> 	rev 4 boards with Micron memory

This shall not be done. All possible memory configurations shall be
handled by a single binary image of U-Boot.

> ads5121_rev3_config
> 	rev3 boards which also have Micron memory but have the
> 	older rev silicon

It makes little sense to me to add new config options for old boards
that are EOL and not distributed any more. 

> diff --git a/board/ads5121/ads5121.c b/board/ads5121/ads5121.c
> index 6c40e94..cb607b4 100644
> --- a/board/ads5121/ads5121.c
> +++ b/board/ads5121/ads5121.c
> @@ -182,29 +182,50 @@ long int fixed_sdram (void)
>  
>  	/* Initialize DDR */
>  	for (i = 0; i < 10; i++)
> -		im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> -
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_PCHG_ALL;
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_RFSH;
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_RFSH;
> -	im->mddrc.ddr_command = CONFIG_SYS_MICRON_NOP;
> +		im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> +
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_PCHG_ALL;
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_RFSH;
> +	im->mddrc.ddr_command = CONFIG_SYS_DDR_NOP;

While performing such a global rename, please let's get rid of the
CONFIG_SYS_ names. These are NOT options that can be changed by the
user in the board config file, so they should not look like such.

> diff --git a/include/configs/ads5121.h b/include/configs/ads5121.h
> index 8fda3f2..2e6ceef 100644
> --- a/include/configs/ads5121.h
> +++ b/include/configs/ads5121.h
...
> +#elif defined(CONFIG_ADS5121_REV3) || \
> +      defined(CONFIG_ADS5121_REV4M)
>  #define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA804A00
>  #define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA804A00
>  #define CONFIG_SYS_MDDRC_TIME_CFG1	 0x68EC1168
>  #define CONFIG_SYS_MDDRC_TIME_CFG2	 0x34310864
> +#else
> +#define CONFIG_SYS_MDDRC_SYS_CFG	 0xFA802b40
> +#define CONFIG_SYS_MDDRC_SYS_CFG_RUN	 0xEA802b40
> +#define CONFIG_SYS_MDDRC_TIME_CFG1	 0x690e1189
> +#define CONFIG_SYS_MDDRC_TIME_CFG2	 0x35410864
>  #endif

If I understand correctly, these are the only  real  changes  to  the
memory  init sequence  right? This is virtually invisible in the huge
number of variable renames. You should split this into  two  separate
patches,  one  doing  the  renames  only, and one adding the real new
configuration.

But as mentioned above - the key point is that we  need  to  come  up
with a solution to support all used memory types with a single binary
image.


I think such a patch cannot go into  mainline  (even  if  the  formal
issues  mentioned  above  were  fixed). Can we please have a solution
that avoilds an explosin of differnt, incompatible binary images  and
config  options  just  for  minor  board modifications like different
memory types?

Thanks.

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A committee is a group that keeps the minutes and loses hours.
                                                      -- Milton Berle