[U-Boot] ARM relocation, probably trivial mistake

[U-Boot] ARM relocation, probably trivial mistake

[Reinhard Meyer]

Hello,

after enabling relocation I get the following output:

U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 16:57:43)

U-Boot code: 21F00000 -> 21F3C140  BSS: -> 21F7D700
CPU: AT91SAM9XE
Crystal frequency:   18.432 MHz
CPU clock        :  198.656 MHz
Master clock     :   99.328 MHz
I2C:   ready
monitor len: 0007D700 <<<< this must be wrong, the binary is short of 256k large
ramsize: 04000000 <<<< correct
Top of RAM usable for U-Boot at: 24000000 <<<< correct
Reserving 501k for U-Boot at: 23f82000
Reserving 143k for malloc() at: 23f5e100
Reserving 24 Bytes for Board Info at: 23f5e0e8
Reserving 88 Bytes for Global Data at: 23f5e090
New Stack Pointer is: 23f5e088
RAM Configuration:
Bank #0: 20000000 64 MiB <<< correct
relocation Offset is: 02082000 <<<< could be right

<<< system hangs here >>>

Details: initial bootstrap loads u-boot to RAM at 21f00000
(1MiB short of 32 MiB which is the minimum RAM populated)

I have set the following values:
#define	CONFIG_SKIP_LOWLEVEL_INIT
#define CONFIG_SKIP_RELOCATE_UBOOT
/*#define	CONFIG_SYS_ARM_WITHOUT_RELOC*/
#define	CONFIG_SYS_NO_ICACHE
#define	CONFIG_SYS_NO_DCACHE

and

/* SDRAM: min 32, upto 128 MB */
#define CONFIG_NR_DRAM_BANKS		1
#define PHYS_SDRAM			0x20000000
#define PHYS_SDRAM_MAX_SIZE		0x08000000
#define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM
#define CONFIG_SYS_MEMTEST_END		0x21e00000
#define CONFIG_SYS_LOAD_ADDR		(PHYS_SDRAM+0x01000000)	/* 16 MB into RAM */
#define	CONFIG_SYS_SDRAM_BASE		PHYS_SDRAM
#define	CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_SDRAM_BASE + 0x1000 - CONFIG_SYS_GBL_DATA_SIZE)

and in config.mk:

TEXT_BASE = 0x21f00000

and

int dram_init(void)
{
	gd->ram_size =
		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
	return 0;
}

void dram_init_banksize(void)
{
	gd->bd->bi_dram[0].start = PHYS_SDRAM;
	gd->bd->bi_dram[0].size =
		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
}

I do probably have a simple misunderstanding of the concept.

1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?

2. TEXT_BASE must be the address where the u-boot is loaded
by 1st loader and startet, right?

With Best Regards,
Reinhard

[U-Boot] ARM relocation, probably trivial mistake

[Stefano Babic]

Reinhard Meyer wrote:
> Hello,
> 

Hi Reinhard,

> int dram_init(void)
> {
> 	gd->ram_size =
> 		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
> 	return 0;
> }
> 
> void dram_init_banksize(void)
> {
> 	gd->bd->bi_dram[0].start = PHYS_SDRAM;
> 	gd->bd->bi_dram[0].size =
> 		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
> }

Do we need both functions ?. I do not add dram_init_banksize() because I
have as you only one bank, and the linker does not complain. However,
which dram_init_banksize is linked in your image ? Yours or the function
defined in arch/arm/lib/board.c ?

> 
> I do probably have a simple misunderstanding of the concept.
> 
> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?

I had the same issue because I forget to remove it from my config file.
After dropping it, I got the u-boot prompt again ;-)

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] ARM relocation, probably trivial mistake

[Reinhard Meyer]

Dear Stefano Babic,
>> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
> 
> I had the same issue because I forget to remove it from my config file.
> After dropping it, I got the u-boot prompt again ;-)
Thank you, that (partially) helped:

U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 17:32:36)

U-Boot code: 21F00000 -> 21F3C180  BSS: -> 21F7D700
CPU: AT91SAM9XE
Crystal frequency:   18.432 MHz
CPU clock        :  198.656 MHz
Master clock     :   99.328 MHz
I2C:   ready
monitor len: 0007D700
ramsize: 04000000
Top of RAM usable for U-Boot at: 24000000
Reserving 501k for U-Boot at: 23f82000
Reserving 143k for malloc() at: 23f5e100
Reserving 24 Bytes for Board Info at: 23f5e0e8
Reserving 88 Bytes for Global Data at: 23f5e090
New Stack Pointer is: 23f5e088
RAM Configuration:
Bank #0: 20000000 64 MiB
relocation Offset is: 02082000
monitor flash len: 0003C180
Now running in RAM - U-Boot at: 23f82000
FLASH: 512 KiB
NAND:  raise: Signal # 8 caught
raise: Signal # 8 caught
raise: Signal # 8 caught
256 MiB
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
MMC:   mci: 0
Net:   macb0, enc1.0, enc1.1, enc1.2
Reset Ethernet PHY
macb0: Starting autonegotiation...
macb0: Autonegotiation timed out (status=0x7849)
macb0: link down (status: 0x7849)
enc1.0: timeout waiting for CLKRDY
enc1.1: timeout waiting for CLKRDY
enc1.2: timeout waiting for CLKRDY
Press SPACE to abort autoboot in 1 seconds

Before the relocation: macb, enc and environment did work :)
I have to look into that issue later...

Best Regards,
Reinhard

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> Dear Stefano Babic,
>>> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
>> I had the same issue because I forget to remove it from my config file.
>> After dropping it, I got the u-boot prompt again ;-)
> Thank you, that (partially) helped:
> 
> U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 17:32:36)
> 
> U-Boot code: 21F00000 -> 21F3C180  BSS: -> 21F7D700
> CPU: AT91SAM9XE
> Crystal frequency:   18.432 MHz
> CPU clock        :  198.656 MHz
> Master clock     :   99.328 MHz
> I2C:   ready
> monitor len: 0007D700
> ramsize: 04000000
> Top of RAM usable for U-Boot at: 24000000
> Reserving 501k for U-Boot at: 23f82000
> Reserving 143k for malloc() at: 23f5e100
> Reserving 24 Bytes for Board Info at: 23f5e0e8
> Reserving 88 Bytes for Global Data at: 23f5e090
> New Stack Pointer is: 23f5e088
> RAM Configuration:
> Bank #0: 20000000 64 MiB
> relocation Offset is: 02082000
> monitor flash len: 0003C180
> Now running in RAM - U-Boot at: 23f82000
> FLASH: 512 KiB
> NAND:  raise: Signal # 8 caught
> raise: Signal # 8 caught
> raise: Signal # 8 caught
> 256 MiB
> *** Warning - bad CRC, using default environment
> 
> In:    serial
> Out:   serial
> Err:   serial
> MMC:   mci: 0
> Net:   macb0, enc1.0, enc1.1, enc1.2
> Reset Ethernet PHY
> macb0: Starting autonegotiation...
> macb0: Autonegotiation timed out (status=0x7849)
> macb0: link down (status: 0x7849)
> enc1.0: timeout waiting for CLKRDY
> enc1.1: timeout waiting for CLKRDY
> enc1.2: timeout waiting for CLKRDY
> Press SPACE to abort autoboot in 1 seconds
> 
> Before the relocation: macb, enc and environment did work :)
> I have to look into that issue later...

No spontaneous idea ... maybe something with gpio pins?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake

[Reinhard Meyer]

Dear Heiko Schocher,
>>
>> Before the relocation: macb, enc and environment did work :)
>> I have to look into that issue later...
> 
> No spontaneous idea ... maybe something with gpio pins?

The relocation should ideally not affect that ;)

I suspect the initiallization now does not call all functions
or in a different order...

I'll have to look deeper into that.

Reinhard

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> Hello,
> 
> after enabling relocation I get the following output:
> 
> U-Boot 2010.09-00106-g6e135b9-dirty (Sep 30 2010 - 16:57:43)
> 
> U-Boot code: 21F00000 -> 21F3C140  BSS: -> 21F7D700
> CPU: AT91SAM9XE
> Crystal frequency:   18.432 MHz
> CPU clock        :  198.656 MHz
> Master clock     :   99.328 MHz
> I2C:   ready
> monitor len: 0007D700 <<<< this must be wrong, the binary is short of 256k large
> ramsize: 04000000 <<<< correct
> Top of RAM usable for U-Boot at: 24000000 <<<< correct
> Reserving 501k for U-Boot at: 23f82000
> Reserving 143k for malloc() at: 23f5e100
> Reserving 24 Bytes for Board Info at: 23f5e0e8
> Reserving 88 Bytes for Global Data at: 23f5e090
> New Stack Pointer is: 23f5e088
> RAM Configuration:
> Bank #0: 20000000 64 MiB <<< correct
> relocation Offset is: 02082000 <<<< could be right
> 
> <<< system hangs here >>>
> 
> Details: initial bootstrap loads u-boot to RAM at 21f00000
> (1MiB short of 32 MiB which is the minimum RAM populated)
> 
> I have set the following values:
> #define	CONFIG_SKIP_LOWLEVEL_INIT
> #define CONFIG_SKIP_RELOCATE_UBOOT

With this, your TEXT_BASE must be == relocation address,
Probably this is not the case ...

> /*#define	CONFIG_SYS_ARM_WITHOUT_RELOC*/
> #define	CONFIG_SYS_NO_ICACHE
> #define	CONFIG_SYS_NO_DCACHE
> 
> and
> 
> /* SDRAM: min 32, upto 128 MB */
> #define CONFIG_NR_DRAM_BANKS		1
> #define PHYS_SDRAM			0x20000000
> #define PHYS_SDRAM_MAX_SIZE		0x08000000
> #define CONFIG_SYS_MEMTEST_START	PHYS_SDRAM
> #define CONFIG_SYS_MEMTEST_END		0x21e00000
> #define CONFIG_SYS_LOAD_ADDR		(PHYS_SDRAM+0x01000000)	/* 16 MB into RAM */
> #define	CONFIG_SYS_SDRAM_BASE		PHYS_SDRAM
> #define	CONFIG_SYS_INIT_SP_ADDR		(CONFIG_SYS_SDRAM_BASE + 0x1000 - CONFIG_SYS_GBL_DATA_SIZE)
> 
> and in config.mk:
> 
> TEXT_BASE = 0x21f00000
> 
> and
> 
> int dram_init(void)
> {
> 	gd->ram_size =
> 		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
> 	return 0;
> }
> 
> void dram_init_banksize(void)
> {
> 	gd->bd->bi_dram[0].start = PHYS_SDRAM;
> 	gd->bd->bi_dram[0].size =
> 		get_ram_size((void*)PHYS_SDRAM, PHYS_SDRAM_MAX_SIZE);
> }

This function is not necessary, because it is equal to the
default in arch/arm/lib/board.c

> I do probably have a simple misunderstanding of the concept.
> 
> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?

If TEXT_BASE == relocation address it could be defined,
never tested.

> 2. TEXT_BASE must be the address where the u-boot is loaded
> by 1st loader and startet, right?

Yep.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4CA4AEFF.3050101@denx.de> you wrote:
> 
> > 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
> 
> If TEXT_BASE == relocation address it could be defined,
> never tested.

We should point out that this is only intended (and allowed) for
boards where U-Boot gets loaded into RAM by some other means (like a
first stage boot loader, say when booting from NAND or SDCard).

When booting from NOR or some other persistent memory TEXT_BASE will
point to that address range, and then it is obviously impossible to
use the same value as relocation address.

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
A man is not complete until he is married -- then he is finished.

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <4CA4AEFF.3050101@denx.de> you wrote:
>>> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
>> If TEXT_BASE == relocation address it could be defined,
>> never tested.
> 
> We should point out that this is only intended (and allowed) for
> boards where U-Boot gets loaded into RAM by some other means (like a
> first stage boot loader, say when booting from NAND or SDCard).
> 
> When booting from NOR or some other persistent memory TEXT_BASE will
> point to that address range, and then it is obviously impossible to
> use the same value as relocation address.

Yes, you are right, that just work on boards, which use a preloader ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake

[Albert ARIBAUD]

Le 01/10/2010 07:25, Heiko Schocher a ?crit :
> Hello Wolfgang,
>
> Wolfgang Denk wrote:
>> Dear Heiko Schocher,
>>
>> In message<4CA4AEFF.3050101@denx.de>  you wrote:
>>>> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
>>> If TEXT_BASE == relocation address it could be defined,
>>> never tested.
>>
>> We should point out that this is only intended (and allowed) for
>> boards where U-Boot gets loaded into RAM by some other means (like a
>> first stage boot loader, say when booting from NAND or SDCard).
>>
>> When booting from NOR or some other persistent memory TEXT_BASE will
>> point to that address range, and then it is obviously impossible to
>> use the same value as relocation address.
>
> Yes, you are right, that just work on boards, which use a preloader ...
>
> bye,
> Heiko

Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all 
boards which use it are made to support relocation, as the code will 
always check if it runs at its intended location and relocate only if 
needed.

Of course, code which is loaded in RAM at a location different from its 
intended one (e.g., during a debugging session) will relocate -- which 
raises the interesting question of overlapping source and destination 
areas. That might of course be completely solved, if there is room 
enough in IRAM (or cache memory) by putting the relocating code there 
alongside initial stack, jumping to it from FLASH, and having it jump 
back to the RAM location of the rest of the code. But is it worth it?

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> Le 01/10/2010 07:25, Heiko Schocher a ?crit :
>> Hello Wolfgang,
>>
>> Wolfgang Denk wrote:
>>> Dear Heiko Schocher,
>>>
>>> In message<4CA4AEFF.3050101@denx.de>  you wrote:
>>>>> 1. should CONFIG_SKIP_RELOCATE_UBOOT be not defined anymore?
>>>> If TEXT_BASE == relocation address it could be defined,
>>>> never tested.
>>>
>>> We should point out that this is only intended (and allowed) for
>>> boards where U-Boot gets loaded into RAM by some other means (like a
>>> first stage boot loader, say when booting from NAND or SDCard).
>>>
>>> When booting from NOR or some other persistent memory TEXT_BASE will
>>> point to that address range, and then it is obviously impossible to
>>> use the same value as relocation address.
>>
>> Yes, you are right, that just work on boards, which use a preloader ...
>>
>> bye,
>> Heiko
> 
> Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all
> boards which use it are made to support relocation, as the code will
> always check if it runs at its intended location and relocate only if
> needed.

Yes, maybe we come to this solution.

> Of course, code which is loaded in RAM at a location different from its
> intended one (e.g., during a debugging session) will relocate -- which
> raises the interesting question of overlapping source and destination
> areas. That might of course be completely solved, if there is room
> enough in IRAM (or cache memory) by putting the relocating code there
> alongside initial stack, jumping to it from FLASH, and having it jump
> back to the RAM location of the rest of the code. But is it worth it?

When starting from Flash, this is no problem, because relocation code
runs from Flash, and copies u-boot to the relocation address in ram and
jumps to it. So no need to copy relocation code.

If u-boot starts in RAM, then it is the task from the preloader
where it copies u-boot code, and if there is somewhere in IRAM
enough room for it, this would be an option. Otherwise it is a
problem if relocation results in overlapping source and destination
areas ... but you will fast detect this problem, when you see,
that u-boot no longer works ;-)

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake

[Reinhard Meyer]

Dear Heiko Schocher,

>>>> In message<4CA4AEFF.3050101@denx.de>   you wrote:
>>>>> If TEXT_BASE == relocation address it could be defined,
>>>>> never tested.

If I think right, relocation address would vary with u-boot size,
so there seems no really reliable way to ensure above "if" is true.
Even worse, when it happens to become untrue because the size
changed, it will try to relocate overlapping areas.

>> Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all
>> boards which use it are made to support relocation, as the code will
>> always check if it runs at its intended location and relocate only if
>> needed.

"intended location" == "relocation address based on ram size" ??

> If u-boot starts in RAM, then it is the task from the preloader
> where it copies u-boot code, and if there is somewhere in IRAM
> enough room for it, this would be an option. Otherwise it is a
> problem if relocation results in overlapping source and destination
> areas ... but you will fast detect this problem, when you see,
> that u-boot no longer works ;-)

I think it would be safe that a preloader loads u-boot to the very
bottom of SDRAM. If the SDRAM is not at least double the size of
u-boot, u-boot needs a diet ;)

Reinhard

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> Dear Heiko Schocher,
> 
>>>>> In message<4CA4AEFF.3050101@denx.de>   you wrote:
>>>>>> If TEXT_BASE == relocation address it could be defined,
>>>>>> never tested.
> 
> If I think right, relocation address would vary with u-boot size,
> so there seems no really reliable way to ensure above "if" is true.
> Even worse, when it happens to become untrue because the size
> changed, it will try to relocate overlapping areas.

Yes, true, but you can with each release try your u-boot, and
eventually fix it ...

>>> Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all
>>> boards which use it are made to support relocation, as the code will
>>> always check if it runs at its intended location and relocate only if
>>> needed.
> 
> "intended location" == "relocation address based on ram size" ??

admitted, just a good idea for boards where ramsize is always fix ...

>> If u-boot starts in RAM, then it is the task from the preloader
>> where it copies u-boot code, and if there is somewhere in IRAM
>> enough room for it, this would be an option. Otherwise it is a
>> problem if relocation results in overlapping source and destination
>> areas ... but you will fast detect this problem, when you see,
>> that u-boot no longer works ;-)
> 
> I think it would be safe that a preloader loads u-boot to the very
> bottom of SDRAM. If the SDRAM is not at least double the size of
> u-boot, u-boot needs a diet ;)

Yep. Idea behind my "intended location == relocation address" is
to save one copy of u-boot (one from the preloader and one from
relocation) ... but this is not a need for all boards, this is
something a board maintainer can decide and use (for example, if
speed is a critical point) ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4CA58665.8080309@denx.de> you wrote:
> 
> > "intended location" == "relocation address based on ram size" ??
> 
> admitted, just a good idea for boards where ramsize is always fix ...

This is _not_ sufficient.

Assume PRAM support.

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
Its always easier short term to pee in the pond
than install a toilet - it's just not a good long term plan.
          - Alan Cox in <20100101145701.6432e7b7@lxorguk.ukuu.org.uk>

[U-Boot] ARM relocation, probably trivial mistake

[Albert ARIBAUD]

Le 01/10/2010 08:39, Reinhard Meyer a ?crit :

>>> Actually CONFIG_SKIP_RELICATE_UBOOT should become unneeded once all
>>> boards which use it are made to support relocation, as the code will
>>> always check if it runs at its intended location and relocate only if
>>> needed.
>
> "intended location" == "relocation address based on ram size" ??

Correct for those boards which support relocation. There are still a few 
which dont, and which still have a fixed "intended location" -- that 
should become less and less prevalent in the coming months.

