diff mbox

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

Message ID 1291361167-9880-1-git-send-email-biessmann@corscience.de
State Rejected
Delegated to: Reinhard Meyer
Headers show

Commit Message

Andreas Bießmann Dec. 3, 2010, 7:26 a.m. UTC
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(-)

Comments

Jens Scharsig Dec. 3, 2010, 5:09 p.m. UTC | #1
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
Andreas Bießmann Dec. 3, 2010, 9:22 p.m. UTC | #2
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
Jens Scharsig Dec. 4, 2010, 11:37 a.m. UTC | #3
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
Albert ARIBAUD Dec. 4, 2010, 12:04 p.m. UTC | #4
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,
Andreas Bießmann Dec. 4, 2010, 1:14 p.m. UTC | #5
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
Albert ARIBAUD Dec. 4, 2010, 6:18 p.m. UTC | #6
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,
diff mbox

Patch

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