[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Martijn de Gouw]

Add autodetection of nr of banks and total sdram size
tested on ixpdg425 and pdnb3 board.

Signed-off-by: martijn de gouw <martijn@martijn.degouw.net>
---
 board/prodrive/pdnb3/pdnb3.c |   31 +++++++++++++-
 cpu/ixp/start.S              |   95
++++++++++++++++++++++++++++++++++++++++++
 include/configs/pdnb3.h      |    2 -
 3 files changed, 125 insertions(+), 3 deletions(-)

diff --git a/board/prodrive/pdnb3/pdnb3.c b/board/prodrive/pdnb3/pdnb3.c
index 3445a3a..a049afe 100644
--- a/board/prodrive/pdnb3/pdnb3.c
+++ b/board/prodrive/pdnb3/pdnb3.c
@@ -120,8 +120,37 @@ int checkboard(void)
 
 int dram_init(void)
 {
+	volatile unsigned long *sdr_config = (unsigned long
*)IXP425_SDR_CONFIG;
+
 	gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
-	gd->bd->bi_dram[0].size  = PHYS_SDRAM_1_SIZE;
+
+	switch ((*sdr_config & ~(0x18))) {
+	case 0x20:
+		gd->bd->bi_dram[0].size  = (8 << 20);
+		break;
+	case 0x21:
+	case 0x22:
+		gd->bd->bi_dram[0].size  = (16 << 20);
+		break;
+	case 0x23:
+	case 0x00:
+		gd->bd->bi_dram[0].size  = (32 << 20);
+		break;
+	case 0x01:
+	case 0x02:
+		gd->bd->bi_dram[0].size  = (64 << 20);
+		break;
+	case 0x03:
+	case 0x04:
+		gd->bd->bi_dram[0].size  = (128 << 20);
+		break;
+	case 0x09:
+		gd->bd->bi_dram[0].size  = (256 << 20);
+		break;
+	default:
+		gd->bd->bi_dram[0].size  = 0;
+		break;
+	}
 
 	return (0);
 }
diff --git a/cpu/ixp/start.S b/cpu/ixp/start.S
index d4c8e33..f1123de 100644
--- a/cpu/ixp/start.S
+++ b/cpu/ixp/start.S
@@ -165,7 +165,16 @@ reset:
 	orr     r1, r1, #0x80000000
 	str     r1, [r2]
 #endif
+
+#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
+
+sdr_init:
 	ldr     r2, =IXP425_SDR_CONFIG
 	str     r1, [r2]
 
@@ -208,6 +217,92 @@ reset:
 	str	r1, [r4]
 	DELAY_FOR 0x4000, r0
 
+	cmp	r9, #0xff
+	beq	sdr_init_done
+
+sdr_test_cs1:
+	/* test if there are chips connected to bank 1 */
+	mov	r9, #0x18 /* holds new SDR_CONFIG value */
+	mov	r1, #(256 << 20)
+	orr	r1, r1, #(128 << 20)
+	orr	r1, r1, #(1 << 20)
+	mov	r2, #0
+	str	r2, [r1]
+	ldr	r3, [r1]
+	cmp	r2, r3
+	bne	sdr_test_cs0
+	orr	r9, r9, #(0x01)
+
+sdr_test_cs0:
+	/* test chip size on bank 0 */
+	/* r1: holds test address */
+	/* r3: holds test value */
+	/* r5: holds mirror address */
+
+	/* clear @ 1MB */
+	mov	r1, #(1 << 20)
+	orr	r1, r1, #(256 << 20)
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* test value and test offset */
+	mov	r3, #0xbe
+	orr	r3, r3, #(0xba << 8)
+	orr	r3, r3, #(0xfe << 16)
+	orr	r3, r3, #(0xca << 24)
+
+	mov	r4, #(1 << 20)
+	orr	r4, r4, #(256 << 20)
+	b	sdr_try_32Mbit
+
+sdr_try_32Mbit:
+	orr	r5, r4, #(4 << 20)
+	str	r3, [r5]
+	ldr	r6, [r1]
+	cmp	r6, r3
+	bne	sdr_try_64Mbit
+	add	r9, r9, #0x00
+	orr	r9, r9, #(1 << 5)
+	b	sdr_reinit
+
+sdr_try_64Mbit:
+	orr	r5, r4, #(4 << 20)
+	str	r3, [r5]
+	ldr	r6, [r1]
+	cmp	r6, r3
+	bne	sdr_try_128Mbit
+	add	r9, r9, #0x02
+	orr	r9, r9, #(1 << 5)
+	b	sdr_reinit
+
+sdr_try_128Mbit:
+	orr	r5, r4, #(8 << 20)
+	str	r3, [r5]
+	ldr	r6, [r1]
+	cmp	r6, r3
+	bne	sdr_try_256Mbit
+	add	r9, r9, #0x00
+	b	sdr_reinit
+
+sdr_try_256Mbit:
+	orr	r5, r4, #(16 << 20)
+	str	r3, [r5]
+	ldr	r6, [r1]
+	cmp	r6, r3
+	bne	sdr_try_512Mbit
+	add	r9, r9, #0x02
+	b	sdr_reinit
+
+sdr_try_512Mbit:
+	add	r9, r9, #0x04
+	b	sdr_reinit
+
+sdr_reinit:
+	mov	r1, r9
+	mov	r9, #0xff
+	b	sdr_init
+
+sdr_init_done:
 	/* copy */
 	mov     r0, #0
 	mov     r4, r0
