[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers
For some reason the AT91rm9200 lowlevel init writes to a bunch of reserved or read-only addresses. All the boards seem to define the value-to-be-written values as zero ... but they shouldn't actually be writing *anything* there.
If there's a real need to write these locations, like an erratum that's not included in the current list, that should be reflected in a source code comment. Looks like maybe some very early BDI-2000 setup code has been carried along by cargo cult programming since at least late 2004 (per GIT history).
Meanwhile, here's a patch/RFC disabling what seems to be bogosity. If it's eventually a "go", the supporting code should be removed. --- cpu/arm920t/at91rm9200/lowlevel_init.S | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
--- a/cpu/arm920t/at91rm9200/lowlevel_init.S +++ b/cpu/arm920t/at91rm9200/lowlevel_init.S @@ -81,6 +81,7 @@ LoopOsc: bne 0b /* delay - this is all done by guess */ ldr r0, =0x00010000 + /* (vs reading PMC_SR for LOCKA, LOCKB ... or MOSCS earlier) */ 1: subs r0, r0, #1 bhi 1b @@ -108,16 +109,22 @@ LoopOsc: .ltorg
SMRDATA: +#if 0 +/* MC_PU* are reserved "do not use" memory controller addresses; + * see table 16-1 of the rm9200 manual + */ .word AT91C_MC_PUIA .word CONFIG_SYS_MC_PUIA_VAL .word AT91C_MC_PUP .word CONFIG_SYS_MC_PUP_VAL .word AT91C_MC_PUER .word CONFIG_SYS_MC_PUER_VAL +/* MC_ASR/AASR are read-only registers, see table 16-1 again */ .word AT91C_MC_ASR .word CONFIG_SYS_MC_ASR_VAL .word AT91C_MC_AASR .word CONFIG_SYS_MC_AASR_VAL +#endif .word AT91C_EBI_CFGR .word CONFIG_SYS_EBI_CFGR_VAL .word AT91C_SMC_CSR0 @@ -128,8 +135,7 @@ SMRDATA: .word CONFIG_SYS_PLLBR_VAL .word AT91C_MCKR .word CONFIG_SYS_MCKR_VAL - /* SMRDATA is 80 bytes long */ - /* here there's a delay of 100 */ + /* here there's a delay */ SMRDATA1: .word AT91C_PIOC_ASR .word CONFIG_SYS_PIOC_ASR_VAL
On Tuesday 09 June 2009, David Brownell wrote:
For some reason the AT91rm9200 lowlevel init writes to a bunch of reserved or read-only addresses. All the boards seem to define the value-to-be-written values as zero ... but they shouldn't actually be writing *anything* there.
Repeat: should not write to reserved or read-ony addresses.
If there's a real need to write these locations, like an erratum that's not included in the current list, that should be reflected in a source code comment. Looks like maybe some very early BDI-2000 setup code has been carried along by cargo cult programming since at least late 2004 (per GIT history).
Meanwhile, here's a patch/RFC disabling what seems to be bogosity. If it's eventually a "go", the supporting code should be removed.
No comments?
cpu/arm920t/at91rm9200/lowlevel_init.S | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
--- a/cpu/arm920t/at91rm9200/lowlevel_init.S +++ b/cpu/arm920t/at91rm9200/lowlevel_init.S @@ -81,6 +81,7 @@ LoopOsc: bne 0b /* delay - this is all done by guess */ ldr r0, =0x00010000
- /* (vs reading PMC_SR for LOCKA, LOCKB ... or MOSCS earlier) */
1: subs r0, r0, #1 bhi 1b @@ -108,16 +109,22 @@ LoopOsc: .ltorg
SMRDATA: +#if 0 +/* MC_PU* are reserved "do not use" memory controller addresses;
- see table 16-1 of the rm9200 manual
- */ .word AT91C_MC_PUIA .word CONFIG_SYS_MC_PUIA_VAL .word AT91C_MC_PUP .word CONFIG_SYS_MC_PUP_VAL .word AT91C_MC_PUER .word CONFIG_SYS_MC_PUER_VAL
+/* MC_ASR/AASR are read-only registers, see table 16-1 again */ .word AT91C_MC_ASR .word CONFIG_SYS_MC_ASR_VAL .word AT91C_MC_AASR .word CONFIG_SYS_MC_AASR_VAL +#endif .word AT91C_EBI_CFGR .word CONFIG_SYS_EBI_CFGR_VAL .word AT91C_SMC_CSR0 @@ -128,8 +135,7 @@ SMRDATA: .word CONFIG_SYS_PLLBR_VAL .word AT91C_MCKR .word CONFIG_SYS_MCKR_VAL
- /* SMRDATA is 80 bytes long */
- /* here there's a delay of 100 */
- /* here there's a delay */
SMRDATA1: .word AT91C_PIOC_ASR .word CONFIG_SYS_PIOC_ASR_VAL
Dear David Brownell,
In message 200906282241.54948.david-b@pacbell.net you wrote:
On Tuesday 09 June 2009, David Brownell wrote:
For some reason the AT91rm9200 lowlevel init writes to a bunch of reserved or read-only addresses. All the boards seem to define the value-to-be-written values as zero ... but they shouldn't actually be writing *anything* there.
Repeat: should not write to reserved or read-ony addresses.
If there's a real need to write these locations, like an erratum that's not included in the current list, that should be reflected in a source code comment. Looks like maybe some very early BDI-2000 setup code has been carried along by cargo cult programming since at least late 2004 (per GIT history).
Meanwhile, here's a patch/RFC disabling what seems to be bogosity. If it's eventually a "go", the supporting code should be removed.
No comments?
If no comments come for a patch-RFC for some days, you should try and re-post itas a real patch.
Best regards,
Wolfgang Denk
On Friday 10 July 2009, Wolfgang Denk wrote:
Dear David Brownell,
In message 200906282241.54948.david-b@pacbell.net you wrote:
On Tuesday 09 June 2009, David Brownell wrote:
Meanwhile, here's a patch/RFC disabling what seems to be bogosity. If it's eventually a "go", the supporting code should be removed.
No comments?
If no comments come for a patch-RFC for some days, you should try and re-post itas a real patch.
Thanks for the remminder ...
participants (2)
-
David Brownell -
Wolfgang Denk