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