[U-Boot] [PATCH v5] arm: ep9315: Return back Cirrus Logic EDB9315A board support

[Albert ARIBAUD]

Hi Sergey,

The patch does not apply cleanly on current u-boot-arm/master.

Also, see my comments below, a continuation from two points I raised
and you answered in V4.

On Mon, 21 Apr 2014 08:39:59 +0000, Sergey Kostanbev
<sergey.kostanbaev at gmail.com> wrote:

> diff --git a/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S b/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> index bf2fa2a..3ac0f88 100644
> --- a/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> +++ b/arch/arm/cpu/arm920t/ep93xx/lowlevel_init.S
> @@ -1,49 +1,458 @@
>  /*
>   * Low-level initialization for EP93xx
>   *
>   * Copyright (C) 2009 Matthias Kaehlcke <matthias at kaehlcke.net>
> + * Copyright (C) 2013
> + * Sergey Kostanabev <sergey.kostanbaev <at> fairwaves.ru>
>   *
>   * Copyright (C) 2006 Dominic Rath <Dominic.Rath at gmx.de>
> + * Copyright (C) 2006 Cirrus Logic Inc.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
>   *
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
>  
> -#include <version.h>
> -#include <asm/arch/ep93xx.h>
> +#include <config.h>
> +#include <asm/arch-ep93xx/ep93xx.h>
> +
> +/*
> +/* Configure the SDRAM based on the supplied settings.
> + *
> + * Input:	r0 - SDRAM DEVCFG register
> + *		r2 - configuration for SDRAM chips
> + * Output:	none
> + * Modifies:	r3, r4
> + */
> +ep93xx_sdram_config:
> +	/* Program the SDRAM device configuration register. */
> +	ldr	r3, =SDRAM_BASE
> +#ifdef CONFIG_EDB93XX_SDCS0
> +	str	r0, [r3, #SDRAM_OFF_DEVCFG0]
> +#endif
> +#ifdef CONFIG_EDB93XX_SDCS1
> +	str	r0, [r3, #SDRAM_OFF_DEVCFG1]
> +#endif
> +#ifdef CONFIG_EDB93XX_SDCS2
> +	str	r0, [r3, #SDRAM_OFF_DEVCFG2]
> +#endif
> +#ifdef CONFIG_EDB93XX_SDCS3
> +	str	r0, [r3, #SDRAM_OFF_DEVCFG3]
> +#endif
> +
> +	/* Set the Initialize and MRS bits (issue continuous NOP commands
> +	 * (INIT & MRS set))
> +	 */
> +	ldr	r4, =(EP93XX_SDRAMCTRL_GLOBALCFG_INIT | \
> +			EP93XX_SDRAMCTRL_GLOBALCFG_MRS | \
> +			EP93XX_SDRAMCTRL_GLOBALCFG_CKE)
> +	str	r4, [r3, #SDRAM_OFF_GLCONFIG]
> +
> +	/* Delay for 200us. */
> +	mov	r4, #0x3000
> +delay1:
> +	subs	r4, r4, #1
> +	bne	delay1
> +
> +	/* Clear the MRS bit to issue a precharge all. */
> +	ldr	r4, =(EP93XX_SDRAMCTRL_GLOBALCFG_INIT | \
> +			EP93XX_SDRAMCTRL_GLOBALCFG_CKE)
> +	str	r4, [r3, #SDRAM_OFF_GLCONFIG]
> +
> +	/* Temporarily set the refresh timer to 0x10. Make it really low so
> +	 * that refresh cycles are generated.
> +	 */
> +	ldr	r4, =0x10
> +	str	r4, [r3, #SDRAM_OFF_REFRSHTIMR]
> +
> +	/* Delay for at least 80 SDRAM clock cycles. */
> +	mov	r4, #80
> +delay2:
> +	subs	r4, r4, #1
> +	bne	delay2
> +
> +	/* Set the refresh timer to the fastest required for any device
> +	 * that might be used. Set 9.6 ms refresh time.
> +	 */
> +	ldr	r4, =0x01e0
> +	str	r4, [r3, #SDRAM_OFF_REFRSHTIMR]
> +
> +	/* Select mode register update mode. */
> +	ldr	r4, =(EP93XX_SDRAMCTRL_GLOBALCFG_CKE | \
> +			EP93XX_SDRAMCTRL_GLOBALCFG_MRS)
> +	str	r4, [r3, #SDRAM_OFF_GLCONFIG]
> +
> +	/* Program the mode register on the SDRAM by performing fake read */
> +	ldr	r4, [r2]
> +
> +	/* Select normal operating mode. */
> +	ldr	r4, =EP93XX_SDRAMCTRL_GLOBALCFG_CKE
> +	str	r4, [r3, #SDRAM_OFF_GLCONFIG]
> +
> +	/* Return to the caller. */
> +	mov	pc, lr
> +
> +/*
> + * Test to see if the SDRAM has been configured in a usable mode.
> + *
> + * Input:	r0 - Test address of SDRAM
> + * Output:	r0 - 0 -- Test OK, -1 -- Failed
> + * Modifies:	r0-r5
> + */
> +ep93xx_sdram_test:
> +	/* Load the test patterns to be written to SDRAM. */
> +	ldr	r1, =0xf00dface
> +	ldr	r2, =0xdeadbeef
> +	ldr	r3, =0x08675309
> +	ldr	r4, =0xdeafc0ed
> +
> +	/* Store the test patterns to SDRAM. */
> +	stmia	r0, {r1-r4}
> +
> +	/* Load the test patterns from SDRAM one at a time and compare them
> +	 * to the actual pattern.
> +	 */
> +	ldr	r5, [r0]
> +	cmp	r5, r1
> +	ldreq	r5, [r0, #0x0004]
> +	cmpeq	r5, r2
> +	ldreq	r5, [r0, #0x0008]
> +	cmpeq	r5, r3
> +	ldreq	r5, [r0, #0x000c]
> +	cmpeq	r5, r4
> +
> +	/* Return -1 if a mismatch was encountered, 0 otherwise. */
> +	mvnne	r0, #0xffffffff
> +	moveq	r0, #0x00000000
> +
> +	/* Return to the caller. */
> +	mov	pc, lr
> +
> +/*
> + * Determine the size of the SDRAM. Use data=address for the scan.
> + *
> + * Input:	r0 - Start SDRAM address
> + * Return:	r0 - Single block size
> + *		r1 - Valid block mask
> + *		r2 - Total block count
> + * Modifies:	r0-r5
> + */

From V4 discussion:

> > Is this needed? Can't the general get_ram_size() function be used?
> >  
> This is REALLY needed. It's a key feature it calculates RAM size at
> runtime otherwise you have to put constant of data-bus width, actual
> memory installed. And this procedure does it and you don't have to
> recompile it for the very specific board with similar processor
> family.

get_ram_size() does calculate the RAM size at run time, so I still fail
to see why a dedicated function is needed. Can you please clarify this
point?

> diff --git a/board/cirrus/edb93xx/u-boot.lds b/board/cirrus/edb93xx/u-boot.lds
> new file mode 100644
> index 0000000..e61aa36
> --- /dev/null
> +++ b/board/cirrus/edb93xx/u-boot.lds
> @@ -0,0 +1,115 @@
> +/*
> + *
> + * Copyright (C) 2013
> + * Sergey Kostanbaev <sergey.kostanbaev <at> fairwaves.ru>
> + *
> + * Copyright (c) 2004-2008 Texas Instruments
> + *
> + * (C) Copyright 2002
> + * Gary Jennejohn, DENX Software Engineering, <garyj at denx.de>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */

From V4 discussion:

> > Does this board need a specific .lds file?
> >  
> Yes. It must have special magic header to start booting otherwise it
> will be considered as invalid image and internal boot ROOM ignore it.

I guess the magic header is this portion:

> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> +	. = 0x00000000;
> +
> +	. = ALIGN(4);
> +	.text : {
> +		*(.__image_copy_start)
> +		arch/arm/cpu/arm920t/start.o (.text*)
----- start of 'magic header'
> +		. = 0x1000;
> +
> +		LONG(0x53555243)
----- end of 'magic header'
> +		*(.text*)
> +	}

If so, then I do understand that you currently need a specific lds
file, but please document this through a comment in the linker script.

Amicalement,
-- 
Albert.