[U-Boot] [PATCH] Add new Elpida memory configuration for ads5121

[Wolfgang Denk]

Dear Martha,

In message <12350848482174-git-send-email-mmarx at silicontkx.com> you wrote:
> Rev 3 and earlier stay with Micron memory settings.  Rev 4 and later
> will default to Elpida settings. CONFIG_ELPIDA forces these setting
> and will run slightly more slowly.  CONFIG_MICRON will force Micron
> settings.  If Micron settings are used, if a memory test fails, it
> will revert to the slower Elpida settings
> 
> Signed-off-by: Martha Marx <mmarx at silicontkx.com>
> ---
>  board/ads5121/ads5121.c   |  154 ++++++++++++++++++++++++++++++++++++---------
>  include/configs/ads5121.h |   41 +++++++-----
>  2 files changed, 150 insertions(+), 45 deletions(-)
> 
> diff --git a/board/ads5121/ads5121.c b/board/ads5121/ads5121.c
> index 6c40e94..deb4a74 100644
> --- a/board/ads5121/ads5121.c
> +++ b/board/ads5121/ads5121.c
> @@ -120,13 +120,53 @@ phys_size_t initdram (int board_type)
>  	return msize;
>  }
>  
> +int simple_mem_check()
> +{
> +	volatile u32 val;
> +	volatile u32 *addr;
> +	int i, j;
> +	u32 save[32];
> +
> +	/*
> +	 * Very very simple non-destructive check for operational RAM.
> +	 */
> +	j = 0;
> +	for (i = CONFIG_SYS_DDR_SIZE / sizeof (long); i > 0; i >>= 1) {
> +		addr = (volatile u32 *) CONFIG_SYS_DDR_BASE + i;
> +		save[j++] = *addr;
> +		*addr = ~i;
> +	}
> +
> +	addr = (volatile u32 *) CONFIG_SYS_DDR_BASE;
> +	save[j] = *addr;
> +	*addr = 0xdeadbeef;
> +	val = *addr;
> +	*addr = save[j];
> +
> +	if (val != 0xdeadbeef)
> +		return (0);
> +
> +	for (i = 1; i <= CONFIG_SYS_DDR_SIZE / sizeof (long); i <<= 1) {
> +		addr = (volatile u32 *) CONFIG_SYS_DDR_BASE + i;
> +		val = *addr;
> +		*addr = save[--j];
> +		if (val != ~i)
> +			return (0);
> +	}
> +	return(1);
> +}

We already have that code, just a bit more general and
non-destructive, as function get_ram_size() in "common/memsize.c".

Please do not reinvent the wheel -  use  the  existing  code  instead
(ideally  this should also be used to auto-adjust the RAM size on the
boards).


>  /*
>   * fixed sdram init -- the board doesn't use memory modules that have serial presence
>   * detect or similar mechanism for discovery of the DRAM settings
>   */
> +

Please do not add random white space changes.

>  long int fixed_sdram (void)
>  {
> +	u32 try_elpida;
>  	volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR;
> +	ushort brd_rev = *(vu_short *) (CONFIG_SYS_CPLD_BASE + 0x00);
> +
>  	u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024;
>  	u32 msize_log2 = __ilog2 (msize);
>  	u32 i;
> @@ -174,41 +214,97 @@ long int fixed_sdram (void)
>  	im->mddrc.lut_table4_alternate_upper = CONFIG_SYS_MDDRCGRP_LUT4_AU;
>  	im->mddrc.lut_table4_alternate_lower = CONFIG_SYS_MDDRCGRP_LUT4_AL;
>  
> +#ifdef CONFIG_MICRON
> +/* forces Micron settings - on rev 4 boards will retry w/Elpida if necessary */
> +	try_elpida = 0;
> +#elif defined(CONFIG_ELPIDA)
> +/* forces Elpida settings w/slower CAS - works w/ both Micron and Elpida rev4 */
> +	try_elpida = 1;
> +#else
> +	if (brd_rev >= 0x0400)
> +		try_elpida = 1;
> +	else
> +		try_elpida = 0;
> +#endif
> +retry:

Please keep the code simple. As discussed by PM before,  please  drop
the  config  stuff  and support for old boards that long have reached
EOL.

Old boards can be used with the existing U-Boot; here in mainline we
should support board rev. 4.x and higher only.

> +	debug("Use %s Memory settings\n\t", try_elpida ? "Elpida" : "Micron");
> +
>  	/* Initialize MDDRC */
> -	im->mddrc.ddr_sys_config = CONFIG_SYS_MDDRC_SYS_CFG;
> -	im->mddrc.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0;
> -	im->mddrc.ddr_time_config1 = CONFIG_SYS_MDDRC_TIME_CFG1;
> -	im->mddrc.ddr_time_config2 = CONFIG_SYS_MDDRC_TIME_CFG2;
> +	if (try_elpida) {
> +		im->mddrc.ddr_sys_config = CONFIG_SYS_MDDRC_SYS_CFG_ELPIDA;
> +		im->mddrc.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0;
> +		im->mddrc.ddr_time_config1 = CONFIG_SYS_MDDRC_TIME_CFG1_ELPIDA;
> +		im->mddrc.ddr_time_config2 = CONFIG_SYS_MDDRC_TIME_CFG2_ELPIDA;
> +	} else {
> +		im->mddrc.ddr_sys_config = CONFIG_SYS_MDDRC_SYS_CFG_MICRON;
> +		im->mddrc.ddr_time_config0 = CONFIG_SYS_MDDRC_TIME_CFG0;
> +		im->mddrc.ddr_time_config1 = CONFIG_SYS_MDDRC_TIME_CFG1_MICRON;
> +		im->mddrc.ddr_time_config2 = CONFIG_SYS_MDDRC_TIME_CFG2_MICRON;
> +	}

Drop this. ALways use the (more relaxed?) Elpida settings.

> +		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;

As mentioned a couple times before, we should start with  an  initial
patch that performas a global rename to get rid of the (inappropiate)
CONFIG_SYS_ names here.

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
As usual, this being a 1.3.x release, I haven't  even  compiled  this
kernel yet. So if it works, you should be doubly impressed.
      - Linus Torvalds in <199506181536.SAA10638 at keos.cs.Helsinki.FI>