>> If u-boot starts in RAM, then it is the task from the preloader
>> where it copies u-boot code, and if there is somewhere in IRAM
>> enough room for it, this would be an option. Otherwise it is a
>> problem if relocation results in overlapping source and destination
>> areas ... but you will fast detect this problem, when you see,
>> that u-boot no longer works ;-)
>
> I think it would be safe that a preloader loads u-boot to the very
> bottom of SDRAM. If the SDRAM is not at least double the size of
> u-boot, u-boot needs a diet ;)

If you make a requirement that the preloader put u-boot in the lowest 
possible RAM location, and if you rewrite the relocation code to move 
from end to start of the source and target locations, rather than from 
start to end, then it will work even for RAM sizes only slightly bigger 
than the u-boot size :) (and it will still work in boot-from-NOR cases).

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Hello,

it seems, that with relocation enabled, some
data does not seem to get initialized properly:

w/o relocation:

mmci
mci: setting clock 194000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 194000 Hz, block size 512
mci: setting clock 24832000 Hz, block size 512
Device: mci
Manufacturer ID: 89
OEM: 303
Name: NCard
Tran Speed: 25000000
Rd Block Len: 512
SD version 2.0
High Capacity: No
Capacity: 2006974464
Bus Width: 4-bit

with relocation:

mmci
mci: setting clock 0 Hz, block size 512
mci: setting clock 0 Hz, block size 512
mci: setting clock 0 Hz, block size 512
mci: setting clock 0 Hz, block size 512
mci: setting clock 0 Hz, block size 512
Device: mci
Manufacturer ID: 89
OEM: 303
Name: NCard
Tran Speed: 25000000
Rd Block Len: 512
SD version 2.0
High Capacity: No
Capacity: 2006974464
Bus Width: 4-bit

Do I understand right what the required changes are:

1. change dram_init
2. make sure TEXT_BASE is correct as to where u-boot is loaded
by a preloader and is NOT pointing near top of RAM.
3. I do not have a board specific .lds - should I now have one?

Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> it seems, that with relocation enabled, some
> data does not seem to get initialized properly:
>
> w/o relocation:
> 
> mmci
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 194000 Hz, block size 512
> mci: setting clock 24832000 Hz, block size 512
> Device: mci
> Manufacturer ID: 89
> OEM: 303
> Name: NCard
> Tran Speed: 25000000
> Rd Block Len: 512
> SD version 2.0
> High Capacity: No
> Capacity: 2006974464
> Bus Width: 4-bit
> 
> with relocation:
> 
> mmci
> mci: setting clock 0 Hz, block size 512

Hmm.. mabe something with at91_clock_init()

This is called in arch_cpu_init(), and
at the end, clocks are stored in

arch/arm/cpu/arm926ejs/at91/clock.c

in "static unsigned long" vars ... as this code
runs before relocation, this seems to me as it
could be the reason for your problems ... but I
can;t try it here ... can you check this?

> mci: setting clock 0 Hz, block size 512
> mci: setting clock 0 Hz, block size 512
> mci: setting clock 0 Hz, block size 512
> mci: setting clock 0 Hz, block size 512
> Device: mci
> Manufacturer ID: 89
> OEM: 303
> Name: NCard
> Tran Speed: 25000000
> Rd Block Len: 512
> SD version 2.0
> High Capacity: No
> Capacity: 2006974464
> Bus Width: 4-bit
> 
> Do I understand right what the required changes are:
> 
> 1. change dram_init

Yep.

> 2. make sure TEXT_BASE is correct as to where u-boot is loaded
> by a preloader and is NOT pointing near top of RAM.

Yep.

> 3. I do not have a board specific .lds - should I now have one?

No.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Heiko Schocher,

> Hmm.. mabe something with at91_clock_init()
>
> This is called in arch_cpu_init(), and
> at the end, clocks are stored in
>
> arch/arm/cpu/arm926ejs/at91/clock.c
>
> in "static unsigned long" vars ... as this code
> runs before relocation, this seems to me as it
> could be the reason for your problems ... but I
> can;t try it here ... can you check this?

Indeed, that is called before relocation, because
the clock frequencies are needed by almost all drivers,
including the timer.

I am unsure now how to fix this. I see two options:

1. calculate afresh every time a get_*_clk_rate() is
called - thus not needing static storage

2. store the values in bd->something (if it's permissible
to expand bd)

Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5BB7A.8050304@emk-elektronik.de> you wrote:
> 
> > in "static unsigned long" vars ... as this code
> > runs before relocation, this seems to me as it
> > could be the reason for your problems ... but I
> > can;t try it here ... can you check this?
> 
> Indeed, that is called before relocation, because
> the clock frequencies are needed by almost all drivers,
> including the timer.

...which is, and always has been, illegal.

> I am unsure now how to fix this. I see two options:
> 
> 1. calculate afresh every time a get_*_clk_rate() is
> called - thus not needing static storage
> 
> 2. store the values in bd->something (if it's permissible
> to expand bd)

Not bd-> but gd-> which was made for exactly that purpose.

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
EMACS belongs in <sys/errno.h>: Editor too big!

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,

> ...which is, and always has been, illegal.

<nitpick>
it became illegal once u-boot for AT91 became required to be relocated
</nitpick>

> Not bd->  but gd->  which was made for exactly that purpose.

typedef	struct	global_data...

I will try that. And fix the whitespace error as well...:)

Thanks,
Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5BFEF.3090208@emk-elektronik.de> you wrote:
> 
> <nitpick>
> it became illegal once u-boot for AT91 became required to be relocated
> </nitpick>

No, it has always been illegal. You might thave been lucky that in
your case the erros did not show up erarlier, but this does not change
anything.

Even very early versions of PPCBoot had this warning in the README:

	Only after this relocation will you have a "normal" C environment;
	until that you are restricted in several ways...


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
"I've seen it. It's rubbish."          - Marvin the Paranoid Android

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,
>> <nitpick>
>> it became illegal once u-boot for AT91 became required to be relocated
>> </nitpick>
> 
> No, it has always been illegal. You might thave been lucky that in
> your case the erros did not show up erarlier, but this does not change
> anything.

Sorry, before recently there was no relocation whatsoever used in
u-boot for AT91 architectures. So there was no before/after relocation.

The only complaint that can be put up is that someone introduced functions
using static data called from code that in other architectures runs before
relocation ;)

Whatever, I am not an advocate for Atmel, neither am I getting paid by them,
I am just a "user" that has to fix problems I did not expect to be there.

For the fix, I see an ugly multiline
#if defined(AT91SAM9260) || defined(AT91SAM9G20) || ...
coming into arch/arm/asm/global_data.h.

There is no common defined value for all AT91 SoCs that could be used.

Will that be ok?

Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5C7DE.6010300@emk-elektronik.de> you wrote:
>
> For the fix, I see an ugly multiline
> #if defined(AT91SAM9260) || defined(AT91SAM9G20) || ...
> coming into arch/arm/asm/global_data.h.
> 
> There is no common defined value for all AT91 SoCs that could be used.

If this is really for all AT91 SoCs, then please feel free to
introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually
you can clean up some other such #if's on the way.

Probably there is a header file common to all AT91 SoCs when then can
use a single such construct to #define the new variable so you don;t
have to touch all the many board config files.

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
A wise person makes his  own  decisions,  a  weak  one  obeys  public
opinion.                                           -- Chinese proverb

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,
>> For the fix, I see an ugly multiline
>> #if defined(AT91SAM9260) || defined(AT91SAM9G20) || ...
>> coming into arch/arm/asm/global_data.h.
>>
>> There is no common defined value for all AT91 SoCs that could be used.
> 
> If this is really for all AT91 SoCs, then please feel free to
> introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually
> you can clean up some other such #if's on the way.

That would have to be set either in each board.h file or in each at91*.h
file.

> Probably there is a header file common to all AT91 SoCs when then can
> use a single such construct to #define the new variable so you don;t
> have to touch all the many board config files.

The only files I can see included in each instance are those of the kind
hardware.h, memory_map.h and similiar. The define does not really belong
in any of those.

I did the changes of adding the clock values to gd, and it became somewhat
better, but there are still issues pending:

1. NAND accesses cause "raise: Signal # 8 caught"
but still work, kernel boots normally.
2. environment is still invalid - when I boot the
"CONFIG_SYS_ARM_WITHOUT_RELOC" version the ENV is ok.
3. saveenv goes way to fast to do anything and
i2c md 57 1000.2 100 shows no change
4. env default -f hangs the system

Q: after relocation, "gd->" values are still available, right?

I am searching for further "violations"...

U-Boot 2010.09-00106-g6e135b9-dirty (Oct 01 2010 - 15:13:14)

U-Boot code: 21F00000 -> 21F3C070  BSS: -> 21F7D600
CPU: AT91SAM9XE
Crystal frequency:   18.432 MHz
CPU clock        :  198.656 MHz
Master clock     :   99.328 MHz
I2C:   ready
monitor len: 0007D600
ramsize: 04000000
Top of RAM usable for U-Boot at: 24000000
Reserving 501k for U-Boot at: 23f82000
Reserving 143k for malloc() at: 23f5e100
Reserving 24 Bytes for Board Info at: 23f5e0e8
Reserving 112 Bytes for Global Data at: 23f5e078
  (112 bytes is exactly correct with the expanded struct)
New Stack Pointer is: 23f5e070
RAM Configuration:
Bank #0: 20000000 64 MiB
relocation Offset is: 02082000
monitor flash len: 0003C070
Now running in RAM - U-Boot at: 23f82000
FLASH: 512 KiB
NAND:  raise: Signal # 8 caught
raise: Signal # 8 caught
raise: Signal # 8 caught
256 MiB
*** Warning - bad CRC, using default environment

In:    serial
Out:   serial
Err:   serial
MMC:   mci: 0
Net:   macb0, enc1.0, enc1.1, enc1.2
Reset Ethernet PHY
macb0: Starting autonegotiation...
macb0: Autonegotiation complete
macb0: link up, 100Mbps full-duplex (lpa: 0x45e1)
Press SPACE to abort autoboot in 1 seconds
TOP9000> env
env - environment handling commands

Usage:
env ask name [message] [size] - ask for environment variable
env default -f - reset default environment
env edit name - edit environment variable
env export [-t | -b | -c] addr [size] - export environmnt
env import [-d] [-t | -b | -c] addr [size] - import environmnt
env print [name ...] - print environment
env run var [...] - run commands in an environment variable
env save - save environment
env set [-f] name [arg ...]

TOP9000> env default -f
(hangs here)

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

> I did the changes of adding the clock values to gd, and it became somewhat
> better, but there are still issues pending:
> 
> 1. NAND accesses cause "raise: Signal # 8 caught"
> but still work, kernel boots normally.
> 2. environment is still invalid - when I boot the
> "CONFIG_SYS_ARM_WITHOUT_RELOC" version the ENV is ok.
> 3. saveenv goes way to fast to do anything and
> i2c md 57 1000.2 100 shows no change
> 4. env default -f hangs the system
> 
> Q: after relocation, "gd->" values are still available, right?
> 
> I am searching for further "violations"...
Found them in timer.c, fixed them, now the NAND issue is gone.

The environment issues still persist. I am at a loss
there now.

Observation: the old style commands "setenv", "printenv", etc.
work, but any "env" command except for "env" alone crashes.

Hints where to look into are welcome, I will investige more
later or tomorrow.

Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5D857.5010009@emk-elektronik.de> you wrote:
>
> The environment issues still persist. I am at a loss
> there now.
> 
> Observation: the old style commands "setenv", "printenv", etc.
> work, but any "env" command except for "env" alone crashes.

OK. If "printenv" works and "env print" fails then it has nothing to
do with the environment code itself, as both call the same function.

It must have something to do with the implementation of subcommands
then. See do_env() in "common/cmd_nvedit.c"; check if the command
table address for find_cmd_tbl() is OK.

Eventually other commands with subcommands fail as well (i2c ?) ?

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
Es ist nicht genug zu wissen, man mu? auch anwenden; es ist nicht ge-
nug zu wollen, man mu? auch tun.   -- Goethe, Maximen und Reflexionen

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,
>> The environment issues still persist. I am at a loss
>> there now.
>>
>> Observation: the old style commands "setenv", "printenv", etc.
>> work, but any "env" command except for "env" alone crashes.
> 
> OK. If "printenv" works and "env print" fails then it has nothing to
> do with the environment code itself, as both call the same function.
> 
> It must have something to do with the implementation of subcommands
> then. See do_env() in "common/cmd_nvedit.c"; check if the command
> table address for find_cmd_tbl() is OK.
> 
> Eventually other commands with subcommands fail as well (i2c ?) ?

Nope, other commands with subcommands work fine. I did not test ALL
but mmc, i2c, nand behave normally.

I suspect something to do with a 256k boundary maybe, u-boot.bin is
short of 256k. I changed the initial bootstrap to load 496k
yesterday (before that the unrelocated version did fail in some
commands).

I'll keep investigating.

But the bad CRC issue must be something unrelated, or maybe not?

Best Regards,
Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> Dear Wolfgang Denk,
>>> The environment issues still persist. I am at a loss
>>> there now.
>>>
>>> Observation: the old style commands "setenv", "printenv", etc.
>>> work, but any "env" command except for "env" alone crashes.
>> OK. If "printenv" works and "env print" fails then it has nothing to
>> do with the environment code itself, as both call the same function.
>>
>> It must have something to do with the implementation of subcommands
>> then. See do_env() in "common/cmd_nvedit.c"; check if the command
>> table address for find_cmd_tbl() is OK.
>>
>> Eventually other commands with subcommands fail as well (i2c ?) ?
> 
> Nope, other commands with subcommands work fine. I did not test ALL
> but mmc, i2c, nand behave normally.

nand and mmc don;t use a command subtable, so they work. i2c do
the needed fixups for subcommandtables ...

And I see in common/cmd_nvedit.c there is a "static cmd_tbl_t cmd_env_sub"
definition, so this must be fixed too (not only for arm, it should
also don;t work on avr32, mips, m68k and sparc, because this
plattforms also need relocation fixups because CONFIG_RELOC_FIXUP_WORKS
is not defined for them ...

> I suspect something to do with a 256k boundary maybe, u-boot.bin is
> short of 256k. I changed the initial bootstrap to load 496k
> yesterday (before that the unrelocated version did fail in some
> commands).
> 
> I'll keep investigating.

Please try my above idea ...

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Steve Sakoman]

On Fri, Oct 1, 2010 at 5:55 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Reinhard Meyer,
>
> In message <4CA5D857.5010009@emk-elektronik.de> you wrote:
>>
>> The environment issues still persist. I am at a loss
>> there now.
>>
>> Observation: the old style commands "setenv", "printenv", etc.
>> work, but any "env" command except for "env" alone crashes.
>
> OK. If "printenv" works and "env print" fails then it has nothing to
> do with the environment code itself, as both call the same function.

I am seeing the same thing on Overo with current top of tree plus my
relocation patch.  Things like "env print" and "env save" just hang.
Perhaps it is a more general problem . . .

Steve

[U-Boot] ARM relocation, question to Heiko

[Reinhard Meyer]

Hello Heiko,

I try to understand how the relocation process could handle pointers (to
functions or other data) in const or data sections.
Your code cannot know what is data and what is a pointer that needs
adjustment?

Best Regards,
Reinhard

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 02/10/2010 09:15, Reinhard Meyer a ?crit :
> Hello Heiko,
>
> I try to understand how the relocation process could handle pointers (to
> functions or other data) in const or data sections.
> Your code cannot know what is data and what is a pointer that needs
> adjustment?
>
> Best Regards,
> Reinhard

Hi Reinhart,

Short answer - the relocation process does not handle pointers inside 
data structures.

And yes, this means the content arrays of pointers such as init_sequence 
is not relocated. Been there, done that, can give you one of the 
tee-shirts I got :)

ATM I have not found a way to fix this, except making the code which 
uses the pointers aware that the are location-sensitive and fix them 
when using them.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Reinhard Meyer]

Dear Albert ARIBAUD,
>> I try to understand how the relocation process could handle pointers (to
>> functions or other data) in const or data sections.
>> Your code cannot know what is data and what is a pointer that needs
>> adjustment?
>>
>> Best Regards,
>> Reinhard
>
> Hi Reinhart,
>
> Short answer - the relocation process does not handle pointers inside
> data structures.
>
> And yes, this means the content arrays of pointers such as init_sequence
> is not relocated. Been there, done that, can give you one of the
> tee-shirts I got :)
>
> ATM I have not found a way to fix this, except making the code which
> uses the pointers aware that the are location-sensitive and fix them
> when using them.

That means that things like this cannot work (with relocation),
unless adding the relocation offset before using the pointer:

const struct {
	const u8 shift;
	const u8 idcode;
	struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
} flashes[] = {
#ifdef CONFIG_SPI_FLASH_SPANSION
	{ 0, 0x01, spi_flash_probe_spansion, },
#endif
#ifdef CONFIG_SPI_FLASH_ATMEL
	{ 0, 0x1F, spi_flash_probe_atmel, },
#endif
#ifdef CONFIG_SPI_FLASH_MACRONIX
	{ 0, 0xc2, spi_flash_probe_macronix, },
#endif
#ifdef CONFIG_SPI_FLASH_WINBOND
	{ 0, 0xef, spi_flash_probe_winbond, },
#endif
#ifdef CONFIG_SPI_FLASH_STMICRO
	{ 0, 0x20, spi_flash_probe_stmicro, },
	{ 0, 0xff, spi_flash_probe_stmicro, },
#endif
#ifdef CONFIG_SPI_FLASH_SST
	{ 0, 0xBF, spi_flash_probe_sst, },
#endif
#ifdef CONFIG_SPI_FRAM_RAMTRON
	{ 6, 0xc2, spi_fram_probe_ramtron, },
# ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
	{ 0, 0xff, spi_fram_probe_ramtron, },
# endif
# undef IDBUF_LEN
# define IDBUF_LEN 9 /* we need to read 6+3 bytes */
#endif
};

And I think there are more places of this type in u-boot...

Best Regards,
Reinhard

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 02/10/2010 10:10, Reinhard Meyer a ?crit :
> Dear Albert ARIBAUD,
>>> I try to understand how the relocation process could handle pointers (to
>>> functions or other data) in const or data sections.
>>> Your code cannot know what is data and what is a pointer that needs
>>> adjustment?
>>>
>>> Best Regards,
>>> Reinhard
>>
>> Hi Reinhart,
>>
>> Short answer - the relocation process does not handle pointers inside
>> data structures.
>>
>> And yes, this means the content arrays of pointers such as init_sequence
>> is not relocated. Been there, done that, can give you one of the
>> tee-shirts I got :)
>>
>> ATM I have not found a way to fix this, except making the code which
>> uses the pointers aware that the are location-sensitive and fix them
>> when using them.
>
> That means that things like this cannot work (with relocation),
> unless adding the relocation offset before using the pointer:
>
> const struct {
> const u8 shift;
> const u8 idcode;
> struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
> } flashes[] = {
> #ifdef CONFIG_SPI_FLASH_SPANSION
> { 0, 0x01, spi_flash_probe_spansion, },
> #endif
> #ifdef CONFIG_SPI_FLASH_ATMEL
> { 0, 0x1F, spi_flash_probe_atmel, },
> #endif
> #ifdef CONFIG_SPI_FLASH_MACRONIX
> { 0, 0xc2, spi_flash_probe_macronix, },
> #endif
> #ifdef CONFIG_SPI_FLASH_WINBOND
> { 0, 0xef, spi_flash_probe_winbond, },
> #endif
> #ifdef CONFIG_SPI_FLASH_STMICRO
> { 0, 0x20, spi_flash_probe_stmicro, },
> { 0, 0xff, spi_flash_probe_stmicro, },
> #endif
> #ifdef CONFIG_SPI_FLASH_SST
> { 0, 0xBF, spi_flash_probe_sst, },
> #endif
> #ifdef CONFIG_SPI_FRAM_RAMTRON
> { 6, 0xc2, spi_fram_probe_ramtron, },
> # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
> { 0, 0xff, spi_fram_probe_ramtron, },
> # endif
> # undef IDBUF_LEN
> # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
> #endif
> };
>
> And I think there are more places of this type in u-boot...
>
> Best Regards,
> Reinhard

