Hello Wolfgang,

On Wed, 14 Apr 2010 17:31:47 +0200
Wolfgang Denk <w...@denx.de> wrote:

> In message <1271254909-20398-2-git-send-email-ag...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to