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

[Wolfgang Denk]

Dear "Martijn de Gouw",

In message <4CD35CD1F8085945B597F80EEC8942130192DEB0 at 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 at 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