arm: cm4008, cm41xx: set gd->ram_size in dram_init
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Quoted 54 lines · On Thursday 05 July 2012 16:48:22 you wrote:
Quoted 41 lines · Yann Vernier
Leave
dram_init_banksizeto set up the bank info data.ram_sizewas previously uninitialized. Also, generalize lowlevel assembly to support more RAM options.Signed-off-by Yann Vernier <[email protected]>---
Changes for v2:
- Update to useCONFIG_SYS_SDRAM_constants
- Update cm41xx also
- Map SDRAM to match configuration---
arch/arm/cpu/arm920t/ks8695/lowlevel_init.S | 8 +++----- board/cm4008/cm4008.c | 5 +++-- board/cm41xx/cm41xx.c | 5 +++-- include/configs/cm4008.h | 5 ++--- include/configs/cm41xx.h | 5 ++--- 5 files changed, 13 insertions(+), 15 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/arm920t/ks8695/lowlevel_init.Sindex e9f1227..df13de6 100644arch/arm/cpu/arm920t/ks8695/lowlevel_init.S +2
-5@@ -131,11 +131,9 @@ highflash: * before relocating, we have to setup RAM timing */ ldr r1, =(KS8695_IO_BASE+KS8695_SDRAM_CTRL0) -#if (PHYS_SDRAM_1_SIZE == 0x02000000) - ldr r2, =0x7fc0000e /* 32MB */ -#else - ldr r2, =0x3fc0000e /* 16MB */ -#endif + /* 8 column address bits, 4 banks, 32 bits data width */ + ldr r2,=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
Quoted 1 line
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Would be great to have a little documentation here why it is shifted that way, unfortunately I can not find a KS8695 spec in the net. Don't get me wrong it is ok to do so. I think it is way better than hard coding the values like before but would be great to understand why it is done that way.
Quoted 3 lines
Best regards
Andreas Bie?mann
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
My personal thought on the matter is that the existing code is rather heavy on magic numbers, ergo the comment once I figured out what the first ROM reconfiguration was even for.
The memory mapping registers allow you to reconfigure the address recognition for each memory region. The low 16 bits are not configurable, so you can only relocate to 64KiB aligned addresses. Bits 25-16 are configured as an inclusive range, with the top value in bits 31-22 of the register, and the bottom in bits 21-12. The lower bits configure things like SDRAM geometry or SRAM/ROM access times, with one value meaning disabled.
Come to think of it, I fear I've miscalculated where the high region actually is. That comment in lowlevel_init should read 48MB and 32MB, not 768 and 512. I'll fix that and insert a little more explanatory text.
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Dear Yann Vernier,
Quoted 2 lines · Yann Vernier
Quoted 1 line · On Thursday 05 July 2012 16:48:22 you wrote:
On 05.07.2012 14:11, Yann Vernier wrote:
<snip>
Quoted 17 lines
Quoted 17 lines
Quoted 4 lines
(quoted diff) +2
-0+ /* 8 column address bits, 4 banks, 32 bits data width */ + ldr r2,=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Would be great to have a little documentation here why it is shifted that way, unfortunately I can not find a KS8695 spec in the net. Don't get me wrong it is ok to do so. I think it is way better than hard coding the values like before but would be great to understand why it is done that way.
Best regards
Quoted 7 lines
Quoted 1 line
Andreas Bie?mann
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
With this information I understand your equation. I think you should not do it that way. You may solve the current state (all devices have 8 column, 4 bank and 32 bit). But one adding (well, if that will ever come ;) another board with different setting will get in trouble here and need to find another solution. Maybe more sophisticated by doing another equation.
I think a straight forward solution here would be to add another special define in the board config, write the magic number down there and maybe describe what the number stands for. You can then just use the previously defined value in start.S.
You may have a look for at91 lowlevel_init, there it is done that way.
Quoted 3 lines
My personal thought on the matter is that the existing code is rather heavy on magic numbers, ergo the comment once I figured out what the first ROM reconfiguration was even for.
Well, thats true.
Best regards
Andreas Bie?mann
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Quoted 20 lines · On Friday 06 July 2012 10:43:40 you wrote:
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Quoted 5 lines
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
With this information I understand your equation. I think you should not do it that way. You may solve the current state (all devices have 8 column, 4 bank and 32 bit). But one adding (well, if that will ever come ;) another board with different setting will get in trouble here and need to find another solution. Maybe more sophisticated by doing another equation.
Quoted 5 lines
I think a straight forward solution here would be to add another special define in the board config, write the magic number down there and maybe describe what the number stands for. You can then just use the previously defined value in
start.S.
You may have a look for at91lowlevel_init, there it is done that way.
I shall. In fact, I may already have this problem as I need to verify the timing on the flash memory (I have the demo board, not a cm4xxx), and the reason I work on this is that we may design another board soon.
I'm a little hesitant about how to fit these changes together, though; for now I have three barely separated patches, but overhauling all those magic numbers changes them all yet again. These three patches are all necessary to make my u-boot work in the first place, while fixing the magic values is a code cleanup change - probably editing arch/arm/include/asm/arch-ks8695/platform.h which currently contains a few meaningless constants like KS8695_SDRAM_START and a consistent misspelling of definitions.
Could I save the value decoding and corresponding configuration changes for a fourth patch?
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Dear Yann Vernier,
Quoted 39 lines · Yann Vernier
Quoted 20 lines · On Friday 06 July 2012 10:43:40 you wrote:
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Quoted 5 lines
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
With this information I understand your equation. I think you should not do it that way. You may solve the current state (all devices have 8 column, 4 bank and 32 bit). But one adding (well, if that will ever come ;) another board with different setting will get in trouble here and need to find another solution. Maybe more sophisticated by doing another equation.
Quoted 5 lines
I think a straight forward solution here would be to add another special define in the board config, write the magic number down there and maybe describe what the number stands for. You can then just use the previously defined value in
start.S.
You may have a look for at91lowlevel_init, there it is done that way.I shall. In fact, I may already have this problem as I need to verify the timing on the flash memory (I have the demo board, not a cm4xxx), and the reason I work on this is that we may design another board soon.
I'm a little hesitant about how to fit these changes together, though; for now I have three barely separated patches, but overhauling all those magic numbers changes them all yet again. These three patches are all necessary to make my u-boot work in the first place, while fixing the magic values is a code cleanup change - probably editing
arch/arm/include/asm/arch-ks8695/platform.hwhich currently contains a few meaningless constants likeKS8695_SDRAM_STARTand a consistent misspelling of definitions.
Quoted 2 lines
Could I save the value decoding and corresponding configuration changes for a fourth patch?
I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm related I guess Albert is the one in question.
I think these three patches are all fixes to get a board already in mainline working. So I think we should try to get these in -rc1. Albert, can you please comment?
Best regards
Andreas Bie?mann
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Hi Andreas,
Quoted 42 lines · Andreas Bie?mann
Dear Yann Vernier,
Quoted 39 lines · Yann Vernier
Quoted 20 lines · On Friday 06 July 2012 10:43:40 you wrote:
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Quoted 5 lines
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
With this information I understand your equation. I think you should not do it that way. You may solve the current state (all devices have 8 column, 4 bank and 32 bit). But one adding (well, if that will ever come ;) another board with different setting will get in trouble here and need to find another solution. Maybe more sophisticated by doing another equation.
Quoted 5 lines
I think a straight forward solution here would be to add another special define in the board config, write the magic number down there and maybe describe what the number stands for. You can then just use the previously defined value in
start.S.
You may have a look for at91lowlevel_init, there it is done that way.I shall. In fact, I may already have this problem as I need to verify the timing on the flash memory (I have the demo board, not a cm4xxx), and the reason I work on this is that we may design another board soon.
I'm a little hesitant about how to fit these changes together, though; for now I have three barely separated patches, but overhauling all those magic numbers changes them all yet again. These three patches are all necessary to make my u-boot work in the first place, while fixing the magic values is a code cleanup change - probably editing
arch/arm/include/asm/arch-ks8695/platform.hwhich currently contains a few meaningless constants likeKS8695_SDRAM_STARTand a consistent misspelling of definitions.
Quoted 6 lines
Quoted 2 lines
Could I save the value decoding and corresponding configuration changes for a fourth patch?
I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm related I guess Albert is the one in question.
Ah, board-related patches. :)
Quoted 3 lines
I think these three patches are all fixes to get a board already in mainline working. So I think we should try to get these in -rc1. Albert, can you please comment?
If the cm4008/cm41xx board maintainer (as per MAINTAINERS, this is Greg Ungerer, Cc:) green-lights it, then I'm ok for pulling it in.
Quoted 3 lines
Best regards
Andreas Bie?mann
Amicalement,
Signature · 1 line
Albert.
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Quoted 45 lines · Albert ARIBAUD
Hi Andreas,
Quoted 42 lines · Andreas Bie?mann
Dear Yann Vernier,
Quoted 39 lines · Yann Vernier
Quoted 20 lines · On Friday 06 July 2012 10:43:40 you wrote:
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
=((CONFIG_SYS_SDRAM_BASE+CONFIG_SYS_SDRAM_SIZE-0x10000)<<(22-16) | \ +
(CONFIG_SYS_SDRAM_BASE>>(16-12) | 0x00e))ugh ... magic. Will it work for every possible setting?
How about one settingCONFIG_SYS_SDRAM_BASE!=0x0? I the base address related to register content in any way?Quoted 5 lines
It's not a guarded secret, although it is far from obvious where to find it. One way is micrel.com - Products - Ethernet ICs / ARM based SOC's - HW Design Kit. Within that zip file, KS8695X_EVAL_HW_RV4.0_DP/RegDescription/KS8695X Register Description v1.1.pdf
With this information I understand your equation. I think you should not do it that way. You may solve the current state (all devices have 8 column, 4 bank and 32 bit). But one adding (well, if that will ever come ;) another board with different setting will get in trouble here and need to find another solution. Maybe more sophisticated by doing another equation.
Quoted 5 lines
I think a straight forward solution here would be to add another special define in the board config, write the magic number down there and maybe describe what the number stands for. You can then just use the previously defined value in
start.S.
You may have a look for at91lowlevel_init, there it is done that way.I shall. In fact, I may already have this problem as I need to verify the timing on the flash memory (I have the demo board, not a cm4xxx), and the reason I work on this is that we may design another board soon.
I'm a little hesitant about how to fit these changes together, though; for now I have three barely separated patches, but overhauling all those magic numbers changes them all yet again. These three patches are all necessary to make my u-boot work in the first place, while fixing the magic values is a code cleanup change - probably editing
arch/arm/include/asm/arch-ks8695/platform.hwhich currently contains a few meaningless constants likeKS8695_SDRAM_STARTand a consistent misspelling of definitions.
Quoted 11 lines
Quoted 6 lines
Quoted 2 lines
Could I save the value decoding and corresponding configuration changes for a fourth patch?
I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm related I guess Albert is the one in question.Ah, board-related patches. :)
Quoted 2 lines
I think these three patches are all fixes to get a board already in mainline working.
Fixes or improvements?
Quoted 5 lines
So I think we should try to get these in -rc1.
Quoted 1 line
Albert, can you please comment?
If the cm4008/cm41xx board maintainer (as per
MAINTAINERS, this is Greg Ungerer, Cc:) green-lights it, then I'm ok for pulling it in.
I have no problem with them. I have only done a visual inspection of the revised patches in the mail archives, and not tested them on the real hardware.
I would suggest defining the proper meanings of the remaining bits in the SDRAM setup registers that are still magic numbers, but that can be a future change.
Regards
Greg
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
Quoted 9 lines · Greg Ungerer
Quoted 8 lines · Albert ARIBAUD
Hi Andreas,
Quoted 4 lines · Andreas Bie?mann
Dear Yann Vernier,
Quoted 1 line · Yann Vernier
On Friday 06 July 2012 10:43:40 you wrote:
<snip>
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
Could I save the value decoding and corresponding configuration changes for a
fourth patch?I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm related I guess Albert is the one in question.
Quoted 6 lines
Quoted 4 lines
Ah, board-related patches. :)
Quoted 2 lines
I think these three patches are all fixes to get a board already in mainline working.
Fixes or improvements?
I think these are fixes cause the board seems to not working without these changes.
Quoted 7 lines
Quoted 6 lines
So I think we should try to get these in -rc1.
Quoted 1 line
Albert, can you please comment?
If the cm4008/cm41xx board maintainer (as per
MAINTAINERS, this is Greg Ungerer,
Cc:) green-lights it, then I'm ok for pulling it in.
Quoted 7 lines
I have no problem with them. I have only done a visual inspection of the revised patches in the mail archives, and not tested them on the real hardware.
I would suggest defining the proper meanings of the remaining bits in the SDRAM setup registers that are still magic numbers, but that can be a future change.
Definitely.
Best regards
Andreas Bie?mann
PATCH v2 1/1 arm: cm4008, cm41xx: set gd->ram_size in dram_init
In reply to Albert ARIBAUDDear Albert Aribaud,
Quoted 7 lines · Albert ARIBAUD
Hi Andreas,
Quoted 4 lines · Andreas Bie?mann
Dear Yann Vernier,
Quoted 1 line · Yann Vernier
On Friday 06 July 2012 10:43:40 you wrote:
<snip>
Quoted 7 lines
Quoted 6 lines
Quoted 2 lines
Could I save the value decoding and corresponding configuration changes for a fourth patch?
I'm fine with this suggestion.
So the next question is who would pull it in mainline? Since this is arm related I guess Albert is the one in question.
Quoted 8 lines
Ah, board-related patches. :)
Quoted 3 lines
I think these three patches are all fixes to get a board already in mainline working. So I think we should try to get these in -rc1. Albert, can you please comment?
If the cm4008/cm41xx board maintainer (as per
MAINTAINERS, this is Greg Ungerer, Cc:) green-lights it, then I'm ok for pulling it in.
Ouch, I should have added the board maintainer in cc earlier ... still a lot to learn for me.
Best regards
Andreas Bie?mann