If this code is intended to execute after relocation [1] then no, it 
will not work.

There are two ways to fix that:

The first one is to make the variable non-const and, after relocation 
but before use, run a fixup loop specifically for this variable. Then 
you can call the (now fixed) functions.

The second one is to fix on-the-fly: provide a field in gd which 
contains the relocation offset in gd (if not done already); in the code 
which calls function pointers, DECLARE_GLOBAL_DATA_PTR and call the 
function through a global macro (defined in some general u-boot header), 
e.g. FIX_RELOCATED_FUNCTION_POINTER(fp), which would offset fp to its 
correct location.

Thus in the code, instead of x = fp(args) you'd have x = 
FIX_RELOCATED_FUNCTION_POINTER(fp)(args).

[1] or, in my case, before relocation but not from the location 
specified at link time. This is a slightly different issue, which the 
first solution fails to address but the second does.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4CA6EC99.5080204@free.fr> you wrote:
>
> There are two ways to fix that:
...

We should find the third way, which is that the tools actually
incude these pointers into the GOT so they get relocated
automatically.

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
There's another way to survive.  Mutual trust -- and help.
	-- Kirk, "Day of the Dove", stardate unknown

[U-Boot] ARM relocation, question to Heiko

[Heiko Schocher]

Hello Reinhard,

Reinhard Meyer wrote:
> Dear Albert ARIBAUD,
>>> I try to understand how the relocation process could handle pointers (to
>>> functions or other data) in const or data sections.
>>> Your code cannot know what is data and what is a pointer that needs
>>> adjustment?
>>>
>>> Best Regards,
>>> Reinhard
>> Hi Reinhart,
>>
>> Short answer - the relocation process does not handle pointers inside
>> data structures.
>>
>> And yes, this means the content arrays of pointers such as init_sequence
>> is not relocated. Been there, done that, can give you one of the

The init_sequence should not called anymore after relocation, as it is
the init_sequence ... or?

>> tee-shirts I got :)
>>
>> ATM I have not found a way to fix this, except making the code which
>> uses the pointers aware that the are location-sensitive and fix them
>> when using them.
> 
> That means that things like this cannot work (with relocation),
> unless adding the relocation offset before using the pointer:

Yep, you have to fix these pointers after relocation ...

> const struct {
> 	const u8 shift;
> 	const u8 idcode;
> 	struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
> } flashes[] = {
> #ifdef CONFIG_SPI_FLASH_SPANSION
> 	{ 0, 0x01, spi_flash_probe_spansion, },
> #endif
[...]
> #ifdef CONFIG_SPI_FRAM_RAMTRON
> 	{ 6, 0xc2, spi_fram_probe_ramtron, },
> # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
> 	{ 0, 0xff, spi_fram_probe_ramtron, },
> # endif
> # undef IDBUF_LEN
> # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
> #endif
> };
> 
> And I think there are more places of this type in u-boot...

Yes, maybe. But relocation as I did for arm, also works
on m68k, sparc, mips, avr32 and they must do also this
fixups, so for common functions (except the new env handling,
which I think got never tested on this architectures?) should
work ...

As I just searching in code: there is a env_relocate()
function (which get called from arch/arm/lib/board.c board_init_r()),
but it did not the necessary work for subcommandtable fixup...
I think this should be the right place to do this ... or?

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 02/10/2010 11:08, Heiko Schocher a ?crit :

>>> Short answer - the relocation process does not handle pointers inside
>>> data structures.
>>>
>>> And yes, this means the content arrays of pointers such as init_sequence
>>> is not relocated. Been there, done that, can give you one of the
>
> The init_sequence should not called anymore after relocation, as it is
> the init_sequence ... or?

... or you may want to have an u-boot binary which is truly 
position-independent. I'd like to have that, but the init_sequence table 
issue makes it difficult.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4CA6FB7E.3070009@free.fr> you wrote:
> 
> >>> Short answer - the relocation process does not handle pointers inside
> >>> data structures.
> >>>
> >>> And yes, this means the content arrays of pointers such as init_sequence
> >>> is not relocated. Been there, done that, can give you one of the
> >
> > The init_sequence should not called anymore after relocation, as it is
> > the init_sequence ... or?
> 
> ... or you may want to have an u-boot binary which is truly =
> position-independent. I'd like to have that, but the init_sequence table =
> issue makes it difficult.

See previous thread by Pter Tyser.

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
Prepare for tomorrow -- get ready.
	-- Edith Keeler, "The City On the Edge of Forever",
	   stardate unknown

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

> Hello Reinhard,
>
> Reinhard Meyer wrote:
> > Dear Albert ARIBAUD,
> >>> I try to understand how the relocation process could handle pointers (to
> >>> functions or other data) in const or data sections.
> >>> Your code cannot know what is data and what is a pointer that needs
> >>> adjustment?
> >>>
> >>> Best Regards,
> >>> Reinhard
> >> Hi Reinhart,
> >>
> >> Short answer - the relocation process does not handle pointers inside
> >> data structures.
> >>
> >> And yes, this means the content arrays of pointers such as init_sequence
> >> is not relocated. Been there, done that, can give you one of the
>
> The init_sequence should not called anymore after relocation, as it is
> the init_sequence ... or?
>
> >> tee-shirts I got :)
> >>
> >> ATM I have not found a way to fix this, except making the code which
> >> uses the pointers aware that the are location-sensitive and fix them
> >> when using them.
> >
> > That means that things like this cannot work (with relocation),
> > unless adding the relocation offset before using the pointer:
>
> Yep, you have to fix these pointers after relocation ...
>
> > const struct {
> >    const u8 shift;
> >    const u8 idcode;
> >    struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
> > } flashes[] = {
> > #ifdef CONFIG_SPI_FLASH_SPANSION
> >    { 0, 0x01, spi_flash_probe_spansion, },
> > #endif
> [...]
> > #ifdef CONFIG_SPI_FRAM_RAMTRON
> >    { 6, 0xc2, spi_fram_probe_ramtron, },
> > # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
> >    { 0, 0xff, spi_fram_probe_ramtron, },
> > # endif
> > # undef IDBUF_LEN
> > # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
> > #endif
> > };
> >
> > And I think there are more places of this type in u-boot...
>
> Yes, maybe. But relocation as I did for arm, also works
> on m68k, sparc, mips, avr32 and they must do also this
> fixups, so for common functions (except the new env handling,
> which I think got never tested on this architectures?) should
> work ...

This pointer problem is solved with the fixup relocs on ppc and
should work without manual relocation. I think this is a ppc
only extension but I might be wrong.
I believe that the other alternative is to do it as x86 does
which I think is the general way which should work on any arch.
Graem Russ would know better.

 Jocke

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/2/2010 3:17 AM, Joakim Tjernlund wrote:
>> Hello Reinhard,
>>
>> Reinhard Meyer wrote:
>>> Dear Albert ARIBAUD,
>>>>> I try to understand how the relocation process could handle pointers (to
>>>>> functions or other data) in const or data sections.
>>>>> Your code cannot know what is data and what is a pointer that needs
>>>>> adjustment?
>>>>>
>>>>> Best Regards,
>>>>> Reinhard
>>>> Hi Reinhart,
>>>>
>>>> Short answer - the relocation process does not handle pointers inside
>>>> data structures.
>>>>
>>>> And yes, this means the content arrays of pointers such as init_sequence
>>>> is not relocated. Been there, done that, can give you one of the
>> The init_sequence should not called anymore after relocation, as it is
>> the init_sequence ... or?
>>
>>>> tee-shirts I got :)
>>>>
>>>> ATM I have not found a way to fix this, except making the code which
>>>> uses the pointers aware that the are location-sensitive and fix them
>>>> when using them.
>>> That means that things like this cannot work (with relocation),
>>> unless adding the relocation offset before using the pointer:
>> Yep, you have to fix these pointers after relocation ...
>>
>>> const struct {
>>>     const u8 shift;
>>>     const u8 idcode;
>>>     struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
>>> } flashes[] = {
>>> #ifdef CONFIG_SPI_FLASH_SPANSION
>>>     { 0, 0x01, spi_flash_probe_spansion, },
>>> #endif
>> [...]
>>> #ifdef CONFIG_SPI_FRAM_RAMTRON
>>>     { 6, 0xc2, spi_fram_probe_ramtron, },
>>> # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
>>>     { 0, 0xff, spi_fram_probe_ramtron, },
>>> # endif
>>> # undef IDBUF_LEN
>>> # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
>>> #endif
>>> };
>>>
>>> And I think there are more places of this type in u-boot...
>> Yes, maybe. But relocation as I did for arm, also works
>> on m68k, sparc, mips, avr32 and they must do also this
>> fixups, so for common functions (except the new env handling,
>> which I think got never tested on this architectures?) should
>> work ...
> This pointer problem is solved with the fixup relocs on ppc and
> should work without manual relocation. I think this is a ppc
> only extension but I might be wrong.

Hi All,
       You are correct that this is a ppc only extension. As such, it is 
not a good candidate for "general" use.

> I believe that the other alternative is to do it as x86 does
> which I think is the general way which should work on any arch.
> Graem Russ would know better.
>
Almost exactly a year ago, this was all pretty much presented by Graeme 
in the threads
Relocation size penalty calculation (October 14, 2009)
i386 Relocation (November 24, 2009)

Using the full relocation scheme eliminates the need for all these 
"fixups" in u-boot C code. I think this is a very desirable result.
It is also not clear to me that hard coding in the relocation as several 
C routines will produce a  u-boot that is "smaller" than the one 
produced by using normal ELF relocation. However, using full relocation 
creates an environment that is true "C" and does not rely on people 
remembering that they may have to fix up some parts of their code. It is 
hard to see much downside in using the full relocation capability 
provided by Graeme's code.
FWIW, the relocation code and data does not have to be moved into ram if 
space is at a premium.

Best Regards,
Bill Campbell

>   Jocke
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>

[U-Boot] ARM relocation, question to Heiko

[Reinhard Meyer]

Dear all,

thanks for all the info.

My AT91 boards will not use relocation for the time being, and if
relocation is god-like enforced I will find a way not to use it.
I don't need to spend 10% more code for all that trouble.

Reinhard

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA77AFA.2090909@emk-elektronik.de> you wrote:
> 
> My AT91 boards will not use relocation for the time being, and if
> relocation is god-like enforced I will find a way not to use it.
> I don't need to spend 10% more code for all that trouble.

Please see
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/85186

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
"Am besten betrachten Sie Fehlermeldungen als eine  Art  Psycho-Test,
mit  dem  herausgefunden  werden soll, wie belastbar Sie sind."
 - Dr. R. Wonneberger, Kompaktf?hrer LaTeX, Kap. 1.6: Fehlermeldungen

[U-Boot] ARM relocation, question to Heiko

[Reinhard Meyer]

Dear J. William Campbell,
> On 10/2/2010 3:17 AM, Joakim Tjernlund wrote:
>>> Hello Reinhard,
>>>
>>> Reinhard Meyer wrote:
>>>> Dear Albert ARIBAUD,
>>>>>> I try to understand how the relocation process could handle pointers (to
>>>>>> functions or other data) in const or data sections.
>>>>>> Your code cannot know what is data and what is a pointer that needs
>>>>>> adjustment?
>>>>>>
>>>>>> Best Regards,
>>>>>> Reinhard
>>>>> Hi Reinhart,
>>>>>
>>>>> Short answer - the relocation process does not handle pointers inside
>>>>> data structures.
>>>>>
>>>>> And yes, this means the content arrays of pointers such as init_sequence
>>>>> is not relocated. Been there, done that, can give you one of the
>>> The init_sequence should not called anymore after relocation, as it is
>>> the init_sequence ... or?
>>>
>>>>> tee-shirts I got :)
>>>>>
>>>>> ATM I have not found a way to fix this, except making the code which
>>>>> uses the pointers aware that the are location-sensitive and fix them
>>>>> when using them.
>>>> That means that things like this cannot work (with relocation),
>>>> unless adding the relocation offset before using the pointer:
>>> Yep, you have to fix these pointers after relocation ...
>>>
>>>> const struct {
>>>> const u8 shift;
>>>> const u8 idcode;
>>>> struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
>>>> } flashes[] = {
>>>> #ifdef CONFIG_SPI_FLASH_SPANSION
>>>> { 0, 0x01, spi_flash_probe_spansion, },
>>>> #endif
>>> [...]
>>>> #ifdef CONFIG_SPI_FRAM_RAMTRON
>>>> { 6, 0xc2, spi_fram_probe_ramtron, },
>>>> # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
>>>> { 0, 0xff, spi_fram_probe_ramtron, },
>>>> # endif
>>>> # undef IDBUF_LEN
>>>> # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
>>>> #endif
>>>> };
>>>>
>>>> And I think there are more places of this type in u-boot...
>>> Yes, maybe. But relocation as I did for arm, also works
>>> on m68k, sparc, mips, avr32 and they must do also this
>>> fixups, so for common functions (except the new env handling,
>>> which I think got never tested on this architectures?) should
>>> work ...
>> This pointer problem is solved with the fixup relocs on ppc and
>> should work without manual relocation. I think this is a ppc
>> only extension but I might be wrong.
>
> Hi All,
> You are correct that this is a ppc only extension. As such, it is not a good candidate for "general" use.
>
>> I believe that the other alternative is to do it as x86 does
>> which I think is the general way which should work on any arch.
>> Graem Russ would know better.
>>
> Almost exactly a year ago, this was all pretty much presented by Graeme in the threads
> Relocation size penalty calculation (October 14, 2009)
> i386 Relocation (November 24, 2009)
>
> Using the full relocation scheme eliminates the need for all these "fixups" in u-boot C code. I think this is a very desirable result.
> It is also not clear to me that hard coding in the relocation as several C routines will produce a u-boot that is "smaller" than the one produced by using normal ELF relocation. However, using full relocation creates an environment that is true "C" and does not rely on people remembering that they may have to fix up some parts of their code. It is hard to see much downside in using the full relocation capability provided by Graeme's code.
> FWIW, the relocation code and data does not have to be moved into ram if space is at a premium.

I agree here. _If_ relocation, it should work without hand-adding
fixup stuff to all functions using initialized data with pointers.
Even Wolfgang forgot to fixup his 2nd level command table in
cmd_nvedit.c ;)

And, for space concerns in flash, relocation should always be an
option on a board by board basis...

And as an idea, if position independent code is used, only pointers
in initialized data need adjustment. Cannot the linker emit a table
of addresses that need fixing?

Best Regards,
Reinhard

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 02/10/2010 22:39, Reinhard Meyer a ?crit :

> And as an idea, if position independent code is used, only pointers
> in initialized data need adjustment. Cannot the linker emit a table
> of addresses that need fixing?

IIU Bill C, yes the linker can emit the information and the startup code 
could use this information instead of relying on hand-provided info; the 
linker file probably needs to be modified in order to provide such info. 
I intend to look into this, but feel free to do too.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Graeme Russ]

On 03/10/10 08:09, Albert ARIBAUD wrote:
> Le 02/10/2010 22:39, Reinhard Meyer a ?crit :
> 
>> And as an idea, if position independent code is used, only pointers
>> in initialized data need adjustment. Cannot the linker emit a table
>> of addresses that need fixing?
> 
> IIU Bill C, yes the linker can emit the information and the startup code 
> could use this information instead of relying on hand-provided info; the 
> linker file probably needs to be modified in order to provide such info. 
> I intend to look into this, but feel free to do too.
> 

As mentioned previously, I have already done this for x86. The linker flags
used are -pic and --emit-relocs. The linker produces a section named
rel.dyn which needs to be processed but not loaded into RAM. rel.dyn
contains a simple list of address (within .text, .data, .rodata etc) each
of which need a simple adjustment equal to the relocation offset.

The size increase of the code + data loaded into RAM is 104012 bytes to
104296 bytes which is only 284 bytes or a mere 0.3% (which is negligible)
with an additional 22424 bytes in rel.dyn (22%) not loaded into RAM

The additional bonus is that .got is not referenced during run-time, so
there is no run-time performance penalty. However, the penalty of
processing 2803 relocation records at startup may not be wholly recovered
during a typical u-boot run-time session.

All this is for x86, and may not apply so neatly to other arches

Regards,

Graeme

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 01:07, Graeme Russ a ?crit :
> On 03/10/10 08:09, Albert ARIBAUD wrote:
>> Le 02/10/2010 22:39, Reinhard Meyer a ?crit :
>>
>>> And as an idea, if position independent code is used, only pointers
>>> in initialized data need adjustment. Cannot the linker emit a table
>>> of addresses that need fixing?
>>
>> IIU Bill C, yes the linker can emit the information and the startup code
>> could use this information instead of relying on hand-provided info; the
>> linker file probably needs to be modified in order to provide such info.
>> I intend to look into this, but feel free to do too.
>
> As mentioned previously, I have already done this for x86. The linker flags
> used are -pic and --emit-relocs. The linker produces a section named
> rel.dyn which needs to be processed but not loaded into RAM. rel.dyn
> contains a simple list of address (within .text, .data, .rodata etc) each
> of which need a simple adjustment equal to the relocation offset.

Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary 
for relocation. You seem to imply it actually is... In my experience, 
-fPIC and-fPIE do increase code by adding GOT relocation to symbols that 
need fixing, so they would indeed be redundant to any other relocation 
mechanism -- I just did some test with basic code and this seems to 
confirm, no -fPIx is needed to get relocation the way you do on ARM.

> The size increase of the code + data loaded into RAM is 104012 bytes to
> 104296 bytes which is only 284 bytes or a mere 0.3% (which is negligible)
> with an additional 22424 bytes in rel.dyn (22%) not loaded into RAM
>
> The additional bonus is that .got is not referenced during run-time, so
> there is no run-time performance penalty. However, the penalty of
> processing 2803 relocation records at startup may not be wholly recovered
> during a typical u-boot run-time session.
>
> All this is for x86, and may not apply so neatly to other arches

Of course. :)

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Graeme Russ]

