[U-Boot] [PATCH 1/3] Revert "i.MX28: Enable additional DRAM address bits"
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,
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 */
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); }
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.
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
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
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
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
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
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
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
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
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
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
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
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
participants (3)
-
Detlev Zundel -
Fabio Estevam -
Marek Vasut