[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

SMRDATA in a/a/c/arm920t/at91/lowlevel_init.c has wrong size. This patch
reduces it to correct size of 40 byte.

Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
---
Dear all,

I thougt about a problem in lowlevel_init() cause of errornous booting from
NOR flash on at91rm9200ek last night . This morning I detected this, but the
patch is untested!

I can not test it til sunday, if one can test it before please send a mail
(Jens?).

regards

Andreas Bie?mann

arch/arm/cpu/arm920t/at91/lowlevel_init.S |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
index eaea9d2..6e397c2 100644
--- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
+++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
@@ -65,7 +65,7 @@ LoopOsc:
 	ldr	r0, =SMRDATA
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #80
+	add	r2, r0, #40
 pllloop:
 	/* the address */
 	ldr	r1, [r0], #4
-- 
1.7.2.3

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Am 2010-12-03 08:26, schrieb Andreas Bie?mann:
> SMRDATA in a/a/c/arm920t/at91/lowlevel_init.c has wrong size. This patch
> reduces it to correct size of 40 byte.
> 
> Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
> ---
> Dear all,
> 
> I thougt about a problem in lowlevel_init() cause of errornous booting from
> NOR flash on at91rm9200ek last night . This morning I detected this, but the
> patch is untested!
> 
> I can not test it til sunday, if one can test it before please send a mail
> (Jens?).

Can you describe the symptoms?
I will test tomorrow?

regards Jens

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear Jens,

Am 03.12.2010 um 18:09 schrieb Jens Scharsig:

> Am 2010-12-03 08:26, schrieb Andreas Bie?mann:
>> SMRDATA in a/a/c/arm920t/at91/lowlevel_init.c has wrong size. This patch
>> reduces it to correct size of 40 byte.
>> 
>> Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
>> ---
>> Dear all,
>> 
>> I thougt about a problem in lowlevel_init() cause of errornous booting from
>> NOR flash on at91rm9200ek last night . This morning I detected this, but the
>> patch is untested!
>> 
>> I can not test it til sunday, if one can test it before please send a mail
>> (Jens?).
> 
> Can you describe the symptoms?

at91rm9200ek booting from NOR flash does not work with v2010.12-rc2 plus patchset 'get at91rm9200ek working with ARM relocation'. I can boot the at91rm9200ek_ram_config build with JTAG and confirm that no BSS values get written from beginning of board_init_f() til calling of relocate_code() cause of a modified version of the test-patch 'arm:board_init_f(): mirror BSS and check after each init_fnc()'.
Cause of the fact that NOR booting and 'SDRAM booting' via JTAG only differ in TEXT_BASE and SKIP_LOWLEVEL_INIT I thought there might be an error in lowlevel_init(). I did not find any statement in  a/a/c/arm920t/at91/lowlevel_init.c which may fail the boot, but I found out the SMRDATA section is wrongly handled.

The SMRDATA is a list of <ADDRESS>:<VALUE> which is handled in pllloop() to initially setup PMC and SMC. However the register setup just before pllloop() is wrong (SMRDATA is not 80 byte big, it is 40 byte!) and therefore we take some data from SMRDATA1 section (again address:value list for SDRAM setup). Cause of that we write some SDRAM configuration registers twice. I assume the wrong parameter does not break the setup (maybe it does ... thats to find out). Nevertheless the setup is wrong. Beside that the whole lowlevel_init() for arm920t/at91 is not really well coded, this could be done better ...

> I will test tomorrow?

It would be great if you can test that pllloop thing to indicate the wrong counter (80 instead of 40) did (or not) break anything. So the question is do we need that specific patch in v2010.12 to get arm920t/at91 boards working or is it a 'nice to have' for future releases, e.g. when we do a complete rework of the lowlevel_init() for arm920t/at91.

regards

Andreas Bie?mann

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Dear Andreas,

> It would be great if you can test that pllloop thing to indicate the wrong counter (80 instead of 40) did (or not) break anything. So the question is do we need that specific patch in v2010.12 to get arm920t/at91 boards working or is it a 'nice to have' for future releases, e.g. when we do a complete rework of the lowlevel_init() for arm920t/at91.
> 
I don't think that this will be fix the nor boot problems.
In my opinion, we have some SOC specific problems:

1. In start.s the vector table is relocated from _start to 0x00. But at this time 0x00 is remaped to nor flash.

2. On NOR boot we have no RAM at 0x00, until SRAM is remaped whit Remap Control Register. Nowhere fount in source.

3. On start NOR is mapped to address 0x0. So if we use 0x1000000 as textbase the board hangs after relocation.

4. I can't found any code to setup CS0 timings (use (slow) defaults only)

But, I can not check my thesis, until I get back my JTAG debugger end of next week.   	

regard

Jens Scharsig

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Albert ARIBAUD]

