[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Marek Vasut]

This reverts commit 69d26d09de1cb93e0a09ca71d9f0d41a66f0756a.

Apparently, this commit got mainline only because of OOT port and causes
breakage on board that is mainline. Revert.

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 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
index 4f62142..0d13537 100644
--- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
@@ -39,7 +39,7 @@ uint32_t dram_vals[] = {
 	0x00000000, 0x00000100, 0x00000000, 0x00000000,
 	0x00000000, 0x00000000, 0x00000000, 0x00000000,
 	0x00000000, 0x00000000, 0x00010101, 0x01010101,
-	0x000f0f01, 0x0f02010a, 0x00000000, 0x00010101,
+	0x000f0f01, 0x0f02020a, 0x00000000, 0x00010101,
 	0x00000100, 0x00000100, 0x00000000, 0x00000002,
 	0x01010000, 0x05060302, 0x06005003, 0x0a0000c8,
 	0x02009c40, 0x0000030c, 0x0036a609, 0x031a0612,
-- 
1.7.10

[U-Boot] [PATCH 2/3] M28: Scan only first 512 MB of DRAM to avoid memory wraparound

[Marek Vasut]

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>
---
 include/configs/m28evk.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/m28evk.h b/include/configs/m28evk.h
index 4016570..7e1661e 100644
--- a/include/configs/m28evk.h
+++ b/include/configs/m28evk.h
@@ -88,7 +88,7 @@
  */
 #define	CONFIG_NR_DRAM_BANKS		1		/* 1 bank of DRAM */
 #define	PHYS_SDRAM_1			0x40000000	/* Base address */
-#define	PHYS_SDRAM_1_SIZE		0x40000000	/* Max 1 GB RAM */
+#define	PHYS_SDRAM_1_SIZE		0x20000000	/* Max 512 MB RAM */
 #define	CONFIG_STACKSIZE		0x00010000	/* 128 KB stack */
 #define	CONFIG_SYS_MALLOC_LEN		0x00400000	/* 4 MB for malloc */
 #define	CONFIG_SYS_GBL_DATA_SIZE	128		/* Initial data */
-- 
1.7.10

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Marek Vasut]

This solves issues when larger amount of DRAM is used.

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 |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
index 0d13537..0b592f7 100644
--- a/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
+++ b/arch/arm/cpu/arm926ejs/mx28/spl_mem_init.c
@@ -237,4 +237,6 @@ void mx28_mem_init(void)
 	early_delay(10000);
 
 	mx28_mem_setup_cpu_and_hbus();
+
+	early_delay(10000);
 }
-- 
1.7.10

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Fabio Estevam]

On Wed, May 2, 2012 at 7:14 PM, Marek Vasut <marex@denx.de> wrote:
> This solves issues when larger amount of DRAM is used.

Shouldn't we check if we are using a "large amount of DRAM"?

If we don't check then even boards with small amount of RAM would have
this additional delay.

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Marek Vasut]

Dear Fabio Estevam,

> On Wed, May 2, 2012 at 7:14 PM, Marek Vasut <marex@denx.de> wrote:
> > This solves issues when larger amount of DRAM is used.
> 
> Shouldn't we check if we are using a "large amount of DRAM"?
> 
> If we don't check then even boards with small amount of RAM would have
> this additional delay.

I'm afraid this worked on boards with a small amound of RAM by sheer accident 
(or good will of the DRAM), time to play safe and fix this.

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Detlev Zundel]

Hi Marek,

> Dear Fabio Estevam,
>
>> On Wed, May 2, 2012 at 7:14 PM, Marek Vasut <marex@denx.de> wrote:
>> > This solves issues when larger amount of DRAM is used.
>> 
>> Shouldn't we check if we are using a "large amount of DRAM"?
>> 
>> If we don't check then even boards with small amount of RAM would have
>> this additional delay.
>
> I'm afraid this worked on boards with a small amound of RAM by sheer
> accident
> (or good will of the DRAM), time to play safe and fix this.

Can you please comment on why we are waiting and why we are waiting
exactly this long?  Can we maybe poll the needed time somehow?

Thanks
  Detlev

-- 
Less talking -- more hacking
                                        -- Olin Shivers
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Marek Vasut]

Dear Detlev Zundel,

> Hi Marek,
> 
> > Dear Fabio Estevam,
> > 
> >> On Wed, May 2, 2012 at 7:14 PM, Marek Vasut <marex@denx.de> wrote:
> >> > This solves issues when larger amount of DRAM is used.
> >> 
> >> Shouldn't we check if we are using a "large amount of DRAM"?
> >> 
> >> If we don't check then even boards with small amount of RAM would have
> >> this additional delay.
> > 
> > I'm afraid this worked on boards with a small amound of RAM by sheer
> > accident
> > (or good will of the DRAM), time to play safe and fix this.
> 
> Can you please comment on why we are waiting and why we are waiting
> exactly this long?  Can we maybe poll the needed time somehow?