diff --git a/include/configs/pdnb3.h b/include/configs/pdnb3.h
index 889207a..ea15052 100644
--- a/include/configs/pdnb3.h
+++ b/include/configs/pdnb3.h
@@ -186,7 +186,6 @@
  */
 #define CONFIG_NR_DRAM_BANKS    1          /* we have 1 bank of DRAM */
 #define PHYS_SDRAM_1            0x00000000 /* SDRAM Bank #1 */
-#define PHYS_SDRAM_1_SIZE       0x02000000 /* 32 MB */
 
 #define CFG_FLASH_BASE          0x50000000
 #define CFG_MONITOR_BASE	CFG_FLASH_BASE
@@ -209,7 +208,6 @@
 /*
  * SDRAM settings
  */
-#define CFG_SDR_CONFIG		0x18
 #define CFG_SDR_MODE_CONFIG	0x1
 #define CFG_SDRAM_REFRESH_CNT	0x81a
 
-- 
1.5.5

Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Wolfgang Denk]

Dear "Martijn de Gouw",

In message <4CD35CD1F8085945B597F80EEC8942130192DEB0@exc01.bk.prodrive.nl> you wrote:
> Add autodetection of nr of banks and total sdram size
> tested on ixpdg425 and pdnb3 board.
> 
> Signed-off-by: martijn de gouw <martijn@martijn.degouw.net>
> ---
>  board/prodrive/pdnb3/pdnb3.c |   31 +++++++++++++-
>  cpu/ixp/start.S              |   95
> ++++++++++++++++++++++++++++++++++++++++++
>  include/configs/pdnb3.h      |    2 -
>  3 files changed, 125 insertions(+), 3 deletions(-)
> 
> diff --git a/board/prodrive/pdnb3/pdnb3.c b/board/prodrive/pdnb3/pdnb3.c
> index 3445a3a..a049afe 100644
> --- a/board/prodrive/pdnb3/pdnb3.c
> +++ b/board/prodrive/pdnb3/pdnb3.c
> @@ -120,8 +120,37 @@ int checkboard(void)
>  
>  int dram_init(void)
>  {
> +	volatile unsigned long *sdr_config = (unsigned long
> *)IXP425_SDR_CONFIG;
^^^^^^^^^^^^^^^^^^^^^^

Your patch has been corrupted by your mailer which wrapped long lines.
Please fix your mailer setup and resubmit.

> diff --git a/cpu/ixp/start.S b/cpu/ixp/start.S
> index d4c8e33..f1123de 100644
> --- a/cpu/ixp/start.S
> +++ b/cpu/ixp/start.S
> @@ -165,7 +165,16 @@ reset:
>  	orr     r1, r1, #0x80000000
>  	str     r1, [r2]
>  #endif
> +
> +#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?

> +sdr_init:
>  	ldr     r2, =IXP425_SDR_CONFIG
>  	str     r1, [r2]
>  
> @@ -208,6 +217,92 @@ reset:
>  	str	r1, [r4]
>  	DELAY_FOR 0x4000, r0
>  
> +	cmp	r9, #0xff
> +	beq	sdr_init_done
> +
> +sdr_test_cs1:
> +	/* test if there are chips connected to bank 1 */
> +	mov	r9, #0x18 /* holds new SDR_CONFIG value */
> +	mov	r1, #(256 << 20)
> +	orr	r1, r1, #(128 << 20)
> +	orr	r1, r1, #(1 << 20)
> +	mov	r2, #0
> +	str	r2, [r1]
> +	ldr	r3, [r1]
> +	cmp	r2, r3
> +	bne	sdr_test_cs0
> +	orr	r9, r9, #(0x01)
> +
> +sdr_test_cs0:
> +	/* test chip size on bank 0 */
> +	/* r1: holds test address */
> +	/* r3: holds test value */
> +	/* r5: holds mirror address */
> +
> +	/* clear @ 1MB */
> +	mov	r1, #(1 << 20)
> +	orr	r1, r1, #(256 << 20)
> +	mov	r2, #0
> +	str	r2, [r1]
> +
> +	/* test value and test offset */
> +	mov	r3, #0xbe
> +	orr	r3, r3, #(0xba << 8)
> +	orr	r3, r3, #(0xfe << 16)
> +	orr	r3, r3, #(0xca << 24)
> +
> +	mov	r4, #(1 << 20)
> +	orr	r4, r4, #(256 << 20)
> +	b	sdr_try_32Mbit
> +
> +sdr_try_32Mbit:
> +	orr	r5, r4, #(4 << 20)
> +	str	r3, [r5]
> +	ldr	r6, [r1]
> +	cmp	r6, r3
> +	bne	sdr_try_64Mbit
> +	add	r9, r9, #0x00
> +	orr	r9, r9, #(1 << 5)
> +	b	sdr_reinit
> +
> +sdr_try_64Mbit:
> +	orr	r5, r4, #(4 << 20)
> +	str	r3, [r5]
> +	ldr	r6, [r1]
> +	cmp	r6, r3
> +	bne	sdr_try_128Mbit
> +	add	r9, r9, #0x02
> +	orr	r9, r9, #(1 << 5)
> +	b	sdr_reinit
> +
> +sdr_try_128Mbit:
> +	orr	r5, r4, #(8 << 20)
> +	str	r3, [r5]
> +	ldr	r6, [r1]
> +	cmp	r6, r3
> +	bne	sdr_try_256Mbit
> +	add	r9, r9, #0x00
> +	b	sdr_reinit
> +
> +sdr_try_256Mbit:
> +	orr	r5, r4, #(16 << 20)
> +	str	r3, [r5]
> +	ldr	r6, [r1]
> +	cmp	r6, r3
> +	bne	sdr_try_512Mbit
> +	add	r9, r9, #0x02
> +	b	sdr_reinit
> +
> +sdr_try_512Mbit:
> +	add	r9, r9, #0x04
> +	b	sdr_reinit
> +
> +sdr_reinit:
> +	mov	r1, r9
> +	mov	r9, #0xff
> +	b	sdr_init
> +
> +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).

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
"Tell the truth and run."                          - Yugoslav proverb

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Martijn de Gouw]