Le 04/12/2010 12:37, Jens Scharsig a ?crit :
> Dear Andreas,
>
>> It would be great if you can test that pllloop thing to indicate the wrong counter (80 instead of 40) did (or not) break anything. So the question is do we need that specific patch in v2010.12 to get arm920t/at91 boards working or is it a 'nice to have' for future releases, e.g. when we do a complete rework of the lowlevel_init() for arm920t/at91.
>>
> I don't think that this will be fix the nor boot problems.
> In my opinion, we have some SOC specific problems:
>
> 1. In start.s the vector table is relocated from _start to 0x00. But at this time 0x00 is remaped to nor flash.

Hmm... This copy (not a relocation) should be done after cpu_init_crit / 
lowlevel_init, which should have at least configured some RAM; but you 
are right that mapping to 0 is somewhat arbitrary, since the reset 
vectors could be some other places (0xffff0000, for instance).

> 2. On NOR boot we have no RAM at 0x00, until SRAM is remaped whit Remap Control Register. Nowhere fount in source.

Should be done in cpu_init_crit/lowlevel_init.

> 3. On start NOR is mapped to address 0x0. So if we use 0x1000000 as textbase the board hangs after relocation.

Hmm... Is the board forced to boot with NOR at 0? If it is then maybe 
you should consider mapping RAM at the top of the address space (I'm 
assuming that the exception vectors can map there). If it's not (e.g. 
boot pins allow a top/bottom placement of the NOR flash and the reset 
vector), you have the choice and can start with NOR at top and RAM at 
bottom.

> 4. I can't found any code to setup CS0 timings (use (slow) defaults only)
>
> But, I can not check my thesis, until I get back my JTAG debugger end of next week.
>
>  regard

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear all,

Am 04.12.2010 um 13:04 schrieb Albert ARIBAUD:

> Le 04/12/2010 12:37, Jens Scharsig a ?crit :
>> Dear Andreas,
>> 
>>> It would be great if you can test that pllloop thing to indicate the wrong counter (80 instead of 40) did (or not) break anything. So the question is do we need that specific patch in v2010.12 to get arm920t/at91 boards working or is it a 'nice to have' for future releases, e.g. when we do a complete rework of the lowlevel_init() for arm920t/at91.
>>> 
>> I don't think that this will be fix the nor boot problems.
>> In my opinion, we have some SOC specific problems:
>> 
>> 1. In start.s the vector table is relocated from _start to 0x00. But at this time 0x00 is remaped to nor flash.
> 
> Hmm... This copy (not a relocation) should be done after cpu_init_crit / 
> lowlevel_init, which should have at least configured some RAM; but you 
> are right that mapping to 0 is somewhat arbitrary, since the reset 
> vectors could be some other places (0xffff0000, for instance).

I think this is historic. I remind of the same sequence in an old atmel provided pre-loader code.
I think we can omit this and let the linker do the location of vector table, are we? Will have a look for that tomorrow evening.