We need to wait for the CPU clock to stabilize. I think there is a bit for that 
that we can poll, indeed. It's in my queue to revise the memory init code 
eventually actually, because I think I can make it even faster and fix some 
rough edges that are still there.

> 
> Thanks
>   Detlev

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Detlev Zundel]

Hi Marek,

[...]

>> Can you please comment on why we are waiting and why we are waiting
>> exactly this long?  Can we maybe poll the needed time somehow?
>
> We need to wait for the CPU clock to stabilize.

This would be a worthwhile comment in the code.

> I think there is a bit for that that we can poll, indeed.

And _that_ is even better ;)

> It's in my queue to revise the memory init code eventually actually,
> because I think I can make it even faster and fix some rough edges
> that are still there.

Thanks!
  Detlev

-- 
Man is a fool, and woman, for tolerating him, is a damned fool
                       -- Mark Twain
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Marek Vasut]

Dear Detlev Zundel,

> Hi Marek,
> 
> [...]
> 
> >> Can you please comment on why we are waiting and why we are waiting
> >> exactly this long?  Can we maybe poll the needed time somehow?
> > 
> > We need to wait for the CPU clock to stabilize.
> 
> This would be a worthwhile comment in the code.

Indeed, will do

> 
> > I think there is a bit for that that we can poll, indeed.
> 
> And _that_ is even better ;)

I'm not doing this in this patch, I want a stable implementation here and I can 
break it later ;-)

> 
> > It's in my queue to revise the memory init code eventually actually,
> > because I think I can make it even faster and fix some rough edges
> > that are still there.
> 
> Thanks!
>   Detlev

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Marek Vasut]

Dear Detlev Zundel,

> Hi Marek,
> 
> [...]
> 
> >> Can you please comment on why we are waiting and why we are waiting
> >> exactly this long?  Can we maybe poll the needed time somehow?
> > 
> > We need to wait for the CPU clock to stabilize.
> 
> This would be a worthwhile comment in the code.
> 
> > I think there is a bit for that that we can poll, indeed.
> 
> And _that_ is even better ;)

And I was wrong, there isn't ... I made a dip into the manual and there's 
nothing :/ So this patch will have to do

> 
> > It's in my queue to revise the memory init code eventually actually,
> > because I think I can make it even faster and fix some rough edges
> > that are still there.
> 
> Thanks!
>   Detlev

Best regards,
Marek Vasut

[U-Boot] [PATCH 3/3] i.MX28: Increase the delay after DRAM init

[Detlev Zundel]

Hi Marek,

> Dear Detlev Zundel,
>
>> Hi Marek,
>> 
>> [...]
>> 
>> >> Can you please comment on why we are waiting and why we are waiting
>> >> exactly this long?  Can we maybe poll the needed time somehow?
>> > 
>> > We need to wait for the CPU clock to stabilize.
>> 
>> This would be a worthwhile comment in the code.
>> 
>> > I think there is a bit for that that we can poll, indeed.
>> 
>> And _that_ is even better ;)
>
> And I was wrong, there isn't ... I made a dip into the manual and there's 
> nothing :/ So this patch will have to do

Ah, too bad.  Then at least comment on how you derived the delay and
what parameters where used for this calculation.

Thanks
  Detlev

-- 
... and I will continue to use a  low-level language to indicate how
machines actually compute.  Readers  who only want to see algorithms
that are already packaged in a plug-in way, using a trendy language,
should buy other people's books.
                                         -- Donald Knuth, Fascicle 1
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Detlev Zundel]

Hi Marek,

> This reverts commit 69d26d09de1cb93e0a09ca71d9f0d41a66f0756a.
>
> Apparently, this commit got mainline only because of OOT port and causes
> breakage on board that is mainline. Revert.

To be honest, I don't understand what this patch or what the original
patch did, nor through what OOT port this hit mainline and what breakage
it causes on other boards.

So also I can read the sentence, I cannot make heads and tails of it.
Can you please write commit messages that people like me can make some
sense from?  I.e. answering the following questions would help me

- What does the original commit do? (its too late to change the original
  commit) 
- Why was the change made in the first place and for what OOT port?
- What breakage is caused on what boards?
- Why can we revert the change without any problems?

Thanks
  Detlev