On 03/10/10 18:10, Albert ARIBAUD wrote:
> Le 03/10/2010 01:07, Graeme Russ a ?crit :
>> On 03/10/10 08:09, Albert ARIBAUD wrote:
>>> Le 02/10/2010 22:39, Reinhard Meyer a ?crit :
>>>
>>>> And as an idea, if position independent code is used, only pointers
>>>> in initialized data need adjustment. Cannot the linker emit a table
>>>> of addresses that need fixing?
>>>
>>> IIU Bill C, yes the linker can emit the information and the startup code
>>> could use this information instead of relying on hand-provided info; the
>>> linker file probably needs to be modified in order to provide such info.
>>> I intend to look into this, but feel free to do too.
>>
>> As mentioned previously, I have already done this for x86. The linker
>> flags
>> used are -pic and --emit-relocs. The linker produces a section named
>> rel.dyn which needs to be processed but not loaded into RAM. rel.dyn
>> contains a simple list of address (within .text, .data, .rodata etc) each
>> of which need a simple adjustment equal to the relocation offset.
> 
> Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary
> for relocation. You seem to imply it actually is... In my experience,
> -fPIC and-fPIE do increase code by adding GOT relocation to symbols that
> need fixing, so they would indeed be redundant to any other relocation
> mechanism -- I just did some test with basic code and this seems to
> confirm, no -fPIx is needed to get relocation the way you do on ARM.
> 

Just to clarify -fpic is a compiler option, -pic is a linker option. x86
has no compile time relocation options (therefore no referencing .got etc).
Using the link time pic option produces the relocation data table
(.rel.dyn) which must be pre-processed before execution can begin at the
relocated address

Cheers,

Graeme

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 10:44, Graeme Russ a ?crit :

>> Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary
>> for relocation. You seem to imply it actually is... In my experience,
>> -fPIC and-fPIE do increase code by adding GOT relocation to symbols that
>> need fixing, so they would indeed be redundant to any other relocation
>> mechanism -- I just did some test with basic code and this seems to
>> confirm, no -fPIx is needed to get relocation the way you do on ARM.
>
> Just to clarify -fpic is a compiler option, -pic is a linker option. x86
> has no compile time relocation options (therefore no referencing .got etc).
> Using the link time pic option produces the relocation data table
> (.rel.dyn) which must be pre-processed before execution can begin at the
> relocated address

Thanks for clarifying, Graeme.

This is consistent with the ARM compile-time options -fPIC/-fPIE vs 
link-time option -pie. So there may be at least an interest in 
investigating ELF-style relocation on ARM and comparing it to GOT-based 
relocation in terms of FLASH and RAM sizes and code speed.

> Cheers,
>
> Graeme

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/3/2010 1:58 AM, Albert ARIBAUD wrote:
> Le 03/10/2010 10:44, Graeme Russ a ?crit :
>
>>> Bill just said that -pic (or, for ARM, -fPIC or -fPIE) was unnecessary
>>> for relocation. You seem to imply it actually is... In my experience,
>>> -fPIC and-fPIE do increase code by adding GOT relocation to symbols 
>>> that
>>> need fixing, so they would indeed be redundant to any other relocation
>>> mechanism -- I just did some test with basic code and this seems to
>>> confirm, no -fPIx is needed to get relocation the way you do on ARM.
>>
>> Just to clarify -fpic is a compiler option, -pic is a linker option. x86
>> has no compile time relocation options (therefore no referencing .got 
>> etc).
>> Using the link time pic option produces the relocation data table
>> (.rel.dyn) which must be pre-processed before execution can begin at the
>> relocated address
>
> Thanks for clarifying, Graeme.
>
> This is consistent with the ARM compile-time options -fPIC/-fPIE vs 
> link-time option -pie. So there may be at least an interest in 
> investigating ELF-style relocation on ARM and comparing it to 
> GOT-based relocation in terms of FLASH and RAM sizes and code speed.
>
Hi All,
       It is for sure that -fPIC/-fPIE programs will contain more 
executable instructions than programs compiled without these options.
The program will also contain more data space for the got. If -fPIC 
actually produced a fully position-independent executable, the extra 
overhead would perhaps be tolerable. However, since it does not do this, 
(problems with initialized data etc.) there is really no advantage in 
using these compile-time options. The executable code and required data 
space for the program without these switches will "always" be smaller 
and faster than with them. In order to fix the remaining issues even 
when using -fPIC, a relocation loop must exist in the u-boot code, 
either one global one or a bunch of user written specific ones. Also, 
the -pie switch will be needed anyway at link time to build the 
relocation table for the remaining relocation requirements.
       Programs compiled without -fPIC will have a larger .rel.dyn table 
than those compiled with -fPIC. However, the table entries in the 
relocation table occupy about the same storage as the code generated by 
the compiler to relocate a reference to the symbol at run time.  So this 
is probably a almost a wash. Also, the dynamic relocation data need not 
be copied into the run-time object, as it is no longer needed. So the 
likely outcome is that the "flash" image is about the same size/slightly 
larger than the one compiled by -fPIC, and that the ram footprint after 
relocation is slightly smaller.
       If one is REALLY pressed for space, the size of the dynamic 
relocation area can be reduced by a post-processor program that would 
re-format the relocation entries. This re-formatting is possible because 
1) ELF is a very general format and we only need a small subset of it, 
and 2) u-boot code will never occupy say 16 MB of space, so each 
relocation can probably be compressed into a 32 bit word. I doubt anyone 
is that desperate, but it IS possible.
       It will be interesting to see what the results of this comparison 
are. For me, the no user awareness of relocation is worth a lot, and the 
fact that the difference/overhead of relocation will all be in exactly 
one place is very appealing.

Best Regards,
Bill Campbell
>> Cheers,
>>
>> Graeme
>
> Amicalement,

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 17:36, J. William Campbell a ?crit :

> Hi All,
> It is for sure that -fPIC/-fPIE programs will contain more executable
> instructions than programs compiled without these options.
> The program will also contain more data space for the got. If -fPIC
> actually produced a fully position-independent executable, the extra
> overhead would perhaps be tolerable. However, since it does not do this,
> (problems with initialized data etc.) there is really no advantage in
> using these compile-time options. The executable code and required data
> space for the program without these switches will "always" be smaller
> and faster than with them. In order to fix the remaining issues even
> when using -fPIC, a relocation loop must exist in the u-boot code,
> either one global one or a bunch of user written specific ones. Also,
> the -pie switch will be needed anyway at link time to build the
> relocation table for the remaining relocation requirements.
> Programs compiled without -fPIC will have a larger .rel.dyn table than
> those compiled with -fPIC. However, the table entries in the relocation
> table occupy about the same storage as the code generated by the
> compiler to relocate a reference to the symbol at run time. So this is
> probably a almost a wash. Also, the dynamic relocation data need not be
> copied into the run-time object, as it is no longer needed. So the
> likely outcome is that the "flash" image is about the same size/slightly
> larger than the one compiled by -fPIC, and that the ram footprint after
> relocation is slightly smaller.
> If one is REALLY pressed for space, the size of the dynamic relocation
> area can be reduced by a post-processor program that would re-format the
> relocation entries. This re-formatting is possible because 1) ELF is a
> very general format and we only need a small subset of it, and 2) u-boot
> code will never occupy say 16 MB of space, so each relocation can
> probably be compressed into a 32 bit word. I doubt anyone is that
> desperate, but it IS possible.
> It will be interesting to see what the results of this comparison are.
> For me, the no user awareness of relocation is worth a lot, and the fact
> that the difference/overhead of relocation will all be in exactly one
> place is very appealing.
>
> Best Regards,
> Bill Campbell

Hi Bill,

Thanks for the explanations. I am experimenting with ELF relocation 
right now, replacing -fPIe with -pie, and this generates .rel.dyn, but 
also many other sections. I'm trying to get rid of them; apparently 
/DISCARD/ing them in the linker file seems to reduce this to a minimum 
(I still have a .got.plt section which seems useless but I cannot remove 
it lest the linker segfaults).

But the .rel.dyn generated by the linker section does not provide 
symbols to mark its start and end, and I have found no documentation in 
binutils ld which would describe how to rewrite the .rel.dyn section and 
add these symbols myself.

How did you manage that for i386? I did not see a linker file in the 
i386 part of u-boot.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 18:47, Albert ARIBAUD a ?crit :

> But the .rel.dyn generated by the linker section does not provide
> symbols to mark its start and end, and I have found no documentation in
> binutils ld which would describe how to rewrite the .rel.dyn section and
> add these symbols myself.
>
> How did you manage that for i386? I did not see a linker file in the
> i386 part of u-boot.

Edit: found the linker, not in the arch part but in board eNET. Now 
trying to do same on ARM.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4CA8A2E0.7090407@comcast.net> you wrote:
>
> executable instructions than programs compiled without these options.
> The program will also contain more data space for the got. If -fPIC
> actually produced a fully position-independent executable, the extra
> overhead would perhaps be tolerable. However, since it does not do this,
> (problems with initialized data etc.) there is really no advantage in
> using these compile-time options. The executable code and required data
> space for the program without these switches will "always" be smaller
> and faster than with them. In order to fix the remaining issues even
> when using -fPIC, a relocation loop must exist in the u-boot code,
> either one global one or a bunch of user written specific ones. Also,

If needed, we should have a global one only.

>        It will be interesting to see what the results of this comparison
> are. For me, the no user awareness of relocation is worth a lot, and the
> fact that the difference/overhead of relocation will all be in exactly
> one place is very appealing.

Agreed.

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
Fools ignore complexity. Pragmatists suffer it. Some  can  avoid  it.
Geniuses remove it.
     - Perlis's Programming Proverb #58, SIGPLAN Notices, Sept.  1982

[U-Boot] ARM relocation, question to Heiko

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> Le 02/10/2010 22:39, Reinhard Meyer a ?crit :
> 
>> And as an idea, if position independent code is used, only pointers
>> in initialized data need adjustment. Cannot the linker emit a table
>> of addresses that need fixing?
> 
> IIU Bill C, yes the linker can emit the information and the startup code 
> could use this information instead of relying on hand-provided info; the 
> linker file probably needs to be modified in order to provide such info. 
> I intend to look into this, but feel free to do too.

I thought therefore is the GOT. But if there is another way, to get
rid of this hand fixing, it would be a good thing.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA79896.2010606@emk-elektronik.de> you wrote:
>
> I agree here. _If_ relocation, it should work without hand-adding
> fixup stuff to all functions using initialized data with pointers.
> Even Wolfgang forgot to fixup his 2nd level command table in
> cmd_nvedit.c ;)

I didn't forget it - at least not in the sensse that I think this is
something that needs to be done.

This works fine on PPC with relocation, and we should make it work
the same on other arches.

> And, for space concerns in flash, relocation should always be an
> option on a board by board basis...

NAK.

> And as an idea, if position independent code is used, only pointers
> in initialized data need adjustment. Cannot the linker emit a table
> of addresses that need fixing?

It does. That's the GOT.

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
I thought my people would grow tired of killing. But you were  right,
they  see it is easier than trading. And it has its pleasures. I feel
it myself. Like the hunt, but with richer rewards.
	-- Apella, "A Private Little War", stardate 4211.8

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/3/2010 11:29 AM, Wolfgang Denk wrote:
> Dear Reinhard Meyer,
>
> In message<4CA79896.2010606@emk-elektronik.de>  you wrote:
>> I agree here. _If_ relocation, it should work without hand-adding
>> fixup stuff to all functions using initialized data with pointers.
>> Even Wolfgang forgot to fixup his 2nd level command table in
>> cmd_nvedit.c ;)
> I didn't forget it - at least not in the sensse that I think this is
> something that needs to be done.
>
> This works fine on PPC with relocation, and we should make it work
> the same on other arches.
>
>> And, for space concerns in flash, relocation should always be an
>> option on a board by board basis...
> NAK.
>
>> And as an idea, if position independent code is used, only pointers
>> in initialized data need adjustment. Cannot the linker emit a table
>> of addresses that need fixing?
> It does. That's the GOT.
I think this is actually a misunderstanding. The purpose of the GOT, at 
least from GCC's point of view, is to hold the absolute addresses of 
private data referenced by shared library code. That is what it was 
invented to do. This is similar to, but not identical with, relocating 
all data references. Initialized data in the library must have a copy 
created (and relocated as necessary if it contains pointers) by the 
runtime linker when the library is initialized in the address space of 
the process using the library. The code in the shared library is -fPIC, 
but it still needs the runtime linker to allocate a copy of the GOT for 
the current user AND to allocate and relocate any data that is required 
for the library that is private to the user. It is that second step 
where we have trouble.

Best Regards,
Bill Campbell
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] ARM relocation, question to Heiko

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> Dear Reinhard Meyer,
> 
> In message <4CA79896.2010606@emk-elektronik.de> you wrote:
>> I agree here. _If_ relocation, it should work without hand-adding
>> fixup stuff to all functions using initialized data with pointers.
>> Even Wolfgang forgot to fixup his 2nd level command table in
>> cmd_nvedit.c ;)
> 
> I didn't forget it - at least not in the sensse that I think this is
> something that needs to be done.
> 
> This works fine on PPC with relocation, and we should make it work
> the same on other arches.

If we find a way for this on ARM, Ack!

>> And, for space concerns in flash, relocation should always be an
>> option on a board by board basis...
> 
> NAK.
> 
>> And as an idea, if position independent code is used, only pointers
>> in initialized data need adjustment. Cannot the linker emit a table
>> of addresses that need fixing?
> 
> It does. That's the GOT.

So I thought too, and I made a fixup in relocate_code() for the GOT
entries. But this don;t work with for example the commandtables.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4CA75BFB.5030208@comcast.net> you wrote:
>
> >>> And I think there are more places of this type in u-boot...
> >> Yes, maybe. But relocation as I did for arm, also works
> >> on m68k, sparc, mips, avr32 and they must do also this
> >> fixups, so for common functions (except the new env handling,
> >> which I think got never tested on this architectures?) should
> >> work ...
> > This pointer problem is solved with the fixup relocs on ppc and
> > should work without manual relocation. I think this is a ppc
> > only extension but I might be wrong.
> 
>        You are correct that this is a ppc only extension. As such, it is 
> not a good candidate for "general" use.

On contrary.

If it works for PPC, then there should be ways to do the same on other
architectures.

> Using the full relocation scheme eliminates the need for all these 
> "fixups" in u-boot C code. I think this is a very desirable result.
> It is also not clear to me that hard coding in the relocation as several 
> C routines will produce a  u-boot that is "smaller" than the one 
> produced by using normal ELF relocation. However, using full relocation 
> creates an environment that is true "C" and does not rely on people 
> remembering that they may have to fix up some parts of their code. It is 
> hard to see much downside in using the full relocation capability 
> provided by Graeme's code.

Agreed. But if we take this path, we need to find an implementation
that looks clean and readable.


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
Overflow on /dev/null, please empty the bit bucket.

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/3/2010 11:14 AM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message<4CA75BFB.5030208@comcast.net>  you wrote:
>>>>> And I think there are more places of this type in u-boot...
>>>> Yes, maybe. But relocation as I did for arm, also works
>>>> on m68k, sparc, mips, avr32 and they must do also this
>>>> fixups, so for common functions (except the new env handling,
>>>> which I think got never tested on this architectures?) should
>>>> work ...
>>> This pointer problem is solved with the fixup relocs on ppc and
>>> should work without manual relocation. I think this is a ppc
>>> only extension but I might be wrong.
>>         You are correct that this is a ppc only extension. As such, it is
>> not a good candidate for "general" use.
> On contrary.
>
> If it works for PPC, then there should be ways to do the same on other
> architectures.
>
Well, maybe so, but GCC won't do it now, and there has been no move by 
other architectures to adopt this capability. I suspect that it is 
extremely unlikley that this capability will ever be ported to other 
architectures since it has been available for so long on PPC without any 
movement to other systems.

>> Using the full relocation scheme eliminates the need for all these
>> "fixups" in u-boot C code. I think this is a very desirable result.
>> It is also not clear to me that hard coding in the relocation as several
>> C routines will produce a  u-boot that is "smaller" than the one
>> produced by using normal ELF relocation. However, using full relocation
>> creates an environment that is true "C" and does not rely on people
>> remembering that they may have to fix up some parts of their code. It is
>> hard to see much downside in using the full relocation capability
>> provided by Graeme's code.
> Agreed. But if we take this path, we need to find an implementation
> that looks clean and readable.
Agreed. This should be possible to do now that there is a better 
understanding of the ELF format by the u-boot community. Perhaps the 
place to start would be trying to port what Graeme has done to ARM or 
perhaps better yet, PPC. Since lots of people on this list are PPC 
folks, we should have a lot of leverage there.

Best Regards,
Bill Campbell
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 20:54, J. William Campbell a ?crit :

> Agreed. This should be possible to do now that there is a better
> understanding of the ELF format by the u-boot community. Perhaps the
> place to start would be trying to port what Graeme has done to ARM or
> perhaps better yet, PPC. Since lots of people on this list are PPC
> folks, we should have a lot of leverage there.

I am currently looking into ELF relocation on ARM.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA6E8E5.2090605@emk-elektronik.de> you wrote:
>
> > And yes, this means the content arrays of pointers such as init_sequence
> > is not relocated. Been there, done that, can give you one of the
> > tee-shirts I got :)

It should work.

Eventually we need to find out which sort of tweaking of compiler
and/or linker options is needed on ARM.

> > ATM I have not found a way to fix this, except making the code which
> > uses the pointers aware that the are location-sensitive and fix them
> > when using them.
> 
> That means that things like this cannot work (with relocation),
> unless adding the relocation offset before using the pointer:
> 
> const struct {
> 	const u8 shift;
> 	const u8 idcode;
> 	struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
> } flashes[] = {
> #ifdef CONFIG_SPI_FLASH_SPANSION
> 	{ 0, 0x01, spi_flash_probe_spansion, },
> #endif
> #ifdef CONFIG_SPI_FLASH_ATMEL
> 	{ 0, 0x1F, spi_flash_probe_atmel, },
> #endif
> #ifdef CONFIG_SPI_FLASH_MACRONIX
> 	{ 0, 0xc2, spi_flash_probe_macronix, },
> #endif
> #ifdef CONFIG_SPI_FLASH_WINBOND
> 	{ 0, 0xef, spi_flash_probe_winbond, },
> #endif
> #ifdef CONFIG_SPI_FLASH_STMICRO
> 	{ 0, 0x20, spi_flash_probe_stmicro, },
> 	{ 0, 0xff, spi_flash_probe_stmicro, },
> #endif
> #ifdef CONFIG_SPI_FLASH_SST
> 	{ 0, 0xBF, spi_flash_probe_sst, },
> #endif
> #ifdef CONFIG_SPI_FRAM_RAMTRON
> 	{ 6, 0xc2, spi_fram_probe_ramtron, },
> # ifdef CONFIG_SPI_FRAM_RAMTRON_NON_JEDEC
> 	{ 0, 0xff, spi_fram_probe_ramtron, },
> # endif
> # undef IDBUF_LEN
> # define IDBUF_LEN 9 /* we need to read 6+3 bytes */
> #endif
> };

