[U-Boot] [PATCH v3 1/3] mpc5121: determine RAM size using get_ram_size()

Wolfgang Denk wd at denx.de
Wed Apr 14 17:31:47 CEST 2010


Dear Anatolij Gustschin,

In message <1271254909-20398-2-git-send-email-agust at denx.de> you wrote:
> Configure 1GiB address range in DDR LAW and
> determine the RAM size. Fix DDR LAW afterwards.

Why 1 GiB? Where is this linit coming from? It seems pretty artificial
to me?

> -	u32 msize = CONFIG_SYS_DDR_SIZE * 1024 * 1024;
> +	u32 msize = 1024 * 1024 * 1024;

I'd rather see a (#define'd) constant used here, espeaically as the
vlue is used again furhter doewn in the code...

>  	u32 i;
>  
> @@ -148,5 +148,10 @@ long int fixed_sdram(ddr512x_config_t *mddrc_config,
>  	out_be32(&im->mddrc.ddr_time_config0, mddrc_config->ddr_time_config0);
>  	out_be32(&im->mddrc.ddr_sys_config, mddrc_config->ddr_sys_config);
>  
> +	msize = get_ram_size(CONFIG_SYS_DDR_BASE, 0x40000000);

... i. e. here. Using two different notations for the same number
makes the code even hearder to read and understand.

I suggest we use CONFIG_SYS_MAX_RAM_SIZE like we do in so many other
boards, and leave it to the board maintainer to set a usefule default
value.

Best regards,

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
"To IBM, 'open' means there is a modicum  of  interoperability  among
some of their equipment."                            - Harv Masterson


More information about the U-Boot mailing list