[U-Boot] [PATCH 1/2] ARM: DRA: EMIF: Change DDR3 settings to use hw leveling

[Sricharan R]

Hi Tom,

On Thursday 07 November 2013 08:33 PM, Tom Rini wrote:
> On Thu, Nov 07, 2013 at 08:17:39PM +0530, Sricharan R wrote:
>
>> Currently the DDR3 memory on DRA7 ES1.0 evm board is enabled using
>> software leveling. This was done since hardware leveling was not
>> working. Now that the right sequence to do hw leveling is identified,
>> use it. This is required for EMIF clockdomain to idle and come back
>> during lowpower usecases.
> [snip]
>> @@ -210,54 +210,76 @@ static void ddr3_leveling(u32 base, const struct emif_regs *regs)
>>  {
>>  	struct emif_reg_struct *emif = (struct emif_reg_struct *)base;
>>  
>> -	/* keep sdram in self-refresh */
> [snip]
>> +	if (omap_revision() != DRA752_ES1_0)	{
>> +		/* keep sdram in self-refresh */
> [snip]
>> +	} else {
>> +		u32 fifo_reg;
> [snip]
>> +		/* Disable leveling */
>> +		writel(0x0, &emif->emif_rd_wr_lvl_rmp_ctl);
>> +	}
> Two things here.  First, it seems that now ddr3_leveling has one
> sequence for "not DRA752_ES1_0" (so likely to get wrong when used on
> DRA752 whatever comes after ES1.0) and another for DRA752_ES1_0.  This
> would be because the it's different EMIF blocks and related HW, so it's
> a different sequence, yes?  Second, the comment at the end of this
> function about "Disable leveling" seems misleading, but maybe it's just
> me.  We're saying "leveling sequence is complete now", yes?
>
> For the second issue, we can expand / clarify the comment.  For the
> first issue, maybe we shouldn't have a single "ddr3_leveling" function
> but per family ones?  There's nothing in common between the two cases
> here, so we're just gaining an indentation level when we could be
> excluding code from the final binary instead (either ifdef or maybe just
> separate files?).
>
 Yes, i also thought about having a separate function for OMAP5/DRA.
 I will do that. For the point about disable leveling, it was intentional.
 
 After we are done with a leveling once during boot, we do not intend to
 enable it anymore. We see that the PHY misbehaves by triggering a wrong
 leveling sequence when EMIF hits idle, which results in wrong delay values
 if it is left enabled. I will add this comment in V2

Regards,
 Sricharan