[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

This function can be overridden at run-time and allows implementors
of new boards based on the i.MX28 chip to fine-tune the memory params.
It is possible to write into the dram_vals array because when the SPL
runs, it is located SRAM. Therefore the location is writable. There is
no possibility of these data to be read-only.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
index 9fa5d29..e17a4d7 100644
--- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
@@ -82,10 +82,18 @@ uint32_t dram_vals[] = {
 	0x00000000, 0x00010001
 };
 
+void __mx28_adjust_memory_params(uint32_t *dram_vals)
+{
+}
+void mx28_adjust_memory_params(uint32_t *dram_vals)
+	__attribute__((weak, alias("__mx28_adjust_memory_params")));
+
 void init_m28_200mhz_ddr2(void)
 {
 	int i;
 
+	mx28_adjust_memory_params(dram_vals);
+
 	for (i = 0; i < ARRAY_SIZE(dram_vals); i++)
 		writel(dram_vals[i], MXS_DRAM_BASE + (4 * i));
 }
-- 
1.7.10

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Fabio Estevam]

Hi Marek,

On Wed, May 30, 2012 at 2:43 AM, Marek Vasut <marex@denx.de> wrote:
> This function can be overridden at run-time and allows implementors
> of new boards based on the i.MX28 chip to fine-tune the memory params.
> It is possible to write into the dram_vals array because when the SPL
> runs, it is located SRAM. Therefore the location is writable. There is
> no possibility of these data to be read-only.

This still does not fix the mx28evk booting issue on u-boot-imx tree.

Have you tried to boot m28evk with the following commit reverted?

commit 19a2066b578f826c7bcb2b96a90434382e8ec8f3
Author: Marek Vasut <marex@denx.de>
Date:   Thu May 3 05:47:19 2012 +0000

    M28: Scan only first 512 MB of DRAM to avoid memory wraparound

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Wed, May 30, 2012 at 2:43 AM, Marek Vasut <marex@denx.de> wrote:
> > This function can be overridden at run-time and allows implementors
> > of new boards based on the i.MX28 chip to fine-tune the memory params.
> > It is possible to write into the dram_vals array because when the SPL
> > runs, it is located SRAM. Therefore the location is writable. There is
> > no possibility of these data to be read-only.
> 
> This still does not fix the mx28evk booting issue on u-boot-imx tree.
> 
> Have you tried to boot m28evk with the following commit reverted?

It's broken, that's why the following patch was applied ;-)

Call this or similar in your spl_boot.c to adjust the DRAM configuration. 
Actually I think even the following value should fix your problem:

void mx28_adjust_memory_params(uint32_t *dram_vals)
{
        dram_vals[0x74 >> 2] = 0x0f02010a;
}

> 
> commit 19a2066b578f826c7bcb2b96a90434382e8ec8f3
> Author: Marek Vasut <marex@denx.de>
> Date:   Thu May 3 05:47:19 2012 +0000
> 
>     M28: Scan only first 512 MB of DRAM to avoid memory wraparound

Best regards,
Marek Vasut

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Fabio Estevam]

Hi Marek,

On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote:

> Call this or similar in your spl_boot.c to adjust the DRAM configuration.
> Actually I think even the following value should fix your problem:
>
> void mx28_adjust_memory_params(uint32_t *dram_vals)
> {
> ? ? ? ?dram_vals[0x74 >> 2] = 0x0f02010a;
> }

I finally had a chance to try this and my mx28evk can boot again with
your patch + the code above.

How do we handle this? Would you send a v2 with this additional code
or should I do it myself after your original patch in this thread gets
applied?

Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] =
0x0f02010a;' means.

It is not something very trivial to figure out :-)

Thanks,

Fabio Estevam

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote:
> > Call this or similar in your spl_boot.c to adjust the DRAM configuration.
> > Actually I think even the following value should fix your problem:
> > 
> > void mx28_adjust_memory_params(uint32_t *dram_vals)
> > {
> >        dram_vals[0x74 >> 2] = 0x0f02010a;
> > }
> 
> I finally had a chance to try this and my mx28evk can boot again with
> your patch + the code above.
> 
> How do we handle this? Would you send a v2 with this additional code
> or should I do it myself after your original patch in this thread gets
> applied?