>> 2. On NOR boot we have no RAM at 0x00, until SRAM is remaped whit Remap Control Register. Nowhere fount in source.
> 
> Should be done in cpu_init_crit/lowlevel_init.
No while booting with BMS pin set to NOR booting from CS0 the NOR flash mounted at CS0 is mapped to 0x0 ... this may really the problem with relocation cause.
Thanks for the input maybe i can test that tomorrow evening.

>> 3. On start NOR is mapped to address 0x0. So if we use 0x1000000 as textbase the board hangs after relocation.
> 
> Hmm... Is the board forced to boot with NOR at 0? If it is then maybe 
> you should consider mapping RAM at the top of the address space (I'm 
> assuming that the exception vectors can map there). If it's not (e.g. 
> boot pins allow a top/bottom placement of the NOR flash and the reset 
> vector), you have the choice and can start with NOR at top and RAM at 
> bottom.
> 
>> 4. I can't found any code to setup CS0 timings (use (slow) defaults only)

That is true, I guess the conservative boot-ROM provided values are set.

regards

Andreas Bie?mann

[U-Boot] [PATCH] at91rm9200: fix lowlevel_init() SMRDATA size

[Albert ARIBAUD]

Hi Andreas,

Le 04/12/2010 14:14, Andreas Bie?mann a ?crit :

>>> 1. In start.s the vector table is relocated from _start to 0x00. But at this time 0x00 is remaped to nor flash.
>>
>> Hmm... This copy (not a relocation) should be done after cpu_init_crit /
>> lowlevel_init, which should have at least configured some RAM; but you
>> are right that mapping to 0 is somewhat arbitrary, since the reset
>> vectors could be some other places (0xffff0000, for instance).
>
> I think this is historic. I remind of the same sequence in an old atmel provided pre-loader code.
> I think we can omit this and let the linker do the location of vector table, are we? Will have a look for that tomorrow evening.

Just a note: the linker will place the reset vector and some others at 
the location where reset actually occurs (hopefully), but the system, 
once started, may well redefine the *exception* vectors, for instance to 
get interrupts working (some boards use IRQs). On targets that boot from 
NOR, the system cannot overwrite the exception vectors at the link-time 
NOR location: it must move the vectors location to RAM. So no, the 
linker cannot always do the location of the vector table.

> regards
>
> Andreas Bie?mann

Amicalement,
-- 
Albert.

[U-Boot] [PATCH alternate version] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
---

Am 03.12.2010 08:26, schrieb Andreas Bie?mann:
> SMRDATA in a/a/c/arm920t/at91/lowlevel_init.c has wrong size. This patch
> reduces it to correct size of 40 byte.
> 
> Signed-off-by: Andreas Bie?mann <biessmann@corscience.de>
> ---
> Dear all,
> 
> I thougt about a problem in lowlevel_init() cause of errornous booting from
> NOR flash on at91rm9200ek last night . This morning I detected this, but the
> patch is untested!
> I can not test it til sunday, if one can test it before please send a mail
> (Jens?).
> 
> regards
> 
> Andreas Bie?mann
> 
> arch/arm/cpu/arm920t/at91/lowlevel_init.S |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> index eaea9d2..6e397c2 100644
> --- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> +++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> @@ -65,7 +65,7 @@ LoopOsc:
>  	ldr	r0, =SMRDATA
>  	ldr	r1, _MTEXT_BASE
>  	sub	r0, r0, r1
> -	add	r2, r0, #80
> +	add	r2, r0, #40
>  pllloop:
>  	/* the address */
>  	ldr	r1, [r0], #4

The patch has no positive or negative effects. But you are right, this is an error with
unknown side effects. 

So we should resolve this in a future-proof way (see below)

 arch/arm/cpu/arm920t/at91/lowlevel_init.S |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
index eaea9d2..1143c7e 100644
--- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
+++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
@@ -65,7 +65,8 @@ LoopOsc:
 	ldr	r0, =SMRDATA
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #80
+	ldr	r2, =SMRDATAE
+	add	r2, r2, r1
 pllloop:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -82,8 +83,8 @@ lock:
 	bhi	lock
 	ldr	r0, =SMRDATA1
 	ldr	r1, _MTEXT_BASE