-- 
#!/usr/bin/perl -l
print ((1 x shift) !~ /^(11+?)\1+$/ ? "prime" : "not prime");
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Marek Vasut]

Dear Detlev Zundel,

> Hi Marek,
> 
> > This reverts commit 69d26d09de1cb93e0a09ca71d9f0d41a66f0756a.
> > 
> > Apparently, this commit got mainline only because of OOT port and causes
> > breakage on board that is mainline. Revert.
> 
> To be honest, I don't understand what this patch or what the original
> patch did, nor through what OOT port this hit mainline and what breakage
> it causes on other boards.

It enabled some additional address bits on X board, allowing it to use full 
512MB of RAM. But because we don't have any other such configured board and X 
board isn't mainline, I reverted this patch. It caused trouble on new 256MB 
configuration of M28.

Once there'll be some module that needs different memory configuration (like X 
board) mainline, we'll add this here, but until then, I'd like to stick with 
common memory init.

> 
> So also I can read the sentence, I cannot make heads and tails of it.
> Can you please write commit messages that people like me can make some
> sense from?  I.e. answering the following questions would help me
> 
> - What does the original commit do? (its too late to change the original
>   commit)

Enabled additional address bits, to address full 512 MB of DRAM on the X board.

> - Why was the change made in the first place and for what OOT port?

Change of a DRAM configuration register that enabled additional address bit, at 
address 512MB of DRAM. Though this caused memory hole on our M28 module with 
256MB of DRAM, which _is_ mainline. X board is OOT and never will be mainlined I 
guess.

> - What breakage is caused on what boards?

See above.

> - Why can we revert the change without any problems?

Because we don't have any mainline port that used this feature.

> 
> Thanks
>   Detlev

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Detlev Zundel]

Hi Marek,

[...]

>> - Why was the change made in the first place and for what OOT port?
>
> Change of a DRAM configuration register that enabled additional
> address bit, at address 512MB of DRAM. Though this caused memory hole
> on our M28 module with 256MB of DRAM, which _is_ mainline. X board is
> OOT and never will be mainlined I guess.

I still do not understand this fully.  What exactly is this "memory
hole" and why is it fatal?  As far as I can remember, there are always
some holes in the adress map, so why is this special?

Apart from that, I think most of these answers should go into the commit
message to understand what is happening.

Thanks in advance
  Detlev

-- 
Every generation laughs at the old fashions, but follows religiously
the new.
                                        -- Henry David Thoreau
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Marek Vasut]

Dear Detlev Zundel,

> Hi Marek,
> 
> [...]
> 
> >> - Why was the change made in the first place and for what OOT port?
> > 
> > Change of a DRAM configuration register that enabled additional
> > address bit, at address 512MB of DRAM. Though this caused memory hole
> > on our M28 module with 256MB of DRAM, which _is_ mainline. X board is
> > OOT and never will be mainlined I guess.
> 
> I still do not understand this fully.  What exactly is this "memory
> hole" and why is it fatal?  As far as I can remember, there are always
> some holes in the adress map, so why is this special?

No, this one created this layout on our 256 MB module:

[chunk of memory][<- same thing][chunk of memory][<- same thing]

so get_ram_size() didn't work with it and it actually overwrote part of the U-
Boot etc.

> 
> Apart from that, I think most of these answers should go into the commit
> message to understand what is happening.

Agreed

> Thanks in advance
>   Detlev

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"

[Detlev Zundel]

Hi Marek,

> Dear Detlev Zundel,
>
>> Hi Marek,
>> 
>> [...]
>> 
>> >> - Why was the change made in the first place and for what OOT port?
>> > 
>> > Change of a DRAM configuration register that enabled additional
>> > address bit, at address 512MB of DRAM. Though this caused memory hole
>> > on our M28 module with 256MB of DRAM, which _is_ mainline. X board is
>> > OOT and never will be mainlined I guess.
>> 
>> I still do not understand this fully.  What exactly is this "memory
>> hole" and why is it fatal?  As far as I can remember, there are always
>> some holes in the adress map, so why is this special?
>
> No, this one created this layout on our 256 MB module:
>
> [chunk of memory][<- same thing][chunk of memory][<- same thing]
>
> so get_ram_size() didn't work with it and it actually overwrote part
> of the U-
> Boot etc.

Ah, so it created what I would call a "memory mirroring" or "memory
aliasing", right?  Now I understand the problem, thanks.

Cheers
  Detlev

-- 
This is  not the first  time my views  on some topic have  inspired in
someone the  desire to psychoanalyze me. Previous  experience leads me
to ask  about your couch. Is  it comfortable? Are its  springs in good
shape?                                     -- Jonh McCarthy
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de