Stefano, can you please apply this? So Fabio can apply his on top of it?

> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] =
> 0x0f02010a;' means.

Well, check that particular register (memory configuration register at +0x74 
offset) in the datasheet. It's basically adjusting the number of enabled address 
row and columns.

> It is not something very trivial to figure out :-)

It actually is, see above ;-)

> Thanks,
> 
> Fabio Estevam

Best regards,
Marek Vasut

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Stefano Babic]

On 18/06/2012 16:23, Marek Vasut wrote:
> Dear Fabio Estevam,
> 
>> Hi Marek,
>>
>> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote:
>>> Call this or similar in your spl_boot.c to adjust the DRAM configuration.
>>> Actually I think even the following value should fix your problem:
>>>
>>> void mx28_adjust_memory_params(uint32_t *dram_vals)
>>> {
>>>        dram_vals[0x74 >> 2] = 0x0f02010a;
>>> }
>>
>> I finally had a chance to try this and my mx28evk can boot again with
>> your patch + the code above.
>>
>> How do we handle this? Would you send a v2 with this additional code
>> or should I do it myself after your original patch in this thread gets
>> applied?
> 
> Stefano, can you please apply this? So Fabio can apply his on top of it?

I do it.
> 
>> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] =
>> 0x0f02010a;' means.
> 
> Well, check that particular register (memory configuration register at +0x74 
> offset) in the datasheet. It's basically adjusting the number of enabled address 
> row and columns.
> 
>> It is not something very trivial to figure out :-)
> 
> It actually is, see above ;-)

I see, but I will never apply a patch programmed in hexadecimal instead
of plain C ;-)

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

Dear Stefano Babic,

> On 18/06/2012 16:23, Marek Vasut wrote:
> > Dear Fabio Estevam,
> > 
> >> Hi Marek,
> >> 
> >> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote:
> >>> Call this or similar in your spl_boot.c to adjust the DRAM
> >>> configuration. Actually I think even the following value should fix
> >>> your problem:
> >>> 
> >>> void mx28_adjust_memory_params(uint32_t *dram_vals)
> >>> {
> >>> 
> >>>        dram_vals[0x74 >> 2] = 0x0f02010a;
> >>> 
> >>> }
> >> 
> >> I finally had a chance to try this and my mx28evk can boot again with
> >> your patch + the code above.
> >> 
> >> How do we handle this? Would you send a v2 with this additional code
> >> or should I do it myself after your original patch in this thread gets
> >> applied?
> > 
> > Stefano, can you please apply this? So Fabio can apply his on top of it?
> 
> I do it.
> 
> >> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] =
> >> 0x0f02010a;' means.
> > 
> > Well, check that particular register (memory configuration register at
> > +0x74 offset) in the datasheet. It's basically adjusting the number of
> > enabled address row and columns.
> > 
> >> It is not something very trivial to figure out :-)
> > 
> > It actually is, see above ;-)
> 
> I see, but I will never apply a patch programmed in hexadecimal instead
> of plain C ;-)

So ... what do you expect me to do? The register is called HW_DRAM_CTL29 (very 
descriptive) ... do you want me to create a file full of HW_DRAM_CTLnn ?

> 
> Stefano

Best regards,
Marek Vasut

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

Dear Stefano Babic,