Well, please keep in mind that all this code is working find on
PowerPC, which has been using relocation right from the beginning.

It is my understanding that we don't suffer from this issue any more
on PPC - Peter Tyser posted relocation fixup patches for PPC about a
year ago or so.

I have to admit that I cannot remeber the final result of this
discussion (there were tool chain dependencies?), but IIRC this has
been solved for PPC.

We should do the same for AMR now.


Peter, could you please fill in the details of that old story?


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
There is nothing in this world constant but inconstancy.      - Swift

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 03/10/2010 20:03, Wolfgang Denk a ?crit :

> Dear Reinhard Meyer,
>
> In message<4CA6E8E5.2090605@emk-elektronik.de>  you wrote:
>>
>>> And yes, this means the content arrays of pointers such as init_sequence
>>> is not relocated. Been there, done that, can give you one of the
>>> tee-shirts I got :)
>
> It should work.
>
> Eventually we need to find out which sort of tweaking of compiler
> and/or linker options is needed on ARM.

While looking for enhancements to Heiko's arm relocation patches, I have 
gone through all relocation related compiler and linker options, and 
then some. -fPIC / -fPIE will not generate GOT fixups for data 
containing pointers, and I have found no additional option that will.

OTOH, -pie will generate this kind of fixup (and all others needed) in 
the form of .rel.dyn relocations, as Bill describes.

> Well, please keep in mind that all this code is working find on
> PowerPC, which has been using relocation right from the beginning.

I don't mean to say that PPC relocation does not work;  I mean to say 
that a mechanism intended for one architecture may not be the optimal 
one for another, and if two mechanisms exist which provide relocation 
and one of them does not require specific tricks in the code, then I 
would choose this one.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4CA8CCC1.2010309@free.fr> you wrote:
> 
> > Well, please keep in mind that all this code is working find on
> > PowerPC, which has been using relocation right from the beginning.
> 
> I don't mean to say that PPC relocation does not work;  I mean to say 
> that a mechanism intended for one architecture may not be the optimal 
> one for another, and if two mechanisms exist which provide relocation 
> and one of them does not require specific tricks in the code, then I 
> would choose this one.

Agreed. But then, I did not mean to suggest that the situaltion on PPC
was perfect. If you find something that works even better for AMR,
then we should try and get the same improvment for PPC (and other
arches) as well.

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
Program maintenance is an entropy-increasing process,  and  even  its
most skilfull execution only delays the subsidence of the system into
unfixable obsolescence.       - Fred Brooks, "The Mythical Man Month"

[U-Boot] ARM relocation, question to Heiko

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> Le 03/10/2010 20:03, Wolfgang Denk a ?crit :
> 
>> Dear Reinhard Meyer,
>>
>> In message<4CA6E8E5.2090605@emk-elektronik.de>  you wrote:
>>>> And yes, this means the content arrays of pointers such as init_sequence
>>>> is not relocated. Been there, done that, can give you one of the
>>>> tee-shirts I got :)
>> It should work.
>>
>> Eventually we need to find out which sort of tweaking of compiler
>> and/or linker options is needed on ARM.
> 
> While looking for enhancements to Heiko's arm relocation patches, I have 
> gone through all relocation related compiler and linker options, and 
> then some. -fPIC / -fPIE will not generate GOT fixups for data 
> containing pointers, and I have found no additional option that will.

Yep, that was also my problem ...

> OTOH, -pie will generate this kind of fixup (and all others needed) in 
> the form of .rel.dyn relocations, as Bill describes.

Ah! Have to look in this, maybe thats the way we have to go ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 08:08, Heiko Schocher a ?crit :
> Hello Albert,
>
> Albert ARIBAUD wrote:
>> Le 03/10/2010 20:03, Wolfgang Denk a ?crit :
>>
>>> Dear Reinhard Meyer,
>>>
>>> In message<4CA6E8E5.2090605@emk-elektronik.de>   you wrote:
>>>>> And yes, this means the content arrays of pointers such as init_sequence
>>>>> is not relocated. Been there, done that, can give you one of the
>>>>> tee-shirts I got :)
>>> It should work.
>>>
>>> Eventually we need to find out which sort of tweaking of compiler
>>> and/or linker options is needed on ARM.
>>
>> While looking for enhancements to Heiko's arm relocation patches, I have
>> gone through all relocation related compiler and linker options, and
>> then some. -fPIC / -fPIE will not generate GOT fixups for data
>> containing pointers, and I have found no additional option that will.
>
> Yep, that was also my problem ...
>
>> OTOH, -pie will generate this kind of fixup (and all others needed) in
>> the form of .rel.dyn relocations, as Bill describes.
>
> Ah! Have to look in this, maybe thats the way we have to go ...

Right now I can build (not run, mind you) u-boot for edminiv2 without 
-fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all 
of the .rel.dyn fixup entries are type 23, that is, relative to the base 
address, which is good. However, here are about ten at the end which are 
type 2 -- symbol-relative -- and I am studying them in order to see if 
they are needed.

If type 23 relocations are all that is needed, then a first ARM ELF 
relocation implementation should 'simply' trade GOT vs .rel.dyn 
relocation in start.S (I am almost there) and remove fixups in 
board_init_r. Start.S would apply type 23 fixups only and ignore the 
rest. Later on we could add a build stage to rewrite the .rel.dyn 
section as suggested, by filtering out non-type-23 relocs and keeping 
only the address part of type-23 ones, reducing the .rel.dyn table 
roughly by half.

The good news is, I can spare a couple more hours today on this. I'll 
let you all know how this fares!

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Reinhard Meyer]

Dear Albert ARIBAUD,
> 
> Right now I can build (not run, mind you) u-boot for edminiv2 without 
> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all 
> of the .rel.dyn fixup entries are type 23, that is, relative to the base 
> address, which is good. However, here are about ten at the end which are 
> type 2 -- symbol-relative -- and I am studying them in order to see if 
> they are needed.
> 
> If type 23 relocations are all that is needed, then a first ARM ELF 
> relocation implementation should 'simply' trade GOT vs .rel.dyn 
> relocation in start.S (I am almost there) and remove fixups in 
> board_init_r. Start.S would apply type 23 fixups only and ignore the 
> rest. Later on we could add a build stage to rewrite the .rel.dyn 
> section as suggested, by filtering out non-type-23 relocs and keeping 
> only the address part of type-23 ones, reducing the .rel.dyn table 
> roughly by half.
> 
> The good news is, I can spare a couple more hours today on this. I'll 
> let you all know how this fares!

Thats good news! How much did the image size increase with this table?

And I am willing to test your efforts on AT91 here, maybe you can send me
the changes to .lds and start.S beforehand so I can see what type of
relocation info gets produced here.

A rather wild, but quite arch independant additional "build stage" for
relocation would be to link u-boot for two different TEXT_BASE values
e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff"
tool should find the 32 bit places where relocation is required and
add a table to the end of u-boot.bin... (Just a rough idea)

Best Regards,
Reinhard

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 09:27, Reinhard Meyer a ?crit :
> Dear Albert ARIBAUD,
>>
>> Right now I can build (not run, mind you) u-boot for edminiv2 without
>> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
>> of the .rel.dyn fixup entries are type 23, that is, relative to the base
>> address, which is good. However, here are about ten at the end which are
>> type 2 -- symbol-relative -- and I am studying them in order to see if
>> they are needed.
>>
>> If type 23 relocations are all that is needed, then a first ARM ELF
>> relocation implementation should 'simply' trade GOT vs .rel.dyn
>> relocation in start.S (I am almost there) and remove fixups in
>> board_init_r. Start.S would apply type 23 fixups only and ignore the
>> rest. Later on we could add a build stage to rewrite the .rel.dyn
>> section as suggested, by filtering out non-type-23 relocs and keeping
>> only the address part of type-23 ones, reducing the .rel.dyn table
>> roughly by half.
>>
>> The good news is, I can spare a couple more hours today on this. I'll
>> let you all know how this fares!
>
> Thats good news! How much did the image size increase with this table?

./MAKEALL edminiv2 results:

    text	   data	    bss	    dec	    hex	filename
  141376	   4388	  16640	 162404	  27a64	./u-boot (for GOT reloc)
  150160	   3819	  16640	 170619	  29a7b	./u-boot (for ELF reloc)

u-boot.bin size in bytes:

  145764 (for GOT reloc)
  153976 (for ELF reloc)

The .rel.dyn table is 18472 bytes, and should eventually shrink by half, 
losing about 9 KB. That would bring the u-boot.bin size down to
145 KB, roughly the same size as GOT reloc -- plus we'd save a few code 
bytes since reloc fixup functions in board_init_r would not be needed 
any more.

> And I am willing to test your efforts on AT91 here, maybe you can send me
> the changes to .lds and start.S beforehand so I can see what type of
> relocation info gets produced here.

I'll post an RFC patch within one or two hours.

> A rather wild, but quite arch independant additional "build stage" for
> relocation would be to link u-boot for two different TEXT_BASE values
> e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff"
> tool should find the 32 bit places where relocation is required and
> add a table to the end of u-boot.bin... (Just a rough idea)

That would be the simplest option even though it's obviously not 
optimal. Feel free to start this if you want, and let's meet at the 
bridge. :)

> Best Regards,
> Reinhard

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Heiko Schocher]

Hello Albert,

Albert ARIBAUD wrote:
> Le 04/10/2010 09:27, Reinhard Meyer a ?crit :
>> Dear Albert ARIBAUD,
>>>
>>> Right now I can build (not run, mind you) u-boot for edminiv2 without
>>> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
>>> of the .rel.dyn fixup entries are type 23, that is, relative to the base
>>> address, which is good. However, here are about ten at the end which are
>>> type 2 -- symbol-relative -- and I am studying them in order to see if
>>> they are needed.
>>>
>>> If type 23 relocations are all that is needed, then a first ARM ELF
>>> relocation implementation should 'simply' trade GOT vs .rel.dyn
>>> relocation in start.S (I am almost there) and remove fixups in
>>> board_init_r. Start.S would apply type 23 fixups only and ignore the
>>> rest. Later on we could add a build stage to rewrite the .rel.dyn
>>> section as suggested, by filtering out non-type-23 relocs and keeping
>>> only the address part of type-23 ones, reducing the .rel.dyn table
>>> roughly by half.
>>>
>>> The good news is, I can spare a couple more hours today on this. I'll
>>> let you all know how this fares!
>>
>> Thats good news! How much did the image size increase with this table?
> 
> ./MAKEALL edminiv2 results:
> 
>    text       data        bss        dec        hex    filename
>  141376       4388      16640     162404      27a64    ./u-boot (for GOT
> reloc)
>  150160       3819      16640     170619      29a7b    ./u-boot (for ELF
> reloc)
> 
> u-boot.bin size in bytes:
> 
>  145764 (for GOT reloc)
>  153976 (for ELF reloc)

Huh...

> The .rel.dyn table is 18472 bytes, and should eventually shrink by half,
> losing about 9 KB. That would bring the u-boot.bin size down to
> 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code

... puuh ;-)

> bytes since reloc fixup functions in board_init_r would not be needed
> any more.

Sounds good. And we can easy test this, by defining CONFIG_RELOC_FIXUP_WORKS
for all arm boards ...

>> And I am willing to test your efforts on AT91 here, maybe you can send me
>> the changes to .lds and start.S beforehand so I can see what type of
>> relocation info gets produced here.
> 
> I'll post an RFC patch within one or two hours.

Thanks! I am wating for it, and try your patches too.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 10:57, Heiko Schocher a ?crit :

>> ./MAKEALL edminiv2 results:
>>
>>     text       data        bss        dec        hex    filename
>>   141376       4388      16640     162404      27a64    ./u-boot (for GOT
>> reloc)
>>   150160       3819      16640     170619      29a7b    ./u-boot (for ELF
>> reloc)
>>
>> u-boot.bin size in bytes:
>>
>>   145764 (for GOT reloc)
>>   153976 (for ELF reloc)
>
> Huh...
>
>> The .rel.dyn table is 18472 bytes, and should eventually shrink by half,
>> losing about 9 KB. That would bring the u-boot.bin size down to
>> 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code
>
> ... puuh ;-)

:)

Think also that if I'm not mistaken, the GOT has to move to RAM while 
the .rel.dyn and .dynsym tables will not be necessary once relocated 
(unless you want u-boot to be able to move around in RAM), so RAM 
footprint would be smaller.

>> bytes since reloc fixup functions in board_init_r would not be needed
>> any more.
>
> Sounds good. And we can easy test this, by defining CONFIG_RELOC_FIXUP_WORKS
> for all arm boards ...

I'll check that.

>>> And I am willing to test your efforts on AT91 here, maybe you can send me
>>> the changes to .lds and start.S beforehand so I can see what type of
>>> relocation info gets produced here.
>>
>> I'll post an RFC patch within one or two hours.
>
> Thanks! I am wating for it, and try your patches too.

Testing on the board right now.

> bye,
> Heiko

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

>
> Le 04/10/2010 10:57, Heiko Schocher a ?crit :
>
> >> ./MAKEALL edminiv2 results:
> >>
> >>     text       data        bss        dec        hex    filename
> >>   141376       4388      16640     162404      27a64    ./u-boot (for GOT
> >> reloc)
> >>   150160       3819      16640     170619      29a7b    ./u-boot (for ELF
> >> reloc)
> >>
> >> u-boot.bin size in bytes:
> >>
> >>   145764 (for GOT reloc)
> >>   153976 (for ELF reloc)
> >
> > Huh...
> >
> >> The .rel.dyn table is 18472 bytes, and should eventually shrink by half,
> >> losing about 9 KB. That would bring the u-boot.bin size down to
> >> 145 KB, roughly the same size as GOT reloc -- plus we'd save a few code
> >
> > ... puuh ;-)
>
> :)
>
> Think also that if I'm not mistaken, the GOT has to move to RAM while
> the .rel.dyn and .dynsym tables will not be necessary once relocated
> (unless you want u-boot to be able to move around in RAM), so RAM
> footprint would be smaller.

You might get away with less relocs using -msdata -G 9999. Not
sure how that works on arm.

       Jocke

[U-Boot] ARM relocation, question to Heiko

[Graeme Russ]

On 04/10/10 19:28, Albert ARIBAUD wrote:
> Le 04/10/2010 09:27, Reinhard Meyer a ?crit :
>> Dear Albert ARIBAUD,
>>>
>>> Right now I can build (not run, mind you) u-boot for edminiv2 without
>>> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all

Any reason to do it in assembler? Have a look at arch/i386/lib/board.c
board_init_f() (especially if you apply my latest patch series)

>>> of the .rel.dyn fixup entries are type 23, that is, relative to the base
>>> address, which is good. However, here are about ten at the end which are
>>> type 2 -- symbol-relative -- and I am studying them in order to see if
>>> they are needed.

Hmm, for x86 they are all type 8 (R_386_RELATIVE) which are a simple Base +
Addend (B + A) entries

Type 23 is R_ARM_RELATIVE which are also B + A (although they can also by S
+ A whatever that means)

>>> If type 23 relocations are all that is needed, then a first ARM ELF
>>> relocation implementation should 'simply' trade GOT vs .rel.dyn
>>> relocation in start.S (I am almost there) and remove fixups in

Removing fixups - sweet, oh so sweet ;)

>>> board_init_r. Start.S would apply type 23 fixups only and ignore the
>>> rest. Later on we could add a build stage to rewrite the .rel.dyn
>>> section as suggested, by filtering out non-type-23 relocs and keeping
>>> only the address part of type-23 ones, reducing the .rel.dyn table
>>> roughly by half.

Also non type-8 for x86 - If all arches reduce down to a single relocation
type in .rel.dyn then we can ignore the type and simply strip all the
'type' fields.

[snip]
> 
>> A rather wild, but quite arch independant additional "build stage" for
>> relocation would be to link u-boot for two different TEXT_BASE values
>> e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff"
>> tool should find the 32 bit places where relocation is required and
>> add a table to the end of u-boot.bin... (Just a rough idea)

I don't think we need to - everything should be handled by .rel.dyn. I
wrote a diff tool to do as you suggest, but I have no need for it now

Looks like we could be onto a winner :)

Regards,

Graeme

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 11:58, Graeme Russ a ?crit :
> On 04/10/10 19:28, Albert ARIBAUD wrote:
>> Le 04/10/2010 09:27, Reinhard Meyer a ?crit :
>>> Dear Albert ARIBAUD,
>>>>
>>>> Right now I can build (not run, mind you) u-boot for edminiv2 without
>>>> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
>
> Any reason to do it in assembler? Have a look at arch/i386/lib/board.c
> board_init_f() (especially if you apply my latest patch series)

The main reasons are that start.S is historically responsible for 
setting up the C environment, and that only in assembly language can you 
ensure that no nasty relocation fixup is going to be required by the 
code that is precisely supposed to do the fixing up. Now it may be 
possible to do the fixups in C on ARM; that'll be a second step IMO.

>>>> of the .rel.dyn fixup entries are type 23, that is, relative to the base
>>>> address, which is good. However, here are about ten at the end which are
>>>> type 2 -- symbol-relative -- and I am studying them in order to see if
>>>> they are needed.
>
> Hmm, for x86 they are all type 8 (R_386_RELATIVE) which are a simple Base +
> Addend (B + A) entries
>
> Type 23 is R_ARM_RELATIVE which are also B + A (although they can also by S
> + A whatever that means)

23 is program base relative -- basically, subtract link-time image base 
address, add run-time image base address and you're set.

>>>> If type 23 relocations are all that is needed, then a first ARM ELF
>>>> relocation implementation should 'simply' trade GOT vs .rel.dyn
>>>> relocation in start.S (I am almost there) and remove fixups in
>
> Removing fixups - sweet, oh so sweet ;)

Apparently sweetness is not far away, see below. :)

>>>> board_init_r. Start.S would apply type 23 fixups only and ignore the
>>>> rest. Later on we could add a build stage to rewrite the .rel.dyn
>>>> section as suggested, by filtering out non-type-23 relocs and keeping
>>>> only the address part of type-23 ones, reducing the .rel.dyn table
>>>> roughly by half.
>
> Also non type-8 for x86 - If all arches reduce down to a single relocation
> type in .rel.dyn then we can ignore the type and simply strip all the
> 'type' fields.

Can't reduce to a single relocation type as produced by the linker, 
because references to linker-file-generated symbols seem to always be 
symbol-relative, not program-relative, even under -pie. This may be 
something to ask on the binutils mailing list, though.

> [snip]
>>
>>> A rather wild, but quite arch independant additional "build stage" for
>>> relocation would be to link u-boot for two different TEXT_BASE values
>>> e.g. TEXT_BASE (as desired) and TEXT_BASE+0x00010010. A special "diff"
>>> tool should find the 32 bit places where relocation is required and
>>> add a table to the end of u-boot.bin... (Just a rough idea)
>
> I don't think we need to - everything should be handled by .rel.dyn. I
> wrote a diff tool to do as you suggest, but I have no need for it now
>
> Looks like we could be onto a winner :)
>
> Regards,
>
> Graeme

