From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marek.vasut@gmail.com>
Date: Tue, 29 Jan 2013 13:26:16 +0100
Subject: [U-Boot] [PATCH v2 2/3] mx23: Enable tRAS lockout (24 bit of
	HW_DRAM_CTL08) as in imx-bootlets
In-Reply-To: <CAP9ODKq0uokUJpoJbnn4HEJm9Jw2x4VZ39KUyKhrMOq2y5g1-g@mail.gmail.com>
References: <1359304956-6540-1-git-send-email-otavio@ossystems.com.br>
	<201301290333.56239.marek.vasut@gmail.com>
	<CAP9ODKq0uokUJpoJbnn4HEJm9Jw2x4VZ39KUyKhrMOq2y5g1-g@mail.gmail.com>
Message-ID: <201301291326.17009.marek.vasut@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Otavio Salvador,

> On Tue, Jan 29, 2013 at 12:33 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Dear Otavio Salvador,
> > 
> >> This enables the 'Fast Auto Pre-Charge' found in the memory chip.
> >> 
> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> ---
> >> Changes in v2:
> >> - Improve commit message
> >> 
> >>  arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c index 836e636..a9efd87
> >> 100644 --- a/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> +++ b/arch/arm/cpu/arm926ejs/mxs/spl_mem_init.c
> >> @@ -90,7 +90,7 @@ static uint32_t dram_vals[] = {
> >> 
> >>  #elif defined(CONFIG_MX23)
> >>  
> >>       0x01010001, 0x00010100, 0x01000101, 0x00000001,
> >>       0x00000101, 0x00000000, 0x00010000, 0x01000001,
> >> 
> >> -     0x00000000, 0x00000001, 0x07000200, 0x00070202,
> >> +     0x01000000, 0x00000001, 0x07000200, 0x00070202,
> >> 
> >>       0x02020000, 0x04040a01, 0x00000201, 0x02040000,
> >>       0x02000000, 0x19000f08, 0x0d0d0000, 0x02021313,
> >>       0x02061521, 0x0000000a, 0x00080008, 0x00200020,
> > 
> > I went through the u-boot mem init and detected you apparently added the
> > following undocumented portion of code (the writel((1 << 24 ...) already:
> > 
> > 112 static void initialize_dram_values(void)
> > 113 {
> > 114         int i;
> > 115
> > 116         mxs_adjust_memory_params(dram_vals);
> > 117
> > 118         for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
> > 119                 writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
> > 120
> > 121 #ifdef CONFIG_MX23
> > 122         writel((1 << 24), MXS_DRAM_BASE + (4 * 8));
> > 123 #endif
> > 124 }
> > 
> > It does enable the TRANS_LOCKOUT. So what the heck is going on here? Are
> > you coding this stuff at random now ? WHAT THE FUCK IS HAPPENING HERE ?!
> 
> What your tone, please.

My tone reflects my frustration here.

> Indeed, it does it. It has been done loooong time ago when we started
> looking at MX23 and it was not obvious for me it.
> 
> I will send a patch reverting it and adding a comment explaning it
> there so it is documented.

No! Again, you are charging forward without thinking first!

Why was this code written like that in the first place? Why was this bit set 
later instead of during the register programming in the first place?

Best regards,
Marek Vasut