> > On 18/06/2012 16:23, Marek Vasut wrote:
> > > Dear Fabio Estevam,
> > > 
> > >> Hi Marek,
> > >> 
> > >> On Thu, May 31, 2012 at 3:45 PM, Marek Vasut <marex@denx.de> wrote:
> > >>> Call this or similar in your spl_boot.c to adjust the DRAM
> > >>> configuration. Actually I think even the following value should fix
> > >>> your problem:
> > >>> 
> > >>> void mx28_adjust_memory_params(uint32_t *dram_vals)
> > >>> {
> > >>> 
> > >>>        dram_vals[0x74 >> 2] = 0x0f02010a;
> > >>> 
> > >>> }
> > >> 
> > >> I finally had a chance to try this and my mx28evk can boot again with
> > >> your patch + the code above.
> > >> 
> > >> How do we handle this? Would you send a v2 with this additional code
> > >> or should I do it myself after your original patch in this thread gets
> > >> applied?
> > > 
> > > Stefano, can you please apply this? So Fabio can apply his on top of
> > > it?
> > 
> > I do it.
> > 
> > >> Also, please explain to the mortal folks what 'dram_vals[0x74 >> 2] =
> > >> 0x0f02010a;' means.
> > > 
> > > Well, check that particular register (memory configuration register at
> > > +0x74 offset) in the datasheet. It's basically adjusting the number of
> > > enabled address row and columns.
> > > 
> > >> It is not something very trivial to figure out :-)
> > > 
> > > It actually is, see above ;-)
> > 
> > I see, but I will never apply a patch programmed in hexadecimal instead
> > of plain C ;-)
> 
> So ... what do you expect me to do? The register is called HW_DRAM_CTL29
> (very descriptive) ... do you want me to create a file full of
> HW_DRAM_CTLnn ?

Would adding such comment be enough to satisfy you? (In slightly more polite 
manner I mean ... damn I'm loosing it, 2 days until exam :'-( ). Sorry for being 
rough, Stefano.

> > Stefano

Best regards,
Marek Vasut

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Stefano Babic]

On 18/06/2012 17:43, Marek Vasut wrote:

>>
>> So ... what do you expect me to do? The register is called HW_DRAM_CTL29
>> (very descriptive) ... do you want me to create a file full of
>> HW_DRAM_CTLnn ?
> 
> Would adding such comment be enough to satisfy you? (In slightly more polite 
> manner I mean ... damn I'm loosing it, 2 days until exam :'-( ). Sorry for being 
> rough, Stefano.

Do not worry, it is normal before exam ;-)

Yes, of course comments will help !

Best regards,
Stefano


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Marek Vasut]

Dear Stefano Babic,

> On 18/06/2012 17:43, Marek Vasut wrote:
> >> So ... what do you expect me to do? The register is called HW_DRAM_CTL29
> >> (very descriptive) ... do you want me to create a file full of
> >> HW_DRAM_CTLnn ?
> > 
> > Would adding such comment be enough to satisfy you? (In slightly more
> > polite manner I mean ... damn I'm loosing it, 2 days until exam :'-( ).
> > Sorry for being rough, Stefano.
> 
> Do not worry, it is normal before exam ;-)

This ... is not exam. This ... is HELL!!!

> Yes, of course comments will help !

Wait ... it isn't my fault, I didn't publish any code that uses the function 
yet, that encodes stuff in hexadecimal. So this patch can be applied as is. It's 
Fabio's patch that's at fault here (and I hereby dump all responsibility on 
Fabio, keh keh :-) ).

But anyway ... we're gonna have this used a lot and I'm not quite sure how to 
handle it. So if comment is enough (aka. I'm hereby adjusting HW_DRAM_CTL29 in 
order to set enabled rows and cols differently, refer to "14.8.27 in mx28 TRM"), 
let's go with it.

Later, I think we should rework the mem init part. It's there from bootlets and 
I think it's all shitty because of that. Well ... the whole SPL is shitty 
because it's from bootlets, but that's a longer run, out of scope of this 
discussion.

> Best regards,
> Stefano

Best regards,
Marek Vasut

[U-Boot] [PATCH] i.MX28: Add function to adjust memory parameters

[Stefano Babic]

On 30/05/2012 07:43, Marek Vasut wrote:
> This function can be overridden at run-time and allows implementors
> of new boards based on the i.MX28 chip to fine-tune the memory params.
> It is possible to write into the dram_vals array because when the SPL
> runs, it is located SRAM. Therefore the location is writable. There is
> no possibility of these data to be read-only.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> ---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================