At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.

Environment is correctly read and can be modified (did not try saving 
though).

Flash operations work (flinfo, erase, cp.b).

Ethernet does not work, however -- ping or tftp just wait without me 
being able to ^C it. I'll look into that as soon as some domestic chores 
are done. :)

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Rogan Dawes]

On 2010/10/04 4:17 PM, Albert ARIBAUD wrote:
> At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.
> 
> Environment is correctly read and can be modified (did not try saving 
> though).
> 
> Flash operations work (flinfo, erase, cp.b).
> 
> Ethernet does not work, however -- ping or tftp just wait without me 
> being able to ^C it. I'll look into that as soon as some domestic chores 
> are done. :)

Excellent work! Congratulations!

Rogan

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 16:25, Rogan Dawes a ?crit :
> On 2010/10/04 4:17 PM, Albert ARIBAUD wrote:
>> At this point I have an ARM926, ELF-relocating, u-boot reaching prompt.
>>
>> Environment is correctly read and can be modified (did not try saving
>> though).
>>
>> Flash operations work (flinfo, erase, cp.b).
>>
>> Ethernet does not work, however -- ping or tftp just wait without me
>> being able to ^C it. I'll look into that as soon as some domestic chores
>> are done. :)
>
> Excellent work! Congratulations!

Thanks, but I'd suggest to wait for congrats until ethernet works -- 
tough I've got a clue, I think; it may have to do with the core 
activating its caches and the driver doing DMA. :)

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Stefan Roese]

On Monday 04 October 2010 17:24:09 Albert ARIBAUD wrote:
> > Excellent work! Congratulations!
> 
> Thanks, but I'd suggest to wait for congrats until ethernet works --
> tough I've got a clue, I think; it may have to do with the core
> activating its caches and the driver doing DMA. :)

Yes. Very likely a aching problem and not an relocation issue. With D-cache 
enabled some IO drivers might have some problems. This will also be a problem 
with for example USB support.

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 18:31, Stefan Roese a ?crit :
> On Monday 04 October 2010 17:24:09 Albert ARIBAUD wrote:
>>> Excellent work! Congratulations!
>>
>> Thanks, but I'd suggest to wait for congrats until ethernet works --
>> tough I've got a clue, I think; it may have to do with the core
>> activating its caches and the driver doing DMA. :)
>
> Yes. Very likely a aching problem and not an relocation issue. With D-cache
> enabled some IO drivers might have some problems. This will also be a problem
> with for example USB support.

... and that is a new proof that assumption is the mother of all screw-ups.

It was not a question of caching; it was a question of trashing the 
address of gd passed from board_init_f() through relocate_code() to 
board_init_r(), and all code before the marvell ethernet driver not 
relying on gd anought to break.

Patches to follow right away as an RFC.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 08:40, Albert ARIBAUD a ?crit :

> Right now I can build (not run, mind you) u-boot for edminiv2 without
> -fPIC/-fPIE, with -pie and a modified u-boot.lds and start.S. Almost all
> of the .rel.dyn fixup entries are type 23, that is, relative to the base
> address, which is good. However, here are about ten at the end which are
> type 2 -- symbol-relative -- and I am studying them in order to see if
> they are needed.

Those type 2 relocations correspond to symbols which are defined in the 
linker file (e.g. __got_start and __got_end) and thus are absolute, not 
relative.

> If type 23 relocations are all that is needed, then a first ARM ELF
> relocation implementation should 'simply' trade GOT vs .rel.dyn
> relocation in start.S (I am almost there) and remove fixups in
> board_init_r. Start.S would apply type 23 fixups only and ignore the
> rest. Later on we could add a build stage to rewrite the .rel.dyn
> section as suggested, by filtering out non-type-23 relocs and keeping
> only the address part of type-23 ones, reducing the .rel.dyn table
> roughly by half.

Al right, so type 2 are needed too, and of course they need a different 
processing than type 23, but the good news is, one should easily make 
the linker provide relative values for type 2 fixups by telling it to 
base the executable at offset 0. The interest of doing so is that all 
fixups in .rel.dyn could be processed homogeneously by adding the actual 
base address of the code to each fixup location.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Peter Tyser]

Hi Wolfgang,

<snip>

> Well, please keep in mind that all this code is working find on
> PowerPC, which has been using relocation right from the beginning.
> 
> It is my understanding that we don't suffer from this issue any more
> on PPC - Peter Tyser posted relocation fixup patches for PPC about a
> year ago or so.
> 
> I have to admit that I cannot remeber the final result of this
> discussion (there were tool chain dependencies?), but IIRC this has
> been solved for PPC.
> 
> We should do the same for AMR now.
> 
> 
> Peter, could you please fill in the details of that old story?

I haven't been following the ARM relocation thread very closely, but a
summary of the PPC relocation is:
- Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported
basic relocation, but didn't relocate static pointers, eg pointers in a
structure, such as the strings in the cmd_pca953x[] table.  So we had to
use the hokey "struct->field += gd->reloc_off" fixups that we still have
for some arches.

- Adding "-mrelocatable" to the gcc's flags would add additional
relocation info into the ".fixup" section that allowed us to properly
fixup static pointers and get rid of the "+= gd->reloc_off" fixups.  The
additional fixup code was relatively small, in the 1-2% ballpark I
think.

- Unfortunately "-mrelocatable" is PPC specific.  I remember dabbling
with other, more general relocation flags like -fPIC, pie, etc, but went
with -mrelocatable mainly because it was a smaller, easier change.  We
could share the already-used relocation fixup code in many PPC arch's
start.S, so it wasn't too hard to get working unlike the other
relocation schemes.

- Graeme Russ was working on relocation for x86 near the same time.  He
started this thread which may provide useful info:
http://www.mail-archive.com/u-boot at lists.denx.de/msg23347.html He
discusses the impact of other compile flags, most of which were generic
so could apply to this ARM discussion hopefully.

Regards,
Peter

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Peter Tyser,

In message <1286167382.22760.19.camel@ptyser-laptop> you wrote:
> 
> > Peter, could you please fill in the details of that old story?
> 
> I haven't been following the ARM relocation thread very closely, but a
> summary of the PPC relocation is:
> - Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported
> basic relocation, but didn't relocate static pointers, eg pointers in a
> structure, such as the strings in the cmd_pca953x[] table.  So we had to
> use the hokey "struct->field += gd->reloc_off" fixups that we still have
> for some arches.
> 
> - Adding "-mrelocatable" to the gcc's flags would add additional
> relocation info into the ".fixup" section that allowed us to properly
> fixup static pointers and get rid of the "+= gd->reloc_off" fixups.  The
> additional fixup code was relatively small, in the 1-2% ballpark I
> think.
> 
> - Unfortunately "-mrelocatable" is PPC specific.  I remember dabbling
> with other, more general relocation flags like -fPIC, pie, etc, but went
> with -mrelocatable mainly because it was a smaller, easier change.  We
> could share the already-used relocation fixup code in many PPC arch's
> start.S, so it wasn't too hard to get working unlike the other
> relocation schemes.
> 
> - Graeme Russ was working on relocation for x86 near the same time.  He
> started this thread which may provide useful info:
> http://www.mail-archive.com/u-boot at lists.denx.de/msg23347.html He
> discusses the impact of other compile flags, most of which were generic
> so could apply to this ARM discussion hopefully.

Thanks alot for the summary.

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
Q: How do you spell "onomatopoeia"?
A: The way it sounds.

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

>
> Hi Wolfgang,
>
> <snip>
>
> > Well, please keep in mind that all this code is working find on
> > PowerPC, which has been using relocation right from the beginning.
> >
> > It is my understanding that we don't suffer from this issue any more
> > on PPC - Peter Tyser posted relocation fixup patches for PPC about a
> > year ago or so.
> >
> > I have to admit that I cannot remeber the final result of this
> > discussion (there were tool chain dependencies?), but IIRC this has
> > been solved for PPC.
> >
> > We should do the same for AMR now.
> >
> >
> > Peter, could you please fill in the details of that old story?
>
> I haven't been following the ARM relocation thread very closely, but a
> summary of the PPC relocation is:
> - Prior to commit 858290178f222d998b6425d85cf06822467918f3 PPC supported
> basic relocation, but didn't relocate static pointers, eg pointers in a
> structure, such as the strings in the cmd_pca953x[] table.  So we had to
> use the hokey "struct->field += gd->reloc_off" fixups that we still have
> for some arches.
>
> - Adding "-mrelocatable" to the gcc's flags would add additional
> relocation info into the ".fixup" section that allowed us to properly
> fixup static pointers and get rid of the "+= gd->reloc_off" fixups.  The
> additional fixup code was relatively small, in the 1-2% ballpark I
> think.
>
> - Unfortunately "-mrelocatable" is PPC specific.  I remember dabbling
> with other, more general relocation flags like -fPIC, pie, etc, but went
> with -mrelocatable mainly because it was a smaller, easier change.  We
> could share the already-used relocation fixup code in many PPC arch's
> start.S, so it wasn't too hard to get working unlike the other
> relocation schemes.
>
> - Graeme Russ was working on relocation for x86 near the same time.  He
> started this thread which may provide useful info:
> http://www.mail-archive.com/u-boot at lists.denx.de/msg23347.html He
> discusses the impact of other compile flags, most of which were generic
> so could apply to this ARM discussion hopefully.

However, I think we will loose the possibility to add "link once, burn and run anywhere"
feature I impl. once(but it was at the time deemed to intrusive) if
we skip -fPIC and go for the linker -pie relocation.

I would suggest to leave ppc as is for the time being and
see how -pie works out on the other archs.

     Jocke

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 09:36, Joakim Tjernlund a ?crit :

> However, I think we will loose the possibility to add "link once, burn and run anywhere"
> feature I impl. once(but it was at the time deemed to intrusive) if
> we skip -fPIC and go for the linker -pie relocation.

On ARM at least, I don't think so. From what I see, the -pie ld option 
without the -fPIC/-fPIE compiler option alone builds a .rel.syn table 
that contains all necessary fixes to mve the code anywhere.

However:

> I would suggest to leave ppc as is for the time being and
> see how -pie works out on the other archs.

I second that. More precisely, I'd suggest to wait for ELF relocation to 
succeed on arm296ejs before considering any other ARM, then any other arch.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

>
> Le 04/10/2010 09:36, Joakim Tjernlund a ?crit :
>
> > However, I think we will loose the possibility to add "link once, burn and
> run anywhere"
> > feature I impl. once(but it was at the time deemed to intrusive) if
> > we skip -fPIC and go for the linker -pie relocation.
>
> On ARM at least, I don't think so. From what I see, the -pie ld option
> without the -fPIC/-fPIE compiler option alone builds a .rel.syn table
> that contains all necessary fixes to mve the code anywhere.

hmm, maybe my memory fails me but doesn't these relocs change the
code to relocate accesses? While in flash you can't do that.
Perhaps this is fixable too with the LINK_OFF method I impl.
but memory fails me as it was quite some time ago.

   Jocke

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 10:28, Joakim Tjernlund a ?crit :
>>
>> Le 04/10/2010 09:36, Joakim Tjernlund a ?crit :
>>
>>> However, I think we will loose the possibility to add "link once, burn and
>> run anywhere"
>>> feature I impl. once(but it was at the time deemed to intrusive) if
>>> we skip -fPIC and go for the linker -pie relocation.
>>
>> On ARM at least, I don't think so. From what I see, the -pie ld option
>> without the -fPIC/-fPIE compiler option alone builds a .rel.syn table
>> that contains all necessary fixes to mve the code anywhere.
>
> hmm, maybe my memory fails me but doesn't these relocs change the
> code to relocate accesses? While in flash you can't do that.
> Perhaps this is fixable too with the LINK_OFF method I impl.
> but memory fails me as it was quite some time ago.
>
>     Jocke

Actually the principle is to link with TEXT_BASE equal to the NOR FLASH 
location of the image [1] so that relocation is not needed there. Only 
when you move the code to RAM do you need relocation.

[1] which voids my idea of setting TEXT_BASE to 0, btw, but that does 
not matter much anyway.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

Albert ARIBAUD <albert.aribaud@free.fr> wrote on 2010/10/04 10:33:05:
>
> Le 04/10/2010 10:28, Joakim Tjernlund a ?crit :
> >>
> >> Le 04/10/2010 09:36, Joakim Tjernlund a ?crit :
> >>
> >>> However, I think we will loose the possibility to add "link once, burn and
> >> run anywhere"
> >>> feature I impl. once(but it was at the time deemed to intrusive) if
> >>> we skip -fPIC and go for the linker -pie relocation.
> >>
> >> On ARM at least, I don't think so. From what I see, the -pie ld option
> >> without the -fPIC/-fPIE compiler option alone builds a .rel.syn table
> >> that contains all necessary fixes to mve the code anywhere.
> >
> > hmm, maybe my memory fails me but doesn't these relocs change the
> > code to relocate accesses? While in flash you can't do that.
> > Perhaps this is fixable too with the LINK_OFF method I impl.
> > but memory fails me as it was quite some time ago.
> >
> >     Jocke
>
> Actually the principle is to link with TEXT_BASE equal to the NOR FLASH
> location of the image [1] so that relocation is not needed there. Only
> when you move the code to RAM do you need relocation.

Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0)
but burn and run into another address. I impl. this quite some time
ago for PPC(search for LINK_OFF)

       Jocke

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 10:52, Joakim Tjernlund a ?crit :
> Albert ARIBAUD<albert.aribaud@free.fr>  wrote on 2010/10/04 10:33:05:
>>
>> Le 04/10/2010 10:28, Joakim Tjernlund a ?crit :
>>>>
>>>> Le 04/10/2010 09:36, Joakim Tjernlund a ?crit :
>>>>
>>>>> However, I think we will loose the possibility to add "link once, burn and
>>>> run anywhere"
>>>>> feature I impl. once(but it was at the time deemed to intrusive) if
>>>>> we skip -fPIC and go for the linker -pie relocation.
>>>>
>>>> On ARM at least, I don't think so. From what I see, the -pie ld option
>>>> without the -fPIC/-fPIE compiler option alone builds a .rel.syn table
>>>> that contains all necessary fixes to mve the code anywhere.
>>>
>>> hmm, maybe my memory fails me but doesn't these relocs change the
>>> code to relocate accesses? While in flash you can't do that.
>>> Perhaps this is fixable too with the LINK_OFF method I impl.
>>> but memory fails me as it was quite some time ago.
>>>
>>>      Jocke
>>
>> Actually the principle is to link with TEXT_BASE equal to the NOR FLASH
>> location of the image [1] so that relocation is not needed there. Only
>> when you move the code to RAM do you need relocation.
>
> Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0)
> but burn and run into another address. I impl. this quite some time
> ago for PPC(search for LINK_OFF)

I am ultimately looking for same here on ARM.

Note however that linking for base address 0 is not mandatory for 
achieving true position independence. What is required is that the code 
which runs from power-up until relocation be able to run anywhere, i.e., 
this code should not require any relocation fixup. That can be achieved 
on ARM by using only relative branches and accessing data only relative 
to pc (e.g. literals) or truly absolute (e.g. HW registers etc).

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4CA999EE.5030309@free.fr> you wrote:
>
> Note however that linking for base address 0 is not mandatory for 
> achieving true position independence. What is required is that the code 
> which runs from power-up until relocation be able to run anywhere, i.e.,
> this code should not require any relocation fixup. That can be achieved 
> on ARM by using only relative branches and accessing data only relative
> to pc (e.g. literals) or truly absolute (e.g. HW registers etc).

That means you need to build all of U-Boot that way, because
significant parts of the code already run before relocation
(including all clocks and timers setup, console setup, printf and all
routines these pull in).

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
Some programming languages manage to  absorb  change,  but  withstand
progress.          -- Epigrams in Programming, ACM SIGPLAN Sept. 1982

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/4/2010 3:13 AM, Wolfgang Denk wrote:
> Dear Albert ARIBAUD,
>
> In message<4CA999EE.5030309@free.fr>  you wrote:
>> Note however that linking for base address 0 is not mandatory for
>> achieving true position independence. What is required is that the code
>> which runs from power-up until relocation be able to run anywhere, i.e.,
>> this code should not require any relocation fixup. That can be achieved
>> on ARM by using only relative branches and accessing data only relative
>> to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
> That means you need to build all of U-Boot that way, because
> significant parts of the code already run before relocation
> (including all clocks and timers setup, console setup, printf and all
> routines these pull in).
>
Yes, I think Wolfgang is correct. This is not going to be easy to do in 
general.  To run anywhere, the code must be true Position Independent 
code. If you intend to use any C code in the initialization, this will 
result in needing -fPIC for at least that code. I am not sure you can 
mix -fPIC and non -fPIC code in the same link, but I expect not. I am a 
bit surprised that it is possible to get even the initialization code to 
be Position Independent, but it appears that on at least some PPC it is 
possible/has been done.
         On a related topic, I did find some information on the 
-mrelocatable history. Take a look at
http://www.mail-archive.com/gcc at gcc.gnu.org/msg02528.html.
If you read both thread entries, it explains -mrelocatable as more or 
less the post-processor that re-formats the ELF relocation information 
into a smaller format and puts it in the text as another segment. What 
Albert is doing now, and Graeme did before,  is the first option, 
creating a loader that understands ELF. This has the advantage that it 
will work on all architectures. However, once this understanding is in 
place, it would be easy to write a small post-processing program that 
would reduce the size of the relocation entries, much like -mrelocatable 
does. This may or may not be necessary, but it is certainly possible.

Best Regards,
Bill Campbell
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] ARM relocation, question to Heiko

[Albert ARIBAUD]

Le 04/10/2010 17:28, J. William Campbell a ?crit :
> On 10/4/2010 3:13 AM, Wolfgang Denk wrote:
>> Dear Albert ARIBAUD,
>>
>> In message<4CA999EE.5030309@free.fr> you wrote:
>>> Note however that linking for base address 0 is not mandatory for
>>> achieving true position independence. What is required is that the code
>>> which runs from power-up until relocation be able to run anywhere, i.e.,
>>> this code should not require any relocation fixup. That can be achieved
>>> on ARM by using only relative branches and accessing data only relative
>>> to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
>> That means you need to build all of U-Boot that way, because
>> significant parts of the code already run before relocation
>> (including all clocks and timers setup, console setup, printf and all
>> routines these pull in).
>>
> Yes, I think Wolfgang is correct. This is not going to be easy to do in
> general. To run anywhere, the code must be true Position Independent
> code. If you intend to use any C code in the initialization, this will
> result in needing -fPIC for at least that code. I am not sure you can
> mix -fPIC and non -fPIC code in the same link, but I expect not. I am a
> bit surprised that it is possible to get even the initialization code to
> be Position Independent, but it appears that on at least some PPC it is
> possible/has been done.

I'm not entirely sure about -fPIC, but it is possible indeed that true 
position independence might need it. For the moment, I'll settle for ELF 
relocatable. :)

