From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatolij Gustschin Date: Thu, 15 Apr 2010 14:20:25 +0200 Subject: [U-Boot] [PATCH v3 1/3] mpc5121: determine RAM size using get_ram_size() In-Reply-To: <20100414153147.0DD22E85054@gemini.denx.de> References: <20100321192306.BFE104C022@gemini.denx.de> <1271254909-20398-1-git-send-email-agust@denx.de> <1271254909-20398-2-git-send-email-agust@denx.de> <20100414153147.0DD22E85054@gemini.denx.de> Message-ID: <20100415142025.5c75e8eb@wker> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Wolfgang, On Wed, 14 Apr 2010 17:31:47 +0200 Wolfgang Denk wrote: > 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? It is the base address for NAND which is 0x40000000 (1GiB) for all mpc512x boards in the U-Boot tree. But now I see that it is also wrong as some boards use 0x30000000 for SRAM base. The upper limit is 2GiB. > > - 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... Will fix. > > 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. Ok, I will submit next patch version with this fix. Thanks! Best regards, Anatolij