-	sub	r0, r0, r1
-	add	r2, r0, #176
+	ldr	r2, =SMRDATA1E
+	add	r2, r2, r1
 sdinit:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -114,6 +115,7 @@ SMRDATA:
 	.word CONFIG_SYS_PLLBR_VAL
 	.word AT91_ASM_PMC_MCKR
 	.word CONFIG_SYS_MCKR_VAL
+SMRDATAE:	
 	/* here there's a delay */
 SMRDATA1:
 	.word AT91_ASM_PIOC_ASR
@@ -160,5 +162,6 @@ SMRDATA1:
 	.word CONFIG_SYS_SDRC_MR_VAL3
 	.word CONFIG_SYS_SDRAM
 	.word CONFIG_SYS_SDRAM_VAL
+SMRDATA1E:		
 	/* SMRDATA1 is 176 bytes long */
 #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
-- 1.7.1 

[U-Boot] [PATCH alternate version] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear Jens Scharsig,

Am 04.12.2010 um 11:53 schrieb Jens Scharsig:

[snip]

> So we should resolve this in a future-proof way (see below)
> 
> arch/arm/cpu/arm920t/at91/lowlevel_init.S |    9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> index eaea9d2..1143c7e 100644
> --- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> +++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> @@ -65,7 +65,8 @@ LoopOsc:
> 	ldr	r0, =SMRDATA
> 	ldr	r1, _MTEXT_BASE
> 	sub	r0, r0, r1
> -	add	r2, r0, #80
> +	ldr	r2, =SMRDATAE
> +	add	r2, r2, r1

Your approach seems better, therefore

Acked-by: Andreas Bie?mann <andreas.devel@googlemail.com>

regards

Andreas Bie?mann

[U-Boot] [PATCH alternate version] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear Jens Scharsig,

Am 04.12.2010 um 11:53 schrieb Jens Scharsig:

> diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> index eaea9d2..1143c7e 100644
> --- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> +++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
> @@ -65,7 +65,8 @@ LoopOsc:
> 	ldr	r0, =SMRDATA
> 	ldr	r1, _MTEXT_BASE
> 	sub	r0, r0, r1
> -	add	r2, r0, #80
> +	ldr	r2, =SMRDATAE
> +	add	r2, r2, r1
> pllloop:
> 	/* the address */
> 	ldr	r1, [r0], #4
> @@ -82,8 +83,8 @@ lock:
> 	bhi	lock
> 	ldr	r0, =SMRDATA1
> 	ldr	r1, _MTEXT_BASE
> -	sub	r0, r0, r1
> -	add	r2, r0, #176
> +	ldr	r2, =SMRDATA1E
> +	add	r2, r2, r1

OUCH, one line to much deleted ...

regards 

Andreas Bie?mann

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
---

Changes since first version:

* fix: remove on line to much (thanks Andreas Biessmann)
* fix: add _MTEXT_BASE instead of sub from table end

 arch/arm/cpu/arm920t/at91/lowlevel_init.S |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
index eaea9d2..b6a9164 100644
--- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
+++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
@@ -65,7 +65,8 @@ LoopOsc:
 	ldr	r0, =SMRDATA
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #80
+	ldr	r2, =SMRDATAE
+	sub	r2, r2, r1
 pllloop:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -83,7 +84,8 @@ lock:
 	ldr	r0, =SMRDATA1
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #176
+	ldr	r2, =SMRDATA1E
+	sub	r2, r2, r1
 sdinit:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -114,6 +116,7 @@ SMRDATA:
 	.word CONFIG_SYS_PLLBR_VAL
 	.word AT91_ASM_PMC_MCKR
 	.word CONFIG_SYS_MCKR_VAL
+SMRDATAE:	
 	/* here there's a delay */
 SMRDATA1:
 	.word AT91_ASM_PIOC_ASR