> On a related topic, I did find some information on the -mrelocatable
> history. Take a look at
> http://www.mail-archive.com/gcc at gcc.gnu.org/msg02528.html.
> If you read both thread entries, it explains -mrelocatable as more or
> less the post-processor that re-formats the ELF relocation information
> into a smaller format and puts it in the text as another segment. What
> Albert is doing now, and Graeme did before, is the first option,
> creating a loader that understands ELF. This has the advantage that it
> will work on all architectures. However, once this understanding is in
> place, it would be easy to write a small post-processing program that
> would reduce the size of the relocation entries, much like -mrelocatable
> does. This may or may not be necessary, but it is certainly possible.
>
> Best Regards,
> Bill Campbell

Thanks Bill. I'll look into it once I get the current issues resolved; 
however it seems GOT-related, and -pie is not GOT-based -- and unlike 
GOT, -pie handles pointers in data, for instance, removing the need for 
manual fixups.

Amicalement,
-- 
Albert.

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4CA9F294.8080007@comcast.net> you wrote:
>
> Yes, I think Wolfgang is correct. This is not going to be easy to do in 
> general.  To run anywhere, the code must be true Position Independent 
> code. If you intend to use any C code in the initialization, this will 
> result in needing -fPIC for at least that code. I am not sure you can 
> mix -fPIC and non -fPIC code in the same link, but I expect not. I am a 
> bit surprised that it is possible to get even the initialization code to 
> be Position Independent, but it appears that on at least some PPC it is 
> possible/has been done.

Not really. On PowerPC, only the first 20 or 30 lines of assembler
code in start.S are position independent; then we compute the link
(resp. execution) address and branch to it. From then, we run from the
very address range we were linked for (starting at TEXT_BASE).

> Albert is doing now, and Graeme did before,  is the first option, 
> creating a loader that understands ELF. This has the advantage that it 
> will work on all architectures. However, once this understanding is in 
> place, it would be easy to write a small post-processing program that 
> would reduce the size of the relocation entries, much like -mrelocatable 
> does. This may or may not be necessary, but it is certainly possible.

Eventually we might even add -mrelocatable support for the other
architectures to the tool chain.

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
Its always easier short term to pee in the pond
than install a toilet - it's just not a good long term plan.
          - Alan Cox in <20100101145701.6432e7b7@lxorguk.ukuu.org.uk>

[U-Boot] ARM relocation, question to Heiko

[J. William Campbell]

  On 10/4/2010 10:06 AM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message<4CA9F294.8080007@comcast.net>  you wrote:
>> Yes, I think Wolfgang is correct. This is not going to be easy to do in
>> general.  To run anywhere, the code must be true Position Independent
>> code. If you intend to use any C code in the initialization, this will
>> result in needing -fPIC for at least that code. I am not sure you can
>> mix -fPIC and non -fPIC code in the same link, but I expect not. I am a
>> bit surprised that it is possible to get even the initialization code to
>> be Position Independent, but it appears that on at least some PPC it is
>> possible/has been done.
> Not really. On PowerPC, only the first 20 or 30 lines of assembler
> code in start.S are position independent; then we compute the link
> (resp. execution) address and branch to it. From then, we run from the
> very address range we were linked for (starting at TEXT_BASE).
Hi Wolfgang,
      You are of course correct. I was referring more to Jocke's 
(joakim.tjernlund at transmode.se) statements regarding:

Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0)
but burn and run into another address. I impl. this quite some time
ago for PPC(search for LINK_OFF)

         I understand from his comment that he had achieved total PIC for the initialization, that would run at any location regardless
of TEXT_BASE. I think this code was not accepted into mainline, so it is not a problem at present. However, any relocation code
added would have to be modified by Jocke if he wished to preserve that capability. I am amazed that he was able to get the
rest of u-boot to work under the constraints you pointed out. It must have been quite tedious.

       I also wish to support Graeme's desire that the added relocation code at the end of the day be written in C. The routine to do the
relocation does not require .bss and is not real long. The obvious advantage of this approach is that all architectures can use it. The
ELF relocation codes will have to be changed to the architecture equivalents, and in some casesarchitecture specific relocation code
processing added, but the theory will always be the same. This approach will make using relocation much easier/trivial for new
architecture  ports, thereby reducing resistance to doing it!

Best Regards,
Bill Campbell


>> Albert is doing now, and Graeme did before,  is the first option,
>> creating a loader that understands ELF. This has the advantage that it
>> will work on all architectures. However, once this understanding is in
>> place, it would be easy to write a small post-processing program that
>> would reduce the size of the relocation entries, much like -mrelocatable
>> does. This may or may not be necessary, but it is certainly possible.
> Eventually we might even add -mrelocatable support for the other
> architectures to the tool chain.
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

"J. William Campbell" <jwilliamcampbell@comcast.net> wrote on 2010/10/04 19:59:47:
> On 10/4/2010 10:06 AM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message <4CA9F294.8080007@comcast.net> you wrote:

>
> Yes, I think Wolfgang is correct. This is not going to be easy to do in
> general.  To run anywhere, the code must be true Position Independent
> code. If you intend to use any C code in the initialization, this will
> result in needing -fPIC for at least that code. I am not sure you can
> mix -fPIC and non -fPIC code in the same link, but I expect not. I am a
> bit surprised that it is possible to get even the initialization code to
> be Position Independent, but it appears that on at least some PPC it is
> possible/has been done.

>
> Not really. On PowerPC, only the first 20 or 30 lines of assembler
> code in start.S are position independent; then we compute the link
> (resp. execution) address and branch to it. From then, we run from the
> very address range we were linked for (starting at TEXT_BASE).

> Hi Wolfgang,
>      You are of course correct. I was referring more to Jocke's (
> joakim.tjernlund at transmode.se) statements regarding:
> Yes, that is there today. I am talking about linking to any TEXT_BASE(say 0)
> but burn and run into another address. I impl. this quite some time
> ago for PPC(search for LINK_OFF)
>
>        I understand from his comment that he had achieved total PIC for the
> initialization, that would run at any location regardless
> of TEXT_BASE. I think this code was not accepted into mainline, so it is not a
> problem at present. However, any relocation code
> added would have to be modified by Jocke if he wished to preserve that
> capability. I am amazed that he was able to get the
> rest of u-boot to work under the constraints you pointed out. It must have
> been quite tedious.

:), actually it wasn't that bad. Wolfgang nearly accepted the code even :)
Mainly, I had to wrap code that accessed global data with a LINK_OFF() function
that calculated the offset and only in code that executed before relocation.
And fix a few things in start.S to be PIC.

 Jocke

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFF06E784F.A10A5A15-ONC12577B2.0065FB3C-C12577B2.0066D69F@transmode.se> you wrote:
> 
> :), actually it wasn't that bad. Wolfgang nearly accepted the code even :)

Yes, I was really tempted because I do appreciate the value of such a
feature.

> Mainly, I had to wrap code that accessed global data with a LINK_OFF() function
> that calculated the offset and only in code that executed before relocation.

That was the part that gave me the creeps. It looked too much
unreadable and error prone to me, especially as this is not only a
one-time conversion but has to be kept in mind for all changes to
related code.

I'm still undecided, to be honest.

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
"...this does not mean that some of us should not want, in  a  rather
dispassionate sort of way, to put a bullet through csh's head."
                   - Larry Wall in <1992Aug6.221512.5963@netlabs.com>

[U-Boot] ARM relocation, question to Heiko

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2010/10/04 23:10:31:
>
> Dear Joakim Tjernlund,
>
> In message <OFF06E784F.A10A5A15-ONC12577B2.0065FB3C-C12577B2.
> 0066D69F at transmode.se> you wrote:
> >
> > :), actually it wasn't that bad. Wolfgang nearly accepted the code even :)
>
> Yes, I was really tempted because I do appreciate the value of such a
> feature.
>
> > Mainly, I had to wrap code that accessed global data with a LINK_OFF() function
> > that calculated the offset and only in code that executed before relocation.
>
> That was the part that gave me the creeps. It looked too much
> unreadable and error prone to me, especially as this is not only a
> one-time conversion but has to be kept in mind for all changes to
> related code.

Yes, that is a bummer. If one could convince gcc to do %pc relative
addressing on strings/constant data the we would be in business I think.
Anyone got gcc connections?

>
> I'm still undecided, to be honest.

[U-Boot] ARM relocation, question to Heiko

[Graeme Russ]

On Monday, October 4, 2010, Wolfgang Denk <wd@denx.de> wrote:
> Dear Albert ARIBAUD,
>
> In message <4CA999EE.5030309@free.fr> you wrote:
>>
>> Note however that linking for base address 0 is not mandatory for
>> achieving true position independence. What is required is that the code
>> which runs from power-up until relocation be able to run anywhere, i.e.,
>> this code should not require any relocation fixup. That can be achieved
>> on ARM by using only relative branches and accessing data only relative
>> to pc (e.g. literals) or truly absolute (e.g. HW registers etc).
>
> That means you need to build all of U-Boot that way, because
> significant parts of the code already run before relocation
> (including all clocks and timers setup, console setup, printf and all
> routines these pull in).
>

Have a look at x86 - Relocation is performed at the first possible moment

This made full relocation for x86 was relatively trivial :)

Regards,

Graeme

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Graeme Russ,

In message <AANLkTikNKFjUQ6Dmw3Ey=0qiEkiM716E=1+3nP3jG_ss@mail.gmail.com> you wrote:
>
> > That means you need to build all of U-Boot that way, because
> > significant parts of the code already run before relocation
> > (including all clocks and timers setup, console setup, printf and all
> > routines these pull in).
> 
> Have a look at x86 - Relocation is performed at the first possible moment
> 
> This made full relocation for x86 was relatively trivial :)

Well, U-Boot is not only a fancy boot loader, but also a hardware
bringup tool. It was designed to make it as easy for the software guy
to bring up code on new hardware. That means, that one of the very
first things we always try to do is get a (usually serial) console
port working, so we can use printf() to get some helpful information
out. This happens especially before doing anythign that is known to be
complicated and error prone, like especially the initialization of
both the memory controller and the RAM system on the board.

I am aware that there are systems out there which perform the RAM
initialization either in hardware or for example table-driven by some
built-in ROM boot loader code. Here RAM initalization is obviously not
such an issue, but nevertheless there is a LOT of code running before
we relocate the code to RAM.

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
"The glory of creation is in its infinite diversity." "And in the way
our differences combine to create meaning and beauty."
	-- Dr. Miranda Jones and Spock, "Is There in Truth No Beauty?",
	   stardate 5630.8

[U-Boot] ARM relocation, question to Heiko

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFE6153B0A.80735DD8-ONC12577B2.002963A5-C12577B2.0029D57C@transmode.se> you wrote:
>
> However, I think we will loose the possibility to add "link once, burn and run anywhere"
> feature I impl. once(but it was at the time deemed to intrusive) if
> we skip -fPIC and go for the linker -pie relocation.
> 
> I would suggest to leave ppc as is for the time being and
> see how -pie works out on the other archs.

I see two tasks here:

Prio 1: fix the current problems on ARM

Prio 2: make architectures as similar as possible.

My dream would be to have all that work in the same way (even if
eventually differen mechanisms need ti be deployed) on ARM, x86 and
PPC (and MIPS? and others?).

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
No question is too silly to ask. Of course, some  questions  are  too
silly to to answer...  - L. Wall & R. L. Schwartz, _Programming Perl_

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Heiko Schocher]

Hello Wolfgang, Reinhard,

Wolfgang Denk wrote:
> Dear Reinhard Meyer,
> 
> In message <4CA5D857.5010009@emk-elektronik.de> you wrote:
>> The environment issues still persist. I am at a loss
>> there now.
>>
>> Observation: the old style commands "setenv", "printenv", etc.
>> work, but any "env" command except for "env" alone crashes.
> 
> OK. If "printenv" works and "env print" fails then it has nothing to
> do with the environment code itself, as both call the same function.
> 
> It must have something to do with the implementation of subcommands
> then. See do_env() in "common/cmd_nvedit.c"; check if the command
> table address for find_cmd_tbl() is OK.
> 
> Eventually other commands with subcommands fail as well (i2c ?) ?

Yep, I think thats the direction. i2c should work, because the
subcommands get fixed in board_init_r() through i2c_reloc()

For fixing commandtable (and subcommands) I made a common
function fixup_cmdtable() in common/command.c see also
commit 620f1f6a64095ed558e68d37f1965d015cd49b02

Note: For powerpc this fixups are not needed, so
CONFIG_RELOC_FIXUP_WORKS is defined for powerpc ... don;t
sure, how exactly this work on powerpc ...

bye,
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5D26D.2090505@emk-elektronik.de> you wrote:
>
> > If this is really for all AT91 SoCs, then please feel free to
> > introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually
> > you can clean up some other such #if's on the way.
> 
> That would have to be set either in each board.h file or in each at91*.h
> file.

Isn't there a central place?

> > Probably there is a header file common to all AT91 SoCs when then can
> > use a single such construct to #define the new variable so you don;t
> > have to touch all the many board config files.
> 
> The only files I can see included in each instance are those of the kind
> hardware.h, memory_map.h and similiar. The define does not really belong
> in any of those.

If no better place is found, we can even add this to <common.h> (we do
similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x,
CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.

> Q: after relocation, "gd->" values are still available, right?

Yes, it gets relocated to RAM, too, and then remains available.

> I am searching for further "violations"...

Good luck!

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
EMACS belongs in <sys/errno.h>: Editor too big!

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,
> In message <4CA5D26D.2090505@emk-elektronik.de> you wrote:
>>> If this is really for all AT91 SoCs, then please feel free to
>>> introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually
>>> you can clean up some other such #if's on the way.
>> That would have to be set either in each board.h file or in each at91*.h
>> file.
> 
> Isn't there a central place?
> 
>>> Probably there is a header file common to all AT91 SoCs when then can
>>> use a single such construct to #define the new variable so you don;t
>>> have to touch all the many board config files.
>> The only files I can see included in each instance are those of the kind
>> hardware.h, memory_map.h and similiar. The define does not really belong
>> in any of those.

I tried arch-at91/hardware.h. Apparently it gets included after
global_data.h. Same would be true for all files included inside hardware.h.
Without reordering the includes (which I don't want to do) the remaining
place would be each board's config file. Since all AT91 boards are broken
anyway right now; adding it could be left to the respective maintainers :)

> If no better place is found, we can even add this to <common.h> (we do
> similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x,
> CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.

Into this chain?
#if defined(CONFIG_MPC852)	|| defined(CONFIG_MPC852T)	|| \
    defined(CONFIG_MPC859)	|| defined(CONFIG_MPC859T)	|| \
    defined(CONFIG_MPC859DSL)	|| \
    defined(CONFIG_MPC866)	|| defined(CONFIG_MPC866T)	|| \
    defined(CONFIG_MPC866P)
# define CONFIG_MPC866_FAMILY 1
#elif defined(CONFIG_MPC870) \

Ugly, but possible :)
Just hoping that everytime a new AT91 comes out, it will not be forgotten to be added there ;)

Reinhard

[U-Boot] AT91 clock and timer cleanups (was: ARM relocation, probably trivial mistake - back to original problem)

[Reinhard Meyer]

I wrote:
> Dear Wolfgang Denk,
>> In message <4CA5D26D.2090505@emk-elektronik.de> you wrote:
>>>> If this is really for all AT91 SoCs, then please feel free to
>>>> introduce a common define (CONFIG_SYS_AT91 ?) and use that. Eventually
>>>> you can clean up some other such #if's on the way.
>>> That would have to be set either in each board.h file or in each at91*.h
>>> file.
>> Isn't there a central place?
>>
>>>> Probably there is a header file common to all AT91 SoCs when then can
>>>> use a single such construct to #define the new variable so you don;t
>>>> have to touch all the many board config files.
>>> The only files I can see included in each instance are those of the kind
>>> hardware.h, memory_map.h and similiar. The define does not really belong
>>> in any of those.
> 
> I tried arch-at91/hardware.h. Apparently it gets included after
> global_data.h. Same would be true for all files included inside hardware.h.
> Without reordering the includes (which I don't want to do) the remaining
> place would be each board's config file. Since all AT91 boards are broken
> anyway right now; adding it could be left to the respective maintainers :)
> 
>> If no better place is found, we can even add this to <common.h> (we do
>> similar things there already for CONFIG_MPC866_FAMILY, CONFIG_MPC86x,
>> CONFIG_MPC8272_FAMILY, CONFIG_TQM8xxM, CONFIG_TQM8xxL, etc.
> 
> Into this chain?
> #if defined(CONFIG_MPC852)	|| defined(CONFIG_MPC852T)	|| \
>     defined(CONFIG_MPC859)	|| defined(CONFIG_MPC859T)	|| \
>     defined(CONFIG_MPC859DSL)	|| \
>     defined(CONFIG_MPC866)	|| defined(CONFIG_MPC866T)	|| \
>     defined(CONFIG_MPC866P)
> # define CONFIG_MPC866_FAMILY 1
> #elif defined(CONFIG_MPC870) \
> 
> Ugly, but possible :)
> Just hoping that everytime a new AT91 comes out, it will not be forgotten to be added there ;)

I have added required variables to global_data.h, it looks like
this now:

...
#ifdef CONFIG_FSL_ESDHC
	unsigned long	sdhc_clk;
#endif
#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9XE)
	/* "static data" needed by at91's clock.c */
 	unsigned long	cpu_clk_rate_hz;
	unsigned long	main_clk_rate_hz;
	unsigned long	mck_rate_hz;
	unsigned long	plla_rate_hz;
	unsigned long	pllb_rate_hz;
	unsigned long	at91_pllb_usb_init;
	/* "static data" needed by at91's timer.c */
	unsigned long	timer_rate_hz;
	unsigned long	tbl;
	unsigned long	tbu;
	unsigned long long	timer_reset_value;
#endif
#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
	unsigned long	relocaddr;	/* Start address of U-Boot in RAM */
...

At the same time I cleaned up timer.c to really operate using
64 bit values like on powerpc (hence tbu, tbl)

The Qs that remain now are:

1. currently I see about 8 CONFIG_AT91SAMxxx defines required
(not only 2 like above). The method is still undecided how to
handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put
all 8 defined() into "global_data.h" (would give 4 lines there)?

2. How many patches would this change need, and who would collect them?
It affects at worst 3 areas:
common.h, arm/global_data.h, at91/clock.c+timer.c ---
Or is it ok to put that into one patch and add it later to my atmel tree?

Reinhard

[U-Boot] AT91 clock and timer cleanups (was: ARM relocation, probably trivial mistake - back to original problem)

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA985DC.4000208@emk-elektronik.de> you wrote:
>
> The Qs that remain now are:
> 
> 1. currently I see about 8 CONFIG_AT91SAMxxx defines required
> (not only 2 like above). The method is still undecided how to
> handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put
> all 8 defined() into "global_data.h" (would give 4 lines there)?

You wille ventually need this in other locations as well, so please
add a single 4-line-#if to some appropriate header file; if there is
no better file for AT91 I will accept this for common.h

