[U-Boot] [PATCH 1/9] ddr: altera: Configuring SDRAM extra cycles timing parameters

[Marek Vasut]

On 09/19/2016 12:11 PM, Chin Liang See wrote:
> On Mon, 2016-09-19 at 16:22 +0200, Marek Vasut wrote:
>> On 09/15/2016 09:26 AM, Chin Liang See wrote:
>>> To enable configuration of sdr.ctrlcfg.extratime1 register which
>>> enable
>>> extra clocks for read to write command timing. This is critical to
>>> ensure successful LPDDR2 interface
>>>
>>> Signed-off-by: Chin Liang See <clsee at altera.com>
>>> ---
>>>  arch/arm/mach-socfpga/include/mach/sdram.h | 8 +++++++-
>>>  arch/arm/mach-socfpga/qts-filter.sh        | 2 +-
>>>  arch/arm/mach-socfpga/wrap_sdram_config.c  | 9 +++++++++
>>>  drivers/ddr/altera/sdram.c                 | 3 +++
>>>  4 files changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/include/mach/sdram.h
>>> b/arch/arm/mach-socfpga/include/mach/sdram.h
>>> index f12bb84..b11228f 100644
>>> --- a/arch/arm/mach-socfpga/include/mach/sdram.h
>>> +++ b/arch/arm/mach-socfpga/include/mach/sdram.h
>>> @@ -30,7 +30,8 @@ struct socfpga_sdr_ctrl {
>>>  	u32	dram_timing4;	/* 0x10 */
>>>  	u32	lowpwr_timing;
>>>  	u32	dram_odt;
>>> -	u32	__padding0[4];
>>> +	u32	extratime1;
>>> +	u32	__padding0[3];
>>>  	u32	dram_addrw;	/* 0x2c */
>>>  	u32	dram_if_width;	/* 0x30 */
>>>  	u32	dram_dev_width;
>>> @@ -88,6 +89,7 @@ struct socfpga_sdram_config {
>>>  	u32	dram_timing4;
>>>  	u32	lowpwr_timing;
>>>  	u32	dram_odt;
>>> +	u32	extratime1;
>>>  	u32	dram_addrw;
>>>  	u32	dram_if_width;
>>>  	u32	dram_dev_width;
>>
>> This seems to be changing the DRAM register layout, is this really
>> correct and was this really tested on AV SoCDK ?
> 
> Previously its treated unused register as default value is good enough.
> But this not true anymore for LPDDR2 and we are exposing extratime1
> register.

I mean the later one , which adds an entry and moves the other registers
by 4 bytes.

> While for testing, I tested both CV and AV SoCDK few times as I also
> worried even they are using DDR3 instead LPDDR2.

Yes, they do, it's in the documentation ;-)

>>
>> [...]
>>
>>> diff --git a/arch/arm/mach-socfpga/wrap_sdram_config.c
>>> b/arch/arm/mach-socfpga/wrap_sdram_config.c
>>> index 31cc7de..d72f5e1 100644
>>> --- a/arch/arm/mach-socfpga/wrap_sdram_config.c
>>> +++ b/arch/arm/mach-socfpga/wrap_sdram_config.c
>>> @@ -81,6 +81,15 @@ static const struct socfpga_sdram_config
>>> sdram_config = {
>>>  			SDR_CTRLGRP_DRAMODT_READ_LSB)		
>>> 	|
>>>  		(CONFIG_HPS_SDR_CTRLCFG_DRAMODT_WRITE <<
>>>  			SDR_CTRLGRP_DRAMODT_WRITE_LSB),
>>> +#ifdef
>>> CONFIG_HPS_SDR_CTRLCFG_EXTRATIME1_CFG_EXTRA_CTL_CLK_RD_TO_WR
>>
>> How come this is not always defined for all boards ?
> 
> This is to ensure it still works if users are using older SOCEDS
> instead of SOCEDS 16.1. Besides that, this is only applicable for
> LPDDR2. With that, patches #2 to #9 are not needed.
> 
> Thanks
> Chin Liang
> 
>>
>>> +	.extratime1 =
>>> +	(CONFIG_HPS_SDR_CTRLCFG_EXTRATIME1_CFG_EXTRA_CTL_CLK_RD_TO
>>> _WR <<
>>> +			SDR_CTRLGRP_EXTRATIME1_RD_TO_WR_LSB)	
>>> 	|
>>> +	(CONFIG_HPS_SDR_CTRLCFG_EXTRATIME1_CFG_EXTRA_CTL_CLK_RD_TO
>>> _WR_BC <<
>>> +			SDR_CTRLGRP_EXTRATIME1_RD_TO_WR_BC_LSB)	
>>> 	|
>>> +(CONFIG_HPS_SDR_CTRLCFG_EXTRATIME1_CFG_EXTRA_CTL_CLK_RD_TO_WR_DIFF
>>> _CHIP <<
>>> +			SDR_CTRLGRP_EXTRATIME1_RD_TO_WR_DIFF_LSB),
>>> +#endif
>>>  	.dram_addrw =
>>>  		(CONFIG_HPS_SDR_CTRLCFG_DRAMADDRW_COLBITS <<
>>>  			SDR_CTRLGRP_DRAMADDRW_COLBITS_LSB)		
>>> |
>>> diff --git a/drivers/ddr/altera/sdram.c
>>> b/drivers/ddr/altera/sdram.c
>>> index 7e4606d..e74c5b0 100644
>>> --- a/drivers/ddr/altera/sdram.c
>>> +++ b/drivers/ddr/altera/sdram.c
>>> @@ -418,6 +418,9 @@ static void sdr_load_regs(const struct
>>> socfpga_sdram_config *cfg)
>>>  
>>>  	debug("Configuring DRAMODT\n");
>>>  	writel(cfg->dram_odt, &sdr_ctrl->dram_odt);
>>> +
>>> +	debug("Configuring EXTRATIME1\n");
>>> +	writel(cfg->extratime1, &sdr_ctrl->extratime1);
>>>  }
>>>  
>>>  /**
>>>
>>
>>


-- 
Best regards,
Marek Vasut