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

Wolfgang Denk
2010-04-14T15:31:47Z

Dear Anatolij Gustschin,

In message <1271254909-20398-2-git-send-email-agust@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