> 2. How many patches would this change need, and who would collect them?
> It affects at worst 3 areas:
> common.h, arm/global_data.h, at91/clock.c+timer.c ---
> Or is it ok to put that into one patch and add it later to my atmel tree?

I see two patches:

Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY
instead.

Patch 2 will fix at91/clock.c+timer.c

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
They say a little knowledge is a dangerous thing,  but it is not  one
half so bad as a lot of ignorance.   - Terry Pratchett, _Equal Rites_

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,
>> The Qs that remain now are:
>>
>> 1. currently I see about 8 CONFIG_AT91SAMxxx defines required
>> (not only 2 like above). The method is still undecided how to
>> handle this. Add "CONFIG_AT91SAM9_FAMILY" to "common.h"? Or put
>> all 8 defined() into "global_data.h" (would give 4 lines there)?
> 
> You wille ventually need this in other locations as well, so please
> add a single 4-line-#if to some appropriate header file; if there is
> no better file for AT91 I will accept this for common.h

There is no at91 common header file that gets included BEFORE global_data.h,
unfortunately.

> 
>> 2. How many patches would this change need, and who would collect them?
>> It affects at worst 3 areas:
>> common.h, arm/global_data.h, at91/clock.c+timer.c ---
>> Or is it ok to put that into one patch and add it later to my atmel tree?
> 
> I see two patches:
> 
> Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY
> instead.
> 
> Patch 2 will fix at91/clock.c+timer.c

Maybe there is a misunderstanding here. I see three patches

1. introduce CONFIG_AT91SAM9_FAMILY to common.h

2. add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
required variables

3. change clock and timer.

2+3 combined in one patch? OK

And I bet, this must be a patch series ;)

Reinhard

[U-Boot] AT91 clock and timer cleanups

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA99373.7020704@emk-elektronik.de> you wrote:
>
> > Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY
> > instead.
> > 
> > Patch 2 will fix at91/clock.c+timer.c
> 
> Maybe there is a misunderstanding here. I see three patches
> 
> 1. introduce CONFIG_AT91SAM9_FAMILY to common.h
> 
> 2. add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
> required variables
> 
> 3. change clock and timer.
> 
> 2+3 combined in one patch? OK

No.  1 + 2 belong together; they deal with the same topic and should
be applied as a single commit.

3 is a completely different story and goes in a separate patch.

> And I bet, this must be a patch series ;)

Not necessarily. The 1+2 combo is independent from 3 (but needed as a
prerequisite, so posting this in a series is fine as well).

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
Lots of people drink from the wrong bottle sometimes.
	-- Edith Keeler, "The City on the Edge of Forever",
	   stardate unknown

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,
>>> Patch 1 will change the #if and introduce CONFIG_AT91SAM9_FAMILY
>>> instead.
>>>
>>> Patch 2 will fix at91/clock.c+timer.c
>> Maybe there is a misunderstanding here. I see three patches
>>
>> 1. introduce CONFIG_AT91SAM9_FAMILY to common.h
>>
>> 2. add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
>> required variables
>>
>> 3. change clock and timer.
>>
>> 2+3 combined in one patch? OK
> 
> No.  1 + 2 belong together; they deal with the same topic and should
> be applied as a single commit.

Ok, so introducing (at this point still) unreferenced variables here is OK?

> 3 is a completely different story and goes in a separate patch.
> 
>> And I bet, this must be a patch series ;)
> 
> Not necessarily. The 1+2 combo is independent from 3 (but needed as a
> prerequisite, so posting this in a series is fine as well).

Ok, I rather post it independant, if thats OK, patch series are still
more trouble ;)


Best Regards,

Reinhard

[U-Boot] AT91 clock and timer cleanups

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA995D7.5010008@emk-elektronik.de> you wrote:
>
> >> Maybe there is a misunderstanding here. I see three patches
> >>
> >> 1. introduce CONFIG_AT91SAM9_FAMILY to common.h
> >>
> >> 2. add an #ifdef CONFIG_AT91SAM9_FAMILY to global_data.h with the
> >> required variables
> >>
> >> 3. change clock and timer.
> >>
> >> 2+3 combined in one patch? OK
> > 
> > No.  1 + 2 belong together; they deal with the same topic and should
> > be applied as a single commit.
> 
> Ok, so introducing (at this point still) unreferenced variables here is OK?

It will not be unreferenced.

It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) ||
defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar
ocurrences like these:

arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
arch/arm/cpu/arm926ejs/at91/lowlevel_init.S:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) \
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260)  || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9263)
arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9260)
arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263)
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \
arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45)
arch/arm/include/asm/arch-at91/at91_pio.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
arch/arm/include/asm/arch-at91/at91_pio.h:      defined(CONFIG_AT91SAM9G10) || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/at91_pio.h:#elif defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) || \
arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9G10)
arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
drivers/i2c/soft_i2c.c: defined(CONFIG_AT91SAM9260) ||  defined(CONFIG_AT91SAM9261) || \
drivers/net/macb.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
drivers/net/macb.c:     defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
drivers/net/macb.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
drivers/net/macb.c:     defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
drivers/usb/host/ohci-at91.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
drivers/usb/host/ohci-at91.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20)
drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
include/i2c.h:  defined(CONFIG_AT91SAM9260) ||  defined(CONFIG_AT91SAM9261) || \


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
He'd heard her use that sweet, innocent  tone  of  voice  before.  It
meant that, pretty soon, there was going to be trouble.
                                        - Terry Pratchett, _Truckers_

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,

I see the misunderstanding here:

> It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) ||
> defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar
> ocurrences like these:

That does not exist yet (its only in my local tree so far!)

> arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> arch/arm/cpu/arm926ejs/at91/lowlevel_init.S:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) \
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260)  || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9263)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9260)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263)
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9263) || \
> arch/arm/include/asm/arch-at91/at91_matrix.h:#if defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45)
> arch/arm/include/asm/arch-at91/at91_pio.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9261) || \
> arch/arm/include/asm/arch-at91/at91_pio.h:      defined(CONFIG_AT91SAM9G10) || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/at91_pio.h:#elif defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G45) || \
> arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#if defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/at91sam9_matrix.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
> arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9261) || defined(CONFIG_AT91SAM9G10)
> arch/arm/include/asm/arch-at91/hardware.h:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> drivers/i2c/soft_i2c.c: defined(CONFIG_AT91SAM9260) ||  defined(CONFIG_AT91SAM9261) || \
> drivers/net/macb.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
> drivers/net/macb.c:     defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> drivers/net/macb.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
> drivers/net/macb.c:     defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> drivers/usb/host/ohci-at91.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
> drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> drivers/usb/host/ohci-at91.c:    defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20)
> drivers/usb/host/ohci-at91.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
> include/i2c.h:  defined(CONFIG_AT91SAM9260) ||  defined(CONFIG_AT91SAM9261) || \

Careful here, the ones here are distinguishing between different "family" members!
See the abundance of #elif's there!

Reinhard

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,
> 
> I see the misunderstanding here:
> 
>> It will be needed to replace the "#if defined(CONFIG_AT91SAM9260) ||
>> defined(CONFIG_AT91SAM9XE)" in global_data.h and a ton of similar
>> ocurrences like these:
> 
> That does not exist yet (its only in my local tree so far!)
> 
>> arch/arm/cpu/arm926ejs/at91/clock.c:#if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
>> arch/arm/cpu/arm926ejs/at91/clock.c:#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)

>> Careful here, the ones here are distinguishing between different "family" members!
>> See the abundance of #elif's there!

The actual example code:
#if defined(CONFIG_AT91RM9200)
	/* mdiv */
	gd->mck_rate_hz = freq / (1 + ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8));
#elif defined(CONFIG_AT91SAM9G20)
	/* mdiv ; (x >> 7) = ((x >> 8) * 2) */
	gd->mck_rate_hz = (mckr & AT91_PMC_MCKR_MDIV_MASK) ?
		freq / ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 7) : freq;
	if (mckr & AT91_PMC_MCKR_MDIV_MASK)
		freq /= 2;			/* processor clock division */
#elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45)
	gd->mck_rate_hz = (mckr & AT91_PMC_MCKR_MDIV_MASK) ==
		(AT91_PMC_MCKR_MDIV_2 | AT91_PMC_MCKR_MDIV_4)
		? freq / 3
		: freq / (1 << ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8));
#else
	gd->mck_rate_hz = freq / (1 << ((mckr & AT91_PMC_MCKR_MDIV_MASK) >> 8));
#endif

Note: I did not write that code, and I am sure it could be made to look less
obfuscated. But that's not my problem right now.

I looked at common.h which is already overcrowded by arch and even
board specifics. I don't want to add anything there. In my tree I have
solved the issue as follows:

1. The board's config file defines "CONFIG_AT91FAMILY" like this:
/* SoC */
#define CONFIG_ARM926EJS	1	/* ARM926EJS Core	*/
#define	CONFIG_AT91FAMILY	1	/* it's a member of AT91 */
#define CONFIG_AT91SAM9260	1	/* Atmel AT91SAM9260 based SoC */
#define CONFIG_AT91SAM9XE	1	/* more specific: AT91SAM9XE */

2. both at91 clock.c and timer.c contain the statements:
#if !defined(CONFIG_AT91FAMILY)
# error You need to define CONFIG_AT91FAMILY in your board config!
#endif
This will catch all boards that are affected. Since all ARM/AT91 boards
are broken right now anyway and need fixing their config file that
should work fine.

3. arm/global_data.h has now:
...
#endif
#ifdef CONFIG_AT91FAMILY
	/* "static data" needed by at91's clock.c */
 	unsigned long	cpu_clk_rate_hz;
	unsigned long	main_clk_rate_hz;
	unsigned long	mck_rate_hz;
	unsigned long	plla_rate_hz;
	unsigned long	pllb_rate_hz;
	unsigned long	at91_pllb_usb_init;
	/* "static data" needed by at91's timer.c */
	unsigned long	timer_rate_hz;
	unsigned long	tbl;
	unsigned long	tbu;
	unsigned long long	timer_reset_value;
#endif
#if !defined(CONFIG_SYS_ARM_WITHOUT_RELOC)
...

I can now post this as a patch.

Best Regards
Reinhard

[U-Boot] AT91 clock and timer cleanups

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA9EB88.5000203@emk-elektronik.de> you wrote:
>
> 1. The board's config file defines "CONFIG_AT91FAMILY" like this:
> /* SoC */
> #define CONFIG_ARM926EJS	1	/* ARM926EJS Core	*/
> #define	CONFIG_AT91FAMILY	1	/* it's a member of AT91 */
> #define CONFIG_AT91SAM9260	1	/* Atmel AT91SAM9260 based SoC */
> #define CONFIG_AT91SAM9XE	1	/* more specific: AT91SAM9XE */

Out of principle: please omit the "1" in all such cases unless you
really want to pass or test for such a numeric value anywhere. As far
as I can tell all relevant code uses only "#ifdef" or "defined(...)",
so not to confuse people better omit the irrelevant value.


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
Hiring experienced unix people is  like  a  built-in  filter  against
idiots. Hiring experienced NT people provides no such guarantee.
            -- Miguel Cruz in WgL96.349$CC.122704 at typhoon2.ba-dsg.net

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,
> In message<4CA9EB88.5000203@emk-elektronik.de>  you wrote:
>>
>> 1. The board's config file defines "CONFIG_AT91FAMILY" like this:
>> /* SoC */
>> #define CONFIG_ARM926EJS	1	/* ARM926EJS Core	*/
>> #define	CONFIG_AT91FAMILY	1	/* it's a member of AT91 */
>> #define CONFIG_AT91SAM9260	1	/* Atmel AT91SAM9260 based SoC */
>> #define CONFIG_AT91SAM9XE	1	/* more specific: AT91SAM9XE */
>
> Out of principle: please omit the "1" in all such cases unless you
> really want to pass or test for such a numeric value anywhere. As far
> as I can tell all relevant code uses only "#ifdef" or "defined(...)",
> so not to confuse people better omit the irrelevant value.

I agree here, and will change that whenever I encounter it in files
I touch (most is copied from the atmel EK config file).
I also see the tab vs. space issue once its quoted.
I think a space after the #define is better.

The general question, however was, if that approach is OK with you.

Best Regards,
Reinhard

[U-Boot] AT91 clock and timer cleanups

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CAA0BB1.7010608@emk-elektronik.de> you wrote:
>
> I agree here, and will change that whenever I encounter it in files
> I touch (most is copied from the atmel EK config file).

Thanks.

> The general question, however was, if that approach is OK with you.

Well, I still prefer to have this added to a single, central location
instead to a large number of board config files.

However, if you have to touch all these board config files anyway (and
accept the need to re-test on all affected boards) then I don't insist
on any specific implementation.

Please don't forget to document the new CONFOG_ variable (in the
README).

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
Too much of anything, even love, isn't necessarily a good thing.
	-- Kirk, "The Trouble with Tribbles", stardate 4525.6

[U-Boot] AT91 clock and timer cleanups

[Reinhard Meyer]

Dear Wolfgang Denk,
>> The general question, however was, if that approach is OK with you.
>
> Well, I still prefer to have this added to a single, central location
> instead to a large number of board config files.

AT91 does not have a really large number of config files.

>
> However, if you have to touch all these board config files anyway (and
> accept the need to re-test on all affected boards) then I don't insist
> on any specific implementation.

No, I can't re-test on other AT91 boards. With the relocation patch
none of them builds right now, and without the clock/timer cleanup
they would not work, even if they would build. Since the respective
maintainers are required to fix for relocation they can also add the
define required to make the relocated code work...

Currently, a "./MAKEALL at91" is pointless.

> Please don't forget to document the new CONFOG_ variable (in the
> README).

In which README and where in it? So far in no README are such variables
listed/explained (of type CONFIG_<arch>,<SoC>,<Family>)

Best regards,

Reinhard

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA590E6.6070701@emk-elektronik.de> you wrote:
> 
> it seems, that with relocation enabled, some
> data does not seem to get initialized properly:

I rather suspect you have code running that violates the
pre-relocation restrictions (no bss segment available, read-only data
segment).

> Do I understand right what the required changes are:
> 
> 1. change dram_init
> 2. make sure TEXT_BASE is correct as to where u-boot is loaded
> by a preloader and is NOT pointing near top of RAM.

...or identical to the final position in RAM.

> 3. I do not have a board specific .lds - should I now have one?

No need for that.

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
Without facts, the decision cannot be made logically. You  must  rely
on your human intuition.
	-- Spock, "Assignment: Earth", stardate unknown

[U-Boot] ARM relocation, probably trivial mistake - back to original problem

[Reinhard Meyer]

Dear Wolfgang Denk,
> Dear Reinhard Meyer,
> 
> In message <4CA590E6.6070701@emk-elektronik.de> you wrote:
>> it seems, that with relocation enabled, some
>> data does not seem to get initialized properly:
> 
> I rather suspect you have code running that violates the
> pre-relocation restrictions (no bss segment available, read-only data
> segment).

Definitely, since AT91 ALWAYS uses a preloader and therefore
until now u-boot was loaded to its final location with working bss,
nobody ever before observed any problems therefore.

There has never been a real need for relocation before.

That means quite some common AT91 code has to be checked and adjusted
because this upcoming "enforcement" of relocation.

> ...or identical to the final position in RAM.

Which never is a good idea.

And Wolfgang, very begin of SDRAM is meant seeing through
"AT91 glasses", where SDRAM never holds any vectors. One has
to be watchful about the initial stack, which currently is at
the begin of SDRAM, too. But that could be put inside the SRAM.

So, for AT91, I think I will put stack into the 4/32k SRAM,
U-Boot at the very beginning. Unfortunately that involves
modifying the preloader as well.

I suspect Atmel will not modify the source of their evaluation
boards, so those will fall out of u-boot soon.

Best Regards,
Reinhard

[U-Boot] ARM relocation, probably trivial mistake

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4CA5821E.3070108@emk-elektronik.de> you wrote:
> 
> I think it would be safe that a preloader loads u-boot to the very
> bottom of SDRAM. If the SDRAM is not at least double the size of
> u-boot, u-boot needs a diet ;)

This may or may not be safe. Some systems (for example, PowerPC) uses
low RAM for exception vectors, and you definitely don;t want to mess
with these.

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
"Life, loathe it or ignore it, you can't like it."
- Marvin the paranoid android

[U-Boot] ARM relocation, probably trivial mistake

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4CA57762.3000201@denx.de> you wrote:
> 
> If u-boot starts in RAM, then it is the task from the preloader
> where it copies u-boot code, and if there is somewhere in IRAM
> enough room for it, this would be an option. Otherwise it is a
> problem if relocation results in overlapping source and destination
> areas ... but you will fast detect this problem, when you see,
> that u-boot no longer works ;-)

I think we have to be careful here.

Please keep in mind that the with the new setup the relocation address
is not a constant, not even for systems that come with a fixed memroy
configuration.

If you enable for example the protected RAM feature, the relocation
address will be shifted down by the amount needed for the reserved
PRAM area - which is variable, as it can be set through an
environment variable.

So assume you have a system with a preloader, and you optimize your
configuration to load U-Bot to the "final" position close to the end
of the RAM.  Not the user defines "setenv pram 128" and reboots.
Now U-Boot will try to relocate itself 128 kB down, which pretty
reliably causes an overlap.

Eventually we have to detect such situations and relocate twice then?

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
The shortest unit of time in the multiverse is the News York  Second,
defined  as  the  period  of  time between the traffic lights turning
green and the cab behind you honking.
                                - Terry Pratchett, _Lords and Ladies_

[U-Boot] ARM relocation, probably trivial mistake

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <4CA57762.3000201@denx.de> you wrote:
>> If u-boot starts in RAM, then it is the task from the preloader
>> where it copies u-boot code, and if there is somewhere in IRAM
>> enough room for it, this would be an option. Otherwise it is a
>> problem if relocation results in overlapping source and destination
>> areas ... but you will fast detect this problem, when you see,
>> that u-boot no longer works ;-)
> 
> I think we have to be careful here.
> 
> Please keep in mind that the with the new setup the relocation address
> is not a constant, not even for systems that come with a fixed memroy
> configuration.
> 
> If you enable for example the protected RAM feature, the relocation
> address will be shifted down by the amount needed for the reserved
> PRAM area - which is variable, as it can be set through an
> environment variable.
> 
> So assume you have a system with a preloader, and you optimize your
> configuration to load U-Bot to the "final" position close to the end
> of the RAM.  Not the user defines "setenv pram 128" and reboots.
> Now U-Boot will try to relocate itself 128 kB down, which pretty
> reliably causes an overlap.

Good point ...

> Eventually we have to detect such situations and relocate twice then?

Hmm.. just an idea:

We calculate the relocation address in arch/arm/lib/board.c board_init_f()

So, it should be possible to add a check if

relocation_address + u-boot_len < TEXT_BASE

If this is not the case, we can try to relocate twice ... but this
will result in one more copy of u-boot ...

... easier would be, not to use such a setting (relocate address ==
TEXT_BASE) on a board which uses such features ...

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany