[U-Boot] [PATCH V2 13/14] ARM: AM43xx: GP_EVM: Add support for DDR3

[Lokesh Vutla <lokeshvutla@ti.com>]

On Wednesday 27 November 2013 05:47 AM, Vaibhav Bedia wrote:
> On Mon, Nov 25, 2013 at 12:18 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>> On Friday 22 November 2013 02:22 AM, Vaibhav Bedia wrote:
>>> On Thu, Nov 21, 2013 at 1:18 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>>> [...]
>>>
>>>> -/*
>>>> - * Get SDRAM type connected to EMIF.
>>>> - * Assuming similar SDRAM parts are connected to both EMIF's
>>>> - * which is typically the case. So it is sufficient to get
>>>> - * SDRAM type from EMIF1.
>>>> - */
>>>> -u32 emif_sdram_type()
>>>> -{
>>>> -       struct emif_reg_struct *emif = (struct emif_reg_struct *)EMIF1_BASE;
>>>> -
>>>> -       return (readl(&emif->emif_sdram_config) &
>>>> -               EMIF_REG_SDRAM_TYPE_MASK) >> EMIF_REG_SDRAM_TYPE_SHIFT;
>>>> -}
>>>> -
>>>>  static inline u32 get_mr(u32 base, u32 cs, u32 mr_addr)
>>>>  {
>>>>         u32 mr;
>>>> diff --git a/arch/arm/include/asm/arch-am33xx/ddr_defs.h b/arch/arm/include/asm/arch-am33xx/ddr_defs.h
>>>> index c98ab7f..646e50f 100644
>>>> --- a/arch/arm/include/asm/arch-am33xx/ddr_defs.h
>>>> +++ b/arch/arm/include/asm/arch-am33xx/ddr_defs.h
>>>> @@ -138,6 +138,14 @@
>>>>  #define  LPDDR2_DATA2_IOCTRL_VALUE   0x20000294
>>>>  #define  LPDDR2_DATA3_IOCTRL_VALUE   0x20000294
>>>>
>>>> +#define  DDR3_ADDRCTRL_WD0_IOCTRL_VALUE 0x00000000
>>>> +#define  DDR3_ADDRCTRL_WD1_IOCTRL_VALUE 0x00000000
>>>> +#define  DDR3_ADDRCTRL_IOCTRL_VALUE   0x84
>>>> +#define  DDR3_DATA0_IOCTRL_VALUE   0x84
>>>> +#define  DDR3_DATA1_IOCTRL_VALUE   0x84
>>>> +#define  DDR3_DATA2_IOCTRL_VALUE   0x84
>>>> +#define  DDR3_DATA3_IOCTRL_VALUE   0x84
>>>> +
>>>>  /**
>>>>   * Configure DMM
>>>>   */
>>>> diff --git a/arch/arm/include/asm/emif.h b/arch/arm/include/asm/emif.h
>>>> index ce6b229..b4a8c9f 100644
>>>> --- a/arch/arm/include/asm/emif.h
>>>> +++ b/arch/arm/include/asm/emif.h
>>>> @@ -1151,6 +1151,20 @@ static inline u32 get_emif_rev(u32 base)
>>>>                 >> EMIF_REG_MAJOR_REVISION_SHIFT;
>>>>  }
>>>>
>>>> +/*
>>>> + * Get SDRAM type connected to EMIF.
>>>> + * Assuming similar SDRAM parts are connected to both EMIF's
>>>> + * which is typically the case. So it is sufficient to get
>>>> + * SDRAM type from EMIF1.
>>>> + */
>>>> +static inline u32 emif_sdram_type(void)
>>>> +{
>>>> +       struct emif_reg_struct *emif = (struct emif_reg_struct *)EMIF1_BASE;
>>>> +
>>>> +       return (readl(&emif->emif_sdram_config) &
>>>> +               EMIF_REG_SDRAM_TYPE_MASK) >> EMIF_REG_SDRAM_TYPE_SHIFT;
>>>> +}
>>>> +
>>>
>>> This change don't make any sense to me. How are the EMIF register bits
>>> any indication
>>> of what memory time is used especially for DDR2/3?
>> What is not making sense here ?
>> If you go and read EMIF spec, it is clearly written that on coming out of reset
>> HW sequence starts according to the value populated in SDRAM config register.
>> As far as I am concerned this is the best way to differentiate between Memories.
>>
> 
> Take a moment to think about where that register value comes from. Does it
> somehow automagically get reconfigured when the chip is put in a board with
> LPDDR2 vs DDR2 vs DDR3? While you are at it also look at the JEDEC specs
> to figure out is there's some way to probe the DDR2/3 memory types.
Ideally the default value should be exported from e-fuse values.
EMIF does some HW sequence according to the value exported here. This filed tells 
what type of memory it is.

I understand the point what if efuse is not blown. I am using this only after we write into sdarm_config register.
This can confirm that we get a correct value.
 If this is not sufficient we can hardcode the register during startup only ?
One more thing is we can get from MR registers of DDR.
But for DDR3 we cannot access MR registers. That is why I didn't go with this approach.

Currently EEPROM doesn't have any details about DDR.
Please let me know if this approach is not good enough.

Thanks and regards,
Lokesh

> 
>> Can you tell me a better way to differentiate between memories in emif file ?
>> Definitely  I should not use board_is_foo() functions.
>>
> 
> AFAIK there is none and hence this way of trying to get the memory type
> is broken.
> 
> Moreover, my understanding was that one of the prime functions of the EEPROM
> board data was to enable differentiation between the memory types.
>