@@ -160,5 +163,6 @@ SMRDATA1:
 	.word CONFIG_SYS_SDRC_MR_VAL3
 	.word CONFIG_SYS_SDRAM
 	.word CONFIG_SYS_SDRAM_VAL
+SMRDATA1E:		
 	/* SMRDATA1 is 176 bytes long */
 #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
-- 1.7.1 

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Reinhard Meyer]

Dear Andreas, dear Jens,
> Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
> ---
> 
> Changes since first version:
> 
> * fix: remove on line to much (thanks Andreas Biessmann)
> * fix: add _MTEXT_BASE instead of sub from table end

Just tell me when a patch is "final" and ready to be applied :)

Best Regards,
Reinhard

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Dear Reinhard,
> 
> Just tell me when a patch is "final" and ready to be applied :)
> 

If Andreas can confirm the patch, I my case its final


regards Jens

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear all,

Am 06.12.2010 um 18:36 schrieb Jens Scharsig:

> Dear Reinhard,
>> 
>> Just tell me when a patch is "final" and ready to be applied :)
>> 
> 
> If Andreas can confirm the patch, I my case its final

Ouch ... it is completely wrong :(

See here ...

---8<---
> @@ -65,7 +65,8 @@ LoopOsc:
> 	ldr	r0, =SMRDATA
> 	ldr	r1, _MTEXT_BASE
> 	sub	r0, r0, r1
> -	add	r2, r0, #80
> +	ldr	r2, =SMRDATAE
> +	add	r2, r2, r1
--->8---

We do load the position of SMRDATA to r0 (which is absolute address of SMRDATA symbol/section however we call it), load the content of _MTEXT_BASE to r1 (which is same as CONFIG_TEXT_BASE).
Then we subtract TEXT_BASE from absolute address of SMRDATA. This lead to some value about 0x500 in r0 which is completely wrong which in turn leads to data abort. ... It would make sense to me to do some construct like this for relocated code, but here we are before relocation and therefore it would be sufficient to 

---8<---
ldr r0, =SMRDATA
ldr r2, =SMRDATAE
--->8---

I have step through the code with JTAG and at least the pll stuff should work correct, but didn't had time to test this in-depth, therefore no patch here.
The approach with begin/end marker is more robust than my first try to get lowlevel_init() fixed for arm920t/at91 but it seems it is completely broken.

More on lowlevel_init at end of the week, I have no time to look for it before. Therefore I do not see it for v2010.12. Jens, can you do some more on that before?

regards

Andreas Bie?mann

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Dear Andreas Bie?mann
> ---8<---
>> @@ -65,7 +65,8 @@ LoopOsc:
>> 	ldr	r0, =SMRDATA
>> 	ldr	r1, _MTEXT_BASE
>> 	sub	r0, r0, r1
>> -	add	r2, r0, #80
>> +	ldr	r2, =SMRDATAE
>> +	add	r2, r2, r1
> --->8---

Ok,this is wrong, But it is code from version 1. The V2 use the correct sub instruction.
> 

> Then we subtract TEXT_BASE from absolute address of SMRDATA. This lead to some value about 0x500 in r0 which is completely wrong which in turn leads to data abort. ... It would make sense to me to do some construct like this for relocated code, but here we are before relocation and therefore it would be sufficient to 

At boot time flash memory is mapded to 0x0. So the address 0x500 and 0x10000500 points the same location in flash.
> 

regards

Jens Scharsig

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Reinhard Meyer]

