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 <cl...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to