[U-Boot] [PATCH 1/2] OMAP3 SPL: Rework memory initalization and devkit 8000 support

Igor Grinberg grinberg at compulab.co.il
Thu Oct 27 22:46:45 CEST 2011


Hi Tom,

That seems like a good change, though I would recommend to split it into
smaller
patches, please, see below:

On 10/26/2011 11:13 PM, Tom Rini wrote:
> This changes to making the board be responsible for providing the
> memory initialization timings in SPL

That probably would be one patch.

>  and converts the devkit 8000
> to this framework.

That, would be another one.

>   As part of this suffix the Micron DDR settings
> with their speed

Next patch.

>  and add a few more timing values that will be needed.

This can go along with the patch that uses those settings.

> We also make sure that in mem_ok() we clear the values off as we may be
> testing the same banks multiple times.

That's should be another patch (although it has only one line).

> Signed-off-by: Tom Rini <trini at ti.com>
> ---
>  arch/arm/cpu/armv7/omap3/mem.c              |    1 +
>  arch/arm/cpu/armv7/omap3/sdrc.c             |  132 +++++++++++++++------------
>  arch/arm/include/asm/arch-omap3/mem.h       |   58 +++++-------
>  arch/arm/include/asm/arch-omap3/sys_proto.h |    2 +-
>  board/timll/devkit8000/devkit8000.c         |   24 +++++
>  5 files changed, 123 insertions(+), 94 deletions(-)
>
> diff --git a/arch/arm/cpu/armv7/omap3/mem.c b/arch/arm/cpu/armv7/omap3/mem.c
> index a01c303..cd5fe5c 100644
> --- a/arch/arm/cpu/armv7/omap3/mem.c
> +++ b/arch/arm/cpu/armv7/omap3/mem.c
> @@ -86,6 +86,7 @@ u32 mem_ok(u32 cs)
>  	writel(0x0, addr + 4);		/* remove pattern off the bus */
>  	val1 = readl(addr + 0x400);	/* get pos A value */
>  	val2 = readl(addr);		/* get val2 */
> +	writel(0x0, addr + 0x400);	/* clear pos A */
>  
>  	if ((val1 != 0) || (val2 != pattern))	/* see if pos A val changed */
>  		return 0;
> diff --git a/arch/arm/cpu/armv7/omap3/sdrc.c b/arch/arm/cpu/armv7/omap3/sdrc.c
> index 0dd1955..4799787 100644
> --- a/arch/arm/cpu/armv7/omap3/sdrc.c
> +++ b/arch/arm/cpu/armv7/omap3/sdrc.c
> @@ -109,15 +109,56 @@ u32 get_sdr_cs_offset(u32 cs)
>  }
>  
>  /*
> + * write_sdrc_timings -
> + *  - Takes CS and associated timings and initalize SDRAM
> + *  - Test CS to make sure it's OK for use
> + */
> +static void write_sdrc_timings(u32 cs, struct sdrc_actim *sdrc_actim_base,
> +		u32 mcfg, u32 ctrla, u32 ctrlb, u32 rfr_ctrl, u32 mr)
> +{
> +	/* Setup timings we got from the board. */
> +	writel(mcfg, &sdrc_base->cs[cs].mcfg);
> +	writel(ctrla, &sdrc_actim_base->ctrla);
> +	writel(ctrlb, &sdrc_actim_base->ctrlb);
> +	writel(rfr_ctrl, &sdrc_base->cs[cs].rfr_ctrl);
> +	writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> +	writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> +	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +	writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +	writel(mr, &sdrc_base->cs[cs].mr);
> +
> +	/*
> +	 * Test ram in this bank
> +	 * Disable if bad or not present
> +	 */
> +	if (!mem_ok(cs))
> +		writel(0, &sdrc_base->cs[cs].mcfg);
> +}
> +
> +/*
>   * do_sdrc_init -
> - *  - Initialize the SDRAM for use.
> - *  - code called once in C-Stack only context for CS0 and a possible 2nd
> - *    time depending on memory configuration from stack+global context
> + *  - Code called once in C-Stack only context for CS0 and with early being
> + *    true and a possible 2nd time depending on memory configuration from
> + *    stack+global context.
>   */
>  void do_sdrc_init(u32 cs, u32 early)
>  {
> -	struct sdrc_actim *sdrc_actim_base0, *sdrc_actim_base1;
> +	struct sdrc_actim *sdrc_actim_base0;
> +	u32 mcfg, ctrla, ctrlb, rfr_ctrl, mr;
> +#ifdef CONFIG_SPL_BUILD
> +	u32 cs_cfg;
> +#endif
>  
> +	/*
> +	 * When called in the early context this may be SPL and we will
> +	 * need to set all of the timings.  This ends up being board
> +	 * specific so we call a helper function to take care of this
> +	 * for us.  Otherwise, to be safe, we need to copy the settings
> +	 * from the first bank to the second.
> +	 */
> +#ifdef CONFIG_SPL_BUILD
> +	get_board_mem_timings(&cs_cfg, &mcfg, &ctrla, &ctrlb, &rfr_ctrl, &mr);
> +#endif
>  	if (early) {
>  		/* reset sdrc controller */
>  		writel(SOFTRESET, &sdrc_base->sysconfig);
> @@ -128,73 +169,45 @@ void do_sdrc_init(u32 cs, u32 early)
>  		/* setup sdrc to ball mux */
>  		writel(SDRC_SHARING, &sdrc_base->sharing);
>  
> -		/* Disable Power Down of CKE cuz of 1 CKE on combo part */
> +		/* Disable Power Down of CKE because of 1 CKE on combo part */
>  		writel(WAKEUPPROC | SRFRONRESET | PAGEPOLICY_HIGH,
>  				&sdrc_base->power);
>  
>  		writel(ENADLL | DLLPHASE_90, &sdrc_base->dlla_ctrl);
>  		sdelay(0x20000);
> -	}
> -
> -/* As long as V_MCFG and V_RFR_CTRL is not defined for all OMAP3 boards we need
> - * to prevent this to be build in non-SPL build */
>  #ifdef CONFIG_SPL_BUILD
> -	/* If we use a SPL there is no x-loader nor config header so we have
> -	 * to do the job ourselfs
> -	 */
> -	if (cs == CS0) {
> -		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
> -
> -		/* General SDRC config */
> -		writel(V_MCFG, &sdrc_base->cs[cs].mcfg);
> -		writel(V_RFR_CTRL, &sdrc_base->cs[cs].rfr_ctrl);
> -
> -		/* AC timings */
> -		writel(V_ACTIMA_165, &sdrc_actim_base0->ctrla);
> -		writel(V_ACTIMB_165, &sdrc_actim_base0->ctrlb);
> -
> -		/* Initialize */
> -		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> -		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> +		/* We only set cs_cfg in this case */
> +		writel(cs_cfg, &sdrc_base->cs_cfg);
> +
> +		/* We need to do both banks now, in many cases. */
> +		write_sdrc_timings(CS0,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
> +		write_sdrc_timings(CS1,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
> +#endif
>  

You probably can remove that empty line.

> -		writel(V_MR, &sdrc_base->cs[cs].mr);
>  	}
> -#endif
>  
>  	/*
> -	 * SDRC timings are set up by x-load or config header
> -	 * We don't need to redo them here.
> -	 * Older x-loads configure only CS0
> -	 * configure CS1 to handle this ommission
> +	 * If we aren't using SPL we have been loaded by some
> +	 * other means which may not have correctly initialized
> +	 * both CS0 and CS1 (such as some older versions of x-loader)
> +	 * so we may be asked now to setup CS1.
>  	 */
>  	if (cs == CS1) {
>  		sdrc_actim_base0 = (struct sdrc_actim *)SDRC_ACTIM_CTRL0_BASE;
> -		sdrc_actim_base1 = (struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE;
> -		writel(readl(&sdrc_base->cs[CS0].mcfg),
> -			&sdrc_base->cs[CS1].mcfg);
> -		writel(readl(&sdrc_base->cs[CS0].rfr_ctrl),
> -			&sdrc_base->cs[CS1].rfr_ctrl);
> -		writel(readl(&sdrc_actim_base0->ctrla),
> -			&sdrc_actim_base1->ctrla);
> -		writel(readl(&sdrc_actim_base0->ctrlb),
> -			&sdrc_actim_base1->ctrlb);
> -
> -		writel(CMD_NOP, &sdrc_base->cs[cs].manual);
> -		writel(CMD_PRECHARGE, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(CMD_AUTOREFRESH, &sdrc_base->cs[cs].manual);
> -		writel(readl(&sdrc_base->cs[CS0].mr),
> -			&sdrc_base->cs[CS1].mr);
> -	}
> +		mcfg = readl(&sdrc_base->cs[CS0].mcfg),
> +		rfr_ctrl = readl(&sdrc_base->cs[CS0].rfr_ctrl);
> +		ctrla = readl(&sdrc_actim_base0->ctrla),
> +		ctrlb = readl(&sdrc_actim_base0->ctrlb);
> +		mr = readl(&sdrc_base->cs[CS0].mr);
> +		write_sdrc_timings(cs,
> +				(struct sdrc_actim *)SDRC_ACTIM_CTRL1_BASE,
> +				mcfg, ctrla, ctrlb, rfr_ctrl, mr);
>  

same here

> -	/*
> -	 * Test ram in this bank
> -	 * Disable if bad or not present
> -	 */
> -	if (!mem_ok(cs))
> -		writel(0, &sdrc_base->cs[cs].mcfg);
> +	}
>  }
>  
>  /*
> @@ -208,8 +221,9 @@ int dram_init(void)
>  	size0 = get_sdr_cs_size(CS0);
>  	/*
>  	 * If a second bank of DDR is attached to CS1 this is
> -	 * where it can be started.  Early init code will init
> -	 * memory on CS0.
> +	 * where it can be found.  If we have SPL that code will have

Why do you need two spaces before the if?

> +	 * initalized it already, otherwise early init code will init
> +	 * memory on CS0 only.
>  	 */
>  	if ((sysinfo.mtype == DDR_COMBO) || (sysinfo.mtype == DDR_STACKED)) {
>  		do_sdrc_init(CS1, NOT_EARLY);
> diff --git a/arch/arm/include/asm/arch-omap3/mem.h b/arch/arm/include/asm/arch-omap3/mem.h
> index 8e28f77..af3504c 100644
> --- a/arch/arm/include/asm/arch-omap3/mem.h
> +++ b/arch/arm/include/asm/arch-omap3/mem.h
> @@ -37,12 +37,28 @@ enum {
>  };
>  #endif /* __ASSEMBLY__ */
>  
> +/* Memory that can be connected to GPMC */
> +#define GPMC_NOR            0
> +#define GPMC_NAND           1
> +#define GPMC_MDOC           2
> +#define GPMC_ONENAND        3
> +#define MMC_NAND            4
> +#define MMC_ONENAND         5
> +#define GPMC_NONE           6
> +#define GPMC_ONENAND_TRY    7
> +

The comment is misleading.
This probably was copied from the X-Loader, and each define specifies
the boot storage devices probing order, right?

I think the above does not belong here as all the GPMC code in this file,
we have omap-gpmc.h file in this directory for the GPMC stuff,
but as the GPMC code is already here, I don't think it is right to ask
you to
move it... So volunteers are welcome...

>  #define EARLY_INIT	1
>  
>  /* Slower full frequency range default timings for x32 operation*/
>  #define SDRC_SHARING	0x00000100
>  #define SDRC_MR_0_SDR	0x00000031
>  
> +/* optimized timings good for current shipping parts */
> +#define SDP_3430_SDRC_RFR_CTRL_100MHz   0x0002da01
> +#define SDP_3430_SDRC_RFR_CTRL_133MHz   0x0003de01 /* 7.8us/7.5ns - 50=0x3de */
> +#define SDP_3430_SDRC_RFR_CTRL_165MHz   0x0004e201 /* 7.8us/6ns - 50=0x4e2 */
> +#define SDP_3430_SDRC_RFR_CTRL_200MHz   0x0005e601 /* 7.8us/5ns - 50=0x5e6 */
> +
>  #define DLL_OFFSET		0
>  #define DLL_WRITEDDRCLKX2DIS	1
>  #define DLL_ENADLL		1
> @@ -138,15 +154,15 @@ enum {
>  #define MICRON_CASWIDTH			0x5
>  #define MICRON_RASWIDTH			0x2
>  #define MICRON_LOCKSTATUS		0x0
> -#define MICRON_V_MCFG ((MICRON_LOCKSTATUS << 30) | (MICRON_RASWIDTH << 24) | \
> -	(MICRON_CASWIDTH << 20) | (MICRON_ADDRMUXLEGACY << 19) | \
> -	(MICRON_RAMSIZE << 8) | (MICRON_BANKALLOCATION << 6) | \
> -	(MICRON_B32NOT16 << 4) | (MICRON_DEEPPD << 3) | \
> -	(MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
> +#define MICRON_V_MCFG_165 		((MICRON_LOCKSTATUS << 30) | \

The above line should be generating a white space error.
There is a mixture of tabs and spaces.
Also, why don't you just use one tab or space?

> +	(MICRON_RASWIDTH << 24) | (MICRON_CASWIDTH << 20) | \
> +	(MICRON_ADDRMUXLEGACY << 19) | (MICRON_RAMSIZE << 8) | \
> +	(MICRON_BANKALLOCATION << 6) | (MICRON_B32NOT16 << 4) | \
> +	(MICRON_DEEPPD << 3) | (MICRON_DDRTYPE << 2) | (MICRON_RAMTYPE))
>  
> -#define MICRON_ARCV				2030
> -#define MICRON_ARE				0x1
> -#define MICRON_V_RFR_CTRL ((MICRON_ARCV << 8) | (MICRON_ARE))
> +#define MICRON_ARCV_165		0x4e2
> +#define MICRON_ARE		0x1

The above values are not aligned with all the others.

> +#define MICRON_V_RFR_CTRL_165	((MICRON_ARCV_165 << 8) | (MICRON_ARE))

The MICRON_ARE macro does not need a parenthesis.

>  
>  #define MICRON_BL				0x2
>  #define MICRON_SIL				0x0
> @@ -194,32 +210,6 @@ enum {
>  		(NUMONYX_XSR_165 << 0) | (NUMONYX_TXP_165 << 8) | \
>  		(NUMONYX_TWTR_165 << 16))
>  
> -#ifdef CONFIG_OMAP3_INFINEON_DDR
> -#define V_ACTIMA_165 INFINEON_V_ACTIMA_165
> -#define V_ACTIMB_165 INFINEON_V_ACTIMB_165
> -#endif
> -
> -#ifdef CONFIG_OMAP3_MICRON_DDR
> -#define V_ACTIMA_165 MICRON_V_ACTIMA_165
> -#define V_ACTIMB_165 MICRON_V_ACTIMB_165
> -#define V_MCFG			MICRON_V_MCFG
> -#define V_RFR_CTRL		MICRON_V_RFR_CTRL
> -#define V_MR			MICRON_V_MR
> -#endif
> -
> -#ifdef CONFIG_OMAP3_NUMONYX_DDR
> -#define V_ACTIMA_165 NUMONYX_V_ACTIMA_165
> -#define V_ACTIMB_165 NUMONYX_V_ACTIMB_165
> -#endif
> -
> -#if !defined(V_ACTIMA_165) || !defined(V_ACTIMB_165)
> -#error "Please choose the right DDR type in config header"
> -#endif
> -
> -#if defined(CONFIG_SPL_BUILD) && (!defined(V_MCFG) || !defined(V_RFR_CTRL))
> -#error "Please choose the right DDR type in config header"
> -#endif
> -
>  /*
>   * GPMC settings -
>   * Definitions is as per the following format
> diff --git a/arch/arm/include/asm/arch-omap3/sys_proto.h b/arch/arm/include/asm/arch-omap3/sys_proto.h
> index 7b60051..a2c317a 100644
> --- a/arch/arm/include/asm/arch-omap3/sys_proto.h
> +++ b/arch/arm/include/asm/arch-omap3/sys_proto.h
> @@ -38,6 +38,7 @@ void per_clocks_enable(void);
>  void memif_init(void);
>  void sdrc_init(void);
>  void do_sdrc_init(u32, u32);
> +void get_board_mem_timings(u32 *, u32 *, u32 *, u32 *, u32 *, u32 *);
>  void emif4_init(void);
>  void gpmc_init(void);
>  void enable_gpmc_cs_config(const u32 *gpmc_config, struct gpmc_cs *cs, u32 base,
> @@ -49,7 +50,6 @@ void set_muxconf_regs(void);
>  u32 get_cpu_family(void);
>  u32 get_cpu_rev(void);
>  u32 get_sku_id(void);
> -u32 get_mem_type(void);

I think the above will break omap2420h4 and apollon boards.
Also, how is this related to that patch?

>  u32 get_sysboot_value(void);
>  u32 is_gpmc_muxed(void);
>  u32 get_gpmc0_type(void);
> diff --git a/board/timll/devkit8000/devkit8000.c b/board/timll/devkit8000/devkit8000.c
> index f50d113..568f02e 100644
> --- a/board/timll/devkit8000/devkit8000.c
> +++ b/board/timll/devkit8000/devkit8000.c
> @@ -138,3 +138,27 @@ int board_eth_init(bd_t *bis)
>  	return dm9000_initialize(bis);
>  }
>  #endif
> +
> +/* 
> + * Routine: get_board_mem_timings
> + * Description: If we use SPL then there is no x-loader nor config header
> + * so we have to setup the DDR timings outself on the first bank.  This

s/outself on/our self from/
Also two spaces before "This".

> + * provides the timing values back to the function that configures
> + * the memory.
> + */
> +void get_board_mem_timings(u32 *cs_cfg, u32 *mcfg, u32 *ctrla, u32 *ctrlb,
> +		u32 *rfr_ctrl, u32 *mr)
> +{
> +	/* 128MiB/bank */
> +	*cs_cfg = 0x1;
> +
> +	/* General SDRC config */
> +	*mcfg = MICRON_V_MCFG_165;
> +	*rfr_ctrl = MICRON_V_RFR_CTRL_165;
> +
> +	/* AC timings */
> +	*ctrla = MICRON_V_ACTIMA_165;
> +	*ctrlb = MICRON_V_ACTIMB_165;
> +
> +	*mr = MICRON_V_MR;
> +}

This looks good.

Regards,
Igor


More information about the U-Boot mailing list