Dear Jens Scharsig, dear Andreas Bie?mann
>> ---8<---
>>> @@ -65,7 +65,8 @@ LoopOsc:
>>> 	ldr	r0, =SMRDATA
>>> 	ldr	r1, _MTEXT_BASE
>>> 	sub	r0, r0, r1
>>> -	add	r2, r0, #80
>>> +	ldr	r2, =SMRDATAE
>>> +	add	r2, r2, r1
>> --->8---
> 
> Ok,this is wrong, But it is code from version 1. The V2 use the correct sub instruction.
> 
>> Then we subtract TEXT_BASE from absolute address of SMRDATA. This lead to some value about 0x500 in r0 which is completely wrong which in turn leads to data abort. ... It would make sense to me to do some construct like this for relocated code, but here we are before relocation and therefore it would be sufficient to 
> 
> At boot time flash memory is mapded to 0x0. So the address 0x500 and 0x10000500 points the same location in flash.

Is there going to be a V3 soon? And please note, when "checkpatching"
V2 there are style problems:

> ERROR: trailing whitespace
> #56: FILE: arch/arm/cpu/arm920t/at91/lowlevel_init.S:119:
> +SMRDATAE:^I$
> 
> ERROR: trailing whitespace
> #64: FILE: arch/arm/cpu/arm920t/at91/lowlevel_init.S:166:
> +SMRDATA1E:^I^I$
> 
> total: 2 errors, 0 warnings, 31 lines checked

I find it a very valuable tool to do a ../linux-2.6/scripts/checkpatch.pl"
before sending the patch ;)

Best Regards,
Reinhard

[U-Boot] [PATCH alternate version V2] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear Jens Scharsig,

Am 12.12.2010 um 20:12 schrieb Jens Scharsig:

> Dear Andreas Bie?mann
>> ---8<---
>>> @@ -65,7 +65,8 @@ LoopOsc:
>>> 	ldr	r0, =SMRDATA
>>> 	ldr	r1, _MTEXT_BASE
>>> 	sub	r0, r0, r1
>>> -	add	r2, r0, #80
>>> +	ldr	r2, =SMRDATAE
>>> +	add	r2, r2, r1
>> --->8---
> 
> Ok,this is wrong, But it is code from version 1. The V2 use the correct sub instruction.

you are right, I copied the wrong part here.

>> Then we subtract TEXT_BASE from absolute address of SMRDATA. This lead to some value about 0x500 in r0 which is completely wrong which in turn leads to data abort. ... It would make sense to me to do some construct like this for relocated code, but here we are before relocation and therefore it would be sufficient to 
> 
> At boot time flash memory is mapded to 0x0. So the address 0x500 and 0x10000500 points the same location in flash.

That is correct. Sorry, I tested your patch with another textbase cause I can not get my openocd/arm-usb-tiny combo working with my at49bv6416 attached NOR flash.

regards

Andreas Bie?mann

[U-Boot] [PATCH alternate V3] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

 * use start/end label for initialization tables instead of fix values

Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>
---

I have trace through the code with JTAG. It should be work with 
CONFIG_TEXT_BASE = 0x0 and CONFIG_TEXT_BASE=0x10000000.

Changes since first version:

* fix: remove on line to much (thanks Andreas Biessmann)
* fix: add _MTEXT_BASE instead of sub from table end

Changes since 2nd version:

* remove trailing whitespace

 arch/arm/cpu/arm920t/at91/lowlevel_init.S |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/arm920t/at91/lowlevel_init.S b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
index eaea9d2..8b58ba9 100644
--- a/arch/arm/cpu/arm920t/at91/lowlevel_init.S
+++ b/arch/arm/cpu/arm920t/at91/lowlevel_init.S
@@ -65,7 +65,8 @@ LoopOsc:
 	ldr	r0, =SMRDATA
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #80
+	ldr	r2, =SMRDATAE
+	sub	r2, r2, r1
 pllloop:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -83,7 +84,8 @@ lock:
 	ldr	r0, =SMRDATA1
 	ldr	r1, _MTEXT_BASE
 	sub	r0, r0, r1
-	add	r2, r0, #176
+	ldr	r2, =SMRDATA1E
+	sub	r2, r2, r1
 sdinit:
 	/* the address */
 	ldr	r1, [r0], #4
