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

[Lokesh Vutla <lokeshvutla@ti.com>]

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.

Can you tell me a better way to differentiate between memories in emif file ?
Definitely  I should not use board_is_foo() functions.

Thanks
Lokesh
> 
> [...]
>>
>> +static void enable_vtt_regulator(void)
>> +{
>> +       u32 temp;
>> +
>> +       /* enable module */
>> +       writel(0x0, AM33XX_GPIO0_BASE + OMAP_GPIO_CTRL);
>> +
>> +       /*enable output for GPIO0_22*/
>> +       writel((1 << 22), AM33XX_GPIO0_BASE + OMAP_GPIO_SETDATAOUT);
>> +       temp = readl(AM33XX_GPIO0_BASE + OMAP_GPIO_OE);
>> +       temp = temp & ~(1 << 22);
>> +       writel(temp, AM33XX_GPIO0_BASE + OMAP_GPIO_OE);
>> +}
>> +
> 
> No magic nos. please. I think you have a #def the pin used for VTT regulator
> in the config file - makes it simpler for others who design their own board.
> 
> Regards,
> Vaibhav
>