Hi Wolfgang

> > +
> > +#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. 

> 
> > +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.

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

Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Wolfgang Denk]

Dear "Martijn de Gouw",

In message <4CD35CD1F8085945B597F80EEC8942130192DEC8@exc01.bk.prodrive.nl> you 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. 

You could set CFG_SDR_CONFIG to that value in your board config file
and avoid the #ifdef ?

> get_ram_size will nog set the memory controller to the correct size.

No, it does not set the size, it auto-detects the size  that  can  be
seen for a specific configuration of the memory controller.

> The ixp can not run C code when memory is not initialized.

Has it not enough on-chip  memory  or  caches  for  that  like  other
processors?

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
Anarchy may not be the best form of government, but it's better  than
no government at all.

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Stefan Roese]

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
=====================================================================

[U-Boot] [PATCH] added autodetect of sdram size and nr of banks for ixp

[Martijn de Gouw]

Hi 

> > > > +#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.

Not complex, just hard to read because of assembly :)

> 
> > > > +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).

This would involve a large change in the code, compared to this patch.
Even the relocation code is in assembler right now (because of this
reason)
 
> 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 

Yes, ofcource, in C life is much simpler.

> case). This way we wouldn't add more custom stuff into the 
> common/generic 
> source files.
> 
> What do you think? 

I think that this code could be used as generic code, it is not board
specific code.
I could change de ifdef structure to enable/disable the autodetection
part, instead of using the CFG_SDR_CONFIG define, but a ifdef would
remain.

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

Ok, makes sense.

Regards, Martijn de Gouw

Disclaimer: The information contained in this email, including any attachments is 
confidential and is for the sole use of the intended recipient(s). Any unauthorized 
review, use, disclosure or distribution is prohibited. If you are not the intended 
recipient, please notify the sender immediately by replying to this message and 
destroy all copies of this message and any attachments.