[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers

[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers

[David Brownell]

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

[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers

[David Brownell]

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
> 

[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
NEW GRAND UNIFIED THEORY DISCLAIMER: The Manufacturer May Technically
Be Entitled to Claim That This Product Is  Ten-Dimensional.  However,
the  Consumer Is Reminded That This Confers No Legal Rights Above and
Beyond Those Applicable to Three-Dimensional Objects, Since the Seven
New Dimensions Are "Rolled Up" into Such a  Small  "Area"  That  They
Cannot Be Detected.

[U-Boot] [patch/rfc] rm9200 lowevel_init: don't touch reserved/readonly registers

[David Brownell]

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 ...