From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Mon, 15 Sep 2008 14:33:46 +0200
Subject: [U-Boot] [PATCH] added autodetect of sdram size and nr of banks
	for ixp
In-Reply-To: <4CD35CD1F8085945B597F80EEC8942130192DEC8@exc01.bk.prodrive.nl>
References: <4CD35CD1F8085945B597F80EEC8942130192DEB0@exc01.bk.prodrive.nl>
	<20080913121215.B49EA24927@gemini.denx.de>
	<4CD35CD1F8085945B597F80EEC8942130192DEC8@exc01.bk.prodrive.nl>
Message-ID: <200809151433.46886.sr@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Martijn,

On Monday 15 September 2008, Martijn de Gouw wrote:
> > > +#ifdef CFG_SDR_CONFIG
> > >  	mov	r1, #CFG_SDR_CONFIG
> > > +	mov	r9, #0xff
> > > +#else
> > > +	mov	r1, #0x1d /* 256 MB, two banks of 128 MB */
> > > +	mov	r9, #0
> > > +#endif
> >
> > I don't want such $ifdef's in global code. Why do you thinkthat 2 x
> > 128 MB would be a default configuration for all IXP based boards?
>
> When CFG_SDR_CONFIG is defined, it holds the memory size, which is set
> in the config.h
> When this value is not set, autodetection is assumed.
> Setting up the board as 2 x 128Mb is used for the autodetection.

Understood. Nevertheless the resulting code is quite complex hard to read.

> > > +sdr_init:
>
> /* snip */
>
> > > +sdr_init_done:
> >
> > This whole test makes not much sense to me. I think the code should be
> > changed to use the standard get_ram_size() funciton instead (see
> > common/memsize.c).
>
> get_ram_size will nog set the memory controller to the correct size.
> The ixp can not run C code when memory is not initialized.

Because of the missing C environment (stack etc)? We could probably use a part 
of the QueueManager SRAM for a temorary initial stack, so that we could run C 
code very early. And do the SDRAM initialization and autodetection in C 
instead of assembler (as done for PPC).

If we could run this SDRAM setup code in C, you could implement a weak default 
init function that could be overwritten by a board specific version (in your 
case). This way we wouldn't add more custom stuff into the common/generic 
source files.

What do you think? 

> Maybe it could be used as a replacement for the code in pdnb.c yes.

Yes. This should be replaced.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================