[U-Boot] [RFC] am33xx: add 600us wait in DDR3 initialization sequence

[Egli, Samuel]

Hi all,
me again. I was wondering, if somebody of you has time to check these 
changes? I would appreciate some feedback. Ultimately, it should
also go upstream but my focus here is reviewing the content of
these changes.

Thanks,

Sam

> -----Original Message-----
> From: Egli, Samuel
> Sent: Freitag, 17. Juli 2015 13:47
> To: trini at konsulko.com; doublesin at ti.com; balbi at ti.com
> Cc: u-boot at lists.denx.de; hs at denx.de; Stefan Roese; Meier, Roger; Senn,
> Joerg
> Subject: [RFC] am33xx: add 600us wait in DDR3 initialization sequence
> 
> Hi all,
> the current implementation in ./arch/arm/cpu/armv7/am33xx/emif4.c does not
> respect the the initialization steps as documented in the DDR3
> specification [1]. We noticed that for our am335x based siemens boards a
> delay after config_ddr(...) call was necessary otherwise boot would fail.
> See 61159b76844437bf9004c3a38b5a4ff1a24860d5
> commit that added a 2ms delay which was merly combating symptoms.
> 
> This is because SDRAM wouldn't be ready yet. We didn't realy know why it
> should be necessary but the delay did the job. However, we figured out now
> that an important wait time is not respected, specifically, step 4:
> 
> "After RESET# transitions HIGH, wait 500µs (minus one clock) with CKE LOW"
> [1]
> 
> We also noticed that it is vendor specific and some SDRAM wouldn't bother
> if
> 500 us wait is done or not.
> 
> See below the patch that guarantees that we have a 600µs wait time before
> the clock enable gets enabled. Maybe a comment on the main steps:
> 
> (1) The first time we call config_sdram the CKE controll is still refused
>     EMIF/DDR PHY. But it has the effect that the init sequence is started
>     anyway, and as a consequence puts RESET to high which is the sole goal
>     of this line.
> 
> In (2) + (3) we wait 600us. 100us more to be on the safe side and then
> give control to EMIF/DDR PHY.
> 
> (4) Now, that the wait time is ensured and CKE can be used by the EMIF/DDR
> PHY
>     we trigger the real sdram configuration sequence.
> 
> 
> This patch works fine for us but I'm not sure if some revising of the
> EMIF_4D5 case is necessary, too. With this patch no delay after
> config_ddr(...) is necessary anymore.
> 
> One thing that I cannot explain well, however, is why no issue was
> observed with other TI boards. A possible explenation is that some delay
> stems from code execution time that is done after sdram config before
> loading u-boot.
> 
> But maybe it has also to do with different DDR3 vendors being used. Our
> observation was that Samsung SDRAM is more likely to not work. And that
> smaller sized SDRAM is less/not prone to ignoring the spec. I.e. SDRAM
> with only 128 MB didn't cause any problem. But 256/512 MB from Samsung
> would not work.
> 
> Kind regards
> 
> Sam
> 
> [1] http://www.micron.com/products/datasheets/%7B2ADCEEB1-307F-4B37-99C9-
> 7302CAA8BC5C%7D?page=8#topic=c_Initialization
> 
> 
> ---
>  arch/arm/cpu/armv7/am33xx/emif4.c |   18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/am33xx/emif4.c
> b/arch/arm/cpu/armv7/am33xx/emif4.c
> index 9cf816c..084b45a 100644
> --- a/arch/arm/cpu/armv7/am33xx/emif4.c
> +++ b/arch/arm/cpu/armv7/am33xx/emif4.c
> @@ -109,10 +109,6 @@ void config_ddr(unsigned int pll, const struct
> ctrl_ioregs *ioregs,
>  	config_ddr_data(data, nr);
>  #ifdef CONFIG_AM33XX
>  	config_io_ctrl(ioregs);
> -
> -	/* Set CKE to be controlled by EMIF/DDR PHY */
> -	writel(DDR_CKE_CTRL_NORMAL, &ddrctrl->ddrckectrl);
> -
>  #endif
>  #ifdef CONFIG_AM43XX
>  	writel(readl(&cm_device->cm_dll_ctrl) & ~0x1, &cm_device-
> >cm_dll_ctrl); @@ -131,9 +127,21 @@ void config_ddr(unsigned int pll,
> const struct ctrl_ioregs *ioregs,
>  	/* Program EMIF instance */
>  	config_ddr_phy(regs, nr);
>  	set_sdram_timings(regs, nr);
> +
>  	if (get_emif_rev(EMIF1_BASE) == EMIF_4D5)
>  		config_sdram_emif4d5(regs, nr);
> -	else
> +	else {
> +		/* (1) Set reset signal with CKE still off */
> +		config_sdram(regs, nr);
> +
> +		/* (2) Add 600us delay between Reset and CKE */
> +		udelay(600);
> +
> +		/* (3) Set CKE to be controlled by EMIF/DDR PHY */
> +		writel(DDR_CKE_CTRL_NORMAL, &ddrctrl->ddrckectrl);
> +
> +		/* (4) Configure sdram */
>  		config_sdram(regs, nr);
> +	}
>  }
>  #endif
> --
> 1.7.10.4