@@ -114,6 +116,7 @@ SMRDATA:
 	.word CONFIG_SYS_PLLBR_VAL
 	.word AT91_ASM_PMC_MCKR
 	.word CONFIG_SYS_MCKR_VAL
+SMRDATAE:
 	/* here there's a delay */
 SMRDATA1:
 	.word AT91_ASM_PIOC_ASR
@@ -160,5 +163,6 @@ SMRDATA1:
 	.word CONFIG_SYS_SDRC_MR_VAL3
 	.word CONFIG_SYS_SDRAM
 	.word CONFIG_SYS_SDRAM_VAL
+SMRDATA1E:
 	/* SMRDATA1 is 176 bytes long */
 #endif /* CONFIG_SKIP_LOWLEVEL_INIT */
-- 1.7.1 

[U-Boot] [PATCH alternate V3] at91rm9200: fix lowlevel_init() SMRDATA size

[Andreas Bießmann]

Dear Jens Scharsig,

Am 18.12.2010 um 13:30 schrieb Jens Scharsig:

> * use start/end label for initialization tables instead of fix values
> 
> Signed-off-by: Jens Scharsig <js_at_ng@scharsoft.de>

Acked-by: Andreas Bie?mann <andreas.devel@googlemail.com>

Reinhard, please can you accept that patch?

regards

Andreas Bie?mann

[U-Boot] [PATCH alternate V3] at91rm9200: fix lowlevel_init() SMRDATA size

[Reinhard Meyer]

Dear Andreas Bie?mann,
> Dear Jens Scharsig,
>
> Am 18.12.2010 um 13:30 schrieb Jens Scharsig:
>
>> * use start/end label for initialization tables instead of fix values
>>
>> Signed-off-by: Jens Scharsig<js_at_ng@scharsoft.de>
>
> Acked-by: Andreas Bie?mann<andreas.devel@googlemail.com>
>
> Reinhard, please can you accept that patch?
I will. (I suppose this is final now?)

Thanks,
Reinhard

[U-Boot] [PATCH alternate V3] at91rm9200: fix lowlevel_init() SMRDATA size

[Jens Scharsig]

Dear Reinhard Meyer
> Dear Andreas Bie?mann,
>> Dear Jens Scharsig,
>>
>> Am 18.12.2010 um 13:30 schrieb Jens Scharsig:
>>
>>> * use start/end label for initialization tables instead of fix values
>>>
>>> Signed-off-by: Jens Scharsig<js_at_ng@scharsoft.de>
>>
>> Acked-by: Andreas Bie?mann<andreas.devel@googlemail.com>
>>
>> Reinhard, please can you accept that patch?
> I will. (I suppose this is final now?)

Yes, it's finial version.

We have curently only 2 boards (EB CPUx9K2 maintained by myself and
AT91RM92000EK by Andreas) which use new style at91rm9200 (arm929t) soc.
Both are tested.

Merry X-Mas

Jens

[U-Boot] [PATCH alternate V3] at91rm9200: fix lowlevel_init() SMRDATA size

[Reinhard Meyer]

Dear Jens Scharsig,
> Dear Reinhard Meyer
>> Dear Andreas Bie?mann,
>>> Dear Jens Scharsig,
>>>
>>> Am 18.12.2010 um 13:30 schrieb Jens Scharsig:
>>>
>>>> * use start/end label for initialization tables instead of fix values
>>>>
>>>> Signed-off-by: Jens Scharsig<js_at_ng@scharsoft.de>
>>>
>>> Acked-by: Andreas Bie?mann<andreas.devel@googlemail.com>
>>>
>>> Reinhard, please can you accept that patch?
>> I will. (I suppose this is final now?)
> 
> Yes, it's finial version.
> 
> We have curently only 2 boards (EB CPUx9K2 maintained by myself and
> AT91RM92000EK by Andreas) which use new style at91rm9200 (arm929t) soc.
> Both are tested.

Added to u-boot-atmel/next.

Thnaks,

Reinhard