[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[Joakim Tjernlund]

Remove dependencies on link address. Use GOT and
add an new function to calculate the actual address.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/cpu/mpc83xx/start.S |   35 +++++++++++++++++++++++++++--------
 1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S
index 0c8e884..1ab8c88 100644
--- a/arch/powerpc/cpu/mpc83xx/start.S
+++ b/arch/powerpc/cpu/mpc83xx/start.S
@@ -71,12 +71,12 @@
  */
 	START_GOT
 	GOT_ENTRY(_GOT2_TABLE_)
+	GOT_ENTRY(_start)
 	GOT_ENTRY(__bss_start)
 	GOT_ENTRY(_end)
 
 #ifndef CONFIG_NAND_SPL
 	GOT_ENTRY(_FIXUP_TABLE_)
-	GOT_ENTRY(_start)
 	GOT_ENTRY(_start_of_vectors)
 	GOT_ENTRY(_end_of_vectors)
 	GOT_ENTRY(transfer_to_handler)
@@ -229,10 +229,12 @@ _start: /* time t 0 */
 	/* there and deflate the flash size back to minimal size      */
 	/*------------------------------------------------------------*/
 	bl map_flash_by_law1
-	lis r4, (CONFIG_SYS_MONITOR_BASE)@h
-	ori r4, r4, (CONFIG_SYS_MONITOR_BASE)@l
-	addi r5, r4, in_flash - _start + EXC_OFF_SYS_RESET
-	mtlr r5
+
+	bl	1f
+1:	mflr	r3   /* get current address */
+	addi	r3, r3, in_flash - 1b
+	bl	add_flash_base
+	mtlr r3
 	blr
 in_flash:
 #if 1 /* Remapping flash with LAW0. */
@@ -831,10 +833,11 @@ relocate_code:
 	bl	_GLOBAL_OFFSET_TABLE_ at local-4
 	mflr	r30
 #endif
-	mr	r3,  r5				/* Destination Address */
-	lis	r4, CONFIG_SYS_MONITOR_BASE at h		/* Source      Address */
-	ori	r4, r4, CONFIG_SYS_MONITOR_BASE at l
+	lwz	r4, GOT(_start)	/* Source Address */
+	addi	r4, r4, -EXC_OFF_SYS_RESET
 	lwz	r5, GOT(__bss_start)
+	mr	r3, r10	/* Destination Address */
+
 	sub	r5, r5, r4
 	li	r6, CONFIG_SYS_CACHELINE_SIZE		/* Cache Line Size */
 
@@ -1128,6 +1131,21 @@ unlock_ram_in_cache:
 #endif /* CONFIG_SYS_INIT_RAM_LOCK */
 
 #ifdef CONFIG_SYS_FLASHBOOT
+
+add_flash_base:
+	/* Check if already inside flash address space. */
+	/* if so, do not add CONFIG_SYS_FLASH_BASE */
+	lis	r4, (CONFIG_SYS_FLASH_BASE)@h
+	ori	r4, r4, (CONFIG_SYS_FLASH_BASE)@l
+	cmplw	cr0, r3, r4
+	ble	cr0, 2f /* r3 < r4 ? */
+	lis	r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@h
+	ori	r6, r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@l
+	cmplw	cr0, r3, r6
+	blelr	cr0	 /* r3 < r6 ? */
+2:	add	r3,r3,r4
+	blr
+
 map_flash_by_law1:
 	/* When booting from ROM (Flash or EPROM), clear the  */
 	/* Address Mask in OR0 so ROM appears everywhere      */
@@ -1179,6 +1197,7 @@ map_flash_by_law1:
 	 */
 remap_flash_by_law0:
 	/* Initialize the BR0 with the boot ROM starting address. */
+	lis r3, (CONFIG_SYS_IMMR)@h  /* r3 <= CONFIG_SYS_IMMR    */
 	lwz r4, BR0(r3)
 	li  r5, 0x7FFF
 	and r4, r4, r5
-- 
1.7.2.2

[U-Boot] [PATCH 2/4] mpc83xx: Add link vs. load address calculation

[Joakim Tjernlund]

link_off calculates the difference between link address and
actual load address. This is a must for true PIC u-boot.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/cpu/mpc83xx/start.S |   26 ++++++++++++++++++++++++++
 include/common.h                 |    7 +++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S
index 1ab8c88..16aed0a 100644
--- a/arch/powerpc/cpu/mpc83xx/start.S
+++ b/arch/powerpc/cpu/mpc83xx/start.S
@@ -419,6 +419,32 @@ ProgramCheck:
 _end_of_vectors:
 
 	. = 0x3000
+#ifdef CONFIG_SYS_TRUE_PIC
+	.globl	link_off /* const void * link_off(const void * ptr) */
+	.type	link_off, @function
+	/*
+	 * Calculates the offset between link address and load address
+	 * and subtracs the offset to from its argument(r3)
+	 * This function must be where _GOT2_TABLE_ is defined
+	 */
+link_off:
+	/* GOT hand coded as we cannot clobber r12 */
+	mflr	r4
+	bl	1f
+	.text	2
+0:	.long	.LCTOC1-1f
+	.text
+1:	mflr	r6
+	lwz	r0,0b-1b(r6)
+	add	r6,r0,r6
+	mtlr	r4
+	la	r4,.L__GOT2_TABLE_(r6) /* addi	r4,r6,.L__GOT2_TABLE_ */
+	lwz	r5,.L__GOT2_TABLE_(r6)
+	sub	r4,r5,r4 /* r4 - r5 */
+	sub	r3,r3,r4 /* r4 - r3 */
+	blr
+	.size	link_off, .-link_off
+#endif
 
 /*
  * This code finishes saving the registers to the exception frame
diff --git a/include/common.h b/include/common.h
index 189ad81..a04bd27 100644
--- a/include/common.h
+++ b/include/common.h
@@ -112,6 +112,13 @@ typedef volatile unsigned char	vu_char;
 #include <asm/arch/hardware.h>
 #endif
 
+#ifdef CONFIG_SYS_TRUE_PIC
+const void * link_off(const void *);
+#else
+#define link_off(x) ((const void *)(x))
+#endif
+#define LINK_OFF(x) ((__typeof__(x))link_off(x))
+
 #include <part.h>
 #include <flash.h>
 #include <image.h>
-- 
1.7.2.2

[U-Boot] [PATCH 3/4] mpc83xx: Add true PIC support.

[Joakim Tjernlund]

By copying the GOT to the end of the INIT_RAM(dcache)
and relocating it there, it is much esier to
support true PIC on u-boot. This cannot handle
FIXUP so C code that depends on fixups before relocation to RAM
must use LINK_OFF to calculate the difference.

This depends on the upcoming single-pic-base option
to gcc.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/cpu/mpc83xx/start.S |   36 ++++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S
index 16aed0a..af44fdd 100644
--- a/arch/powerpc/cpu/mpc83xx/start.S
+++ b/arch/powerpc/cpu/mpc83xx/start.S
@@ -291,6 +291,34 @@ in_flash:
 	/* Needed for upcoming -msingle-pic-base */
 	bl	_GLOBAL_OFFSET_TABLE_ at local-4
 	mflr	r30
+#ifdef CONFIG_SYS_TRUE_PIC
+	/*
+	 * Copy GOT to cache and relocate it
+	 * This assumes there is enough space at the end
+	 * INIT_RAM to hold a copy of the GOT.
+	 */
+	li r3,0
+	bl link_off /* r3 holds link offset at return */
+	lis r9, (CONFIG_SYS_INIT_RAM_ADDR+CONFIG_SYS_INIT_RAM_SIZE)@h
+	ori r9,r9,(CONFIG_SYS_INIT_RAM_ADDR+CONFIG_SYS_INIT_RAM_SIZE)@l
+	lwz r11,GOT(_GOT2_TABLE_)
+	add r11,r11,r3
+
+	li r4, __got2_entries at sectoff@l
+	mtctr r4
+	slwi r5,r4,2 /* r4 * 4 */
+	subf r9,r5,r9 /* r9 - r5 */
+
+	subf r10,r9,r11 /* r11 - r9 */
+	subf r30,r10,r30 /* r30 - r10, point PIC(r30) to new GOT */
+	addi r11,r11,-4
+	addi r9,r9,-4
+1:	/* copy GOT and add link offset */
+	lwzu r0,4(r11)
+	add r0,r0,r3
+	stwu r0,4(r9)
+	bdnz 1b
+#endif
 #endif
 	/* r3: IMMR */
 	lis	r3, CONFIG_SYS_IMMR at h
@@ -859,9 +887,17 @@ relocate_code:
 	bl	_GLOBAL_OFFSET_TABLE_ at local-4
 	mflr	r30
 #endif
+#ifdef CONFIG_SYS_TRUE_PIC
+	li	r3, 0
+	bl	link_off /* const void * link_off(const void *) */
+#endif
 	lwz	r4, GOT(_start)	/* Source Address */
 	addi	r4, r4, -EXC_OFF_SYS_RESET
 	lwz	r5, GOT(__bss_start)
+#ifdef CONFIG_SYS_TRUE_PIC
+	add	r4, r4, r3 /* Add link offset */
+	add	r5, r5, r3 /* Add link offset */
+#endif
 	mr	r3, r10	/* Destination Address */
 
 	sub	r5, r5, r4
-- 
1.7.2.2

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Only these 2 call sites depends on fixups for my mpc8321 based
board.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
 arch/powerpc/lib/board.c            |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c
index 7a1cae7..88d9dd8 100644
--- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
@@ -507,7 +507,7 @@ int prt_83xx_rsr(void)
 	sep = " ";
 	for (i = 0; i < n; i++)
 		if (rsr & bits[i].mask) {
-			printf("%s%s", sep, bits[i].desc);
+			printf("%s%s", sep, LINK_OFF(bits[i].desc));
 			sep = ", ";
 		}
 	puts("\n");
diff --git a/arch/powerpc/lib/board.c b/arch/powerpc/lib/board.c
index 9759e23..4ffec36 100644
--- a/arch/powerpc/lib/board.c
+++ b/arch/powerpc/lib/board.c
@@ -386,7 +386,7 @@ void board_init_f (ulong bootflag)
 #endif
 
 	for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
-		if ((*init_fnc_ptr) () != 0) {
+		if ((LINK_OFF(*init_fnc_ptr)) () != 0) {
 			hang ();
 		}
 	}
-- 
1.7.2.2

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1292838435-14958-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Only these 2 call sites depends on fixups for my mpc8321 based
> board.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
>  arch/powerpc/lib/board.c            |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> index 7a1cae7..88d9dd8 100644
> --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
> +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> @@ -507,7 +507,7 @@ int prt_83xx_rsr(void)
>  	sep = " ";
>  	for (i = 0; i < n; i++)
>  		if (rsr & bits[i].mask) {
> -			printf("%s%s", sep, bits[i].desc);
> +			printf("%s%s", sep, LINK_OFF(bits[i].desc));
>  			sep = ", ";
>  		}


Is my understanding correct that these changes are sufficient only for
your board, and only for your current configuration?  And that your
code would break (resp. require more LINK_OFF fixups) if you would -
for example - decide to enable CONFIG_DISPLAY_AER_FULL in your board
configuration (cf. print_83xx_arb_event() above in the same source
file) ?

I object against such a fragile and insular approach.



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
It is impractical for  the  standard  to  attempt  to  constrain  the
behavior  of code that does not obey the constraints of the standard.
                                                          - Doug Gwyn

[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1292838435-14958-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> Remove dependencies on link address. Use GOT and
> add an new function to calculate the actual address.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  arch/powerpc/cpu/mpc83xx/start.S |   35 +++++++++++++++++++++++++++--------
>  1 files changed, 27 insertions(+), 8 deletions(-)

It seems this code introduces some subtle changes.

> -	lis r4, (CONFIG_SYS_MONITOR_BASE)@h
> -	ori r4, r4, (CONFIG_SYS_MONITOR_BASE)@l
> -	addi r5, r4, in_flash - _start + EXC_OFF_SYS_RESET
> -	mtlr r5

The original code references CONFIG_SYS_MONITOR_BASE.

> +	bl	add_flash_base
...
> +add_flash_base:
> +	/* Check if already inside flash address space. */
> +	/* if so, do not add CONFIG_SYS_FLASH_BASE */
> +	lis	r4, (CONFIG_SYS_FLASH_BASE)@h
> +	ori	r4, r4, (CONFIG_SYS_FLASH_BASE)@l
> +	cmplw	cr0, r3, r4
> +	ble	cr0, 2f /* r3 < r4 ? */
> +	lis	r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@h
> +	ori	r6, r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@l
> +	cmplw	cr0, r3, r6
> +	blelr	cr0	 /* r3 < r6 ? */
> +2:	add	r3,r3,r4
> +	blr

But your new code does not reference CONFIG_SYS_MONITOR_BASE at all,
but uses CONFIG_SYS_FLASH_BASE instead.


On which boards has this been tested?

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 many DEC repairman does it take to fix a flat ?
A:  Five; four to hold the car up and one to swap tires.

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/09 21:29:04:
>
> Dear Joakim Tjernlund,
>
> In message <1292838435-14958-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Only these 2 call sites depends on fixups for my mpc8321 based
> > board.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
> >  arch/powerpc/lib/board.c            |    2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > index 7a1cae7..88d9dd8 100644
> > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > @@ -507,7 +507,7 @@ int prt_83xx_rsr(void)
> >     sep = " ";
> >     for (i = 0; i < n; i++)
> >        if (rsr & bits[i].mask) {
> > -         printf("%s%s", sep, bits[i].desc);
> > +         printf("%s%s", sep, LINK_OFF(bits[i].desc));
> >           sep = ", ";
> >        }
>
>
> Is my understanding correct that these changes are sufficient only for
> your board, and only for your current configuration?  And that your
> code would break (resp. require more LINK_OFF fixups) if you would -
> for example - decide to enable CONFIG_DISPLAY_AER_FULL in your board
> configuration (cf. print_83xx_arb_event() above in the same source
> file) ?

It would break only if link address != load address. That is, if you
want to use my new CONFIG_SYS_TRUE_PIC feature and be able to load
u-boot at any address regardless of link address you would
have to add LINK_OFF calls into print_83xx_arb_event() too if
you want to use it.

>
> I object against such a fragile and insular approach.

Considering you were tempted to add my previous approach which
had LINK_OFF calls all over I don't see were this objection comes
from. Have you changed your mind?

 Jocke

[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/09 21:44:08:
>
> Dear Joakim Tjernlund,
>
> In message <1292838435-14958-1-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > Remove dependencies on link address. Use GOT and
> > add an new function to calculate the actual address.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >  arch/powerpc/cpu/mpc83xx/start.S |   35 +++++++++++++++++++++++++++--------
> >  1 files changed, 27 insertions(+), 8 deletions(-)
>
> It seems this code introduces some subtle changes.
>
> > -   lis r4, (CONFIG_SYS_MONITOR_BASE)@h
> > -   ori r4, r4, (CONFIG_SYS_MONITOR_BASE)@l
> > -   addi r5, r4, in_flash - _start + EXC_OFF_SYS_RESET
> > -   mtlr r5
>
> The original code references CONFIG_SYS_MONITOR_BASE.
>
> > +   bl   add_flash_base
> ...
> > +add_flash_base:
> > +   /* Check if already inside flash address space. */
> > +   /* if so, do not add CONFIG_SYS_FLASH_BASE */
> > +   lis   r4, (CONFIG_SYS_FLASH_BASE)@h
> > +   ori   r4, r4, (CONFIG_SYS_FLASH_BASE)@l
> > +   cmplw   cr0, r3, r4
> > +   ble   cr0, 2f /* r3 < r4 ? */
> > +   lis   r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@h
> > +   ori   r6, r6, (CONFIG_SYS_FLASH_BASE+(CONFIG_SYS_FLASH_SIZE*1024*1024-1))@l
> > +   cmplw   cr0, r3, r6
> > +   blelr   cr0    /* r3 < r6 ? */
> > +2:   add   r3,r3,r4
> > +   blr
>
> But your new code does not reference CONFIG_SYS_MONITOR_BASE at all,
> but uses CONFIG_SYS_FLASH_BASE instead.

You can't assume a fixed address when doing PIC therefore the change.

>
>
> On which boards has this been tested?

Only on our custom 83xx boards.

[U-Boot] [PATCH 3/4] mpc83xx: Add true PIC support.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <1292838435-14958-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> By copying the GOT to the end of the INIT_RAM(dcache)
> and relocating it there, it is much esier to
> support true PIC on u-boot. This cannot handle
> FIXUP so C code that depends on fixups before relocation to RAM
> must use LINK_OFF to calculate the difference.
...
> +	/*
> +	 * Copy GOT to cache and relocate it
> +	 * This assumes there is enough space at the end
> +	 * INIT_RAM to hold a copy of the GOT.
> +	 */

Is it correct to assume that system will crash horribly if for some
reason the GOT should not fit?

And this means that there is no way to adapt this approach to systems
where initial RAM is restrictd and not big enough to hold a copy of
the GOT?

[U-Boot] [PATCH 3/4] mpc83xx: Add true PIC support.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/09 21:54:34:
>
> Dear Joakim Tjernlund,
>
> In message <1292838435-14958-3-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > By copying the GOT to the end of the INIT_RAM(dcache)
> > and relocating it there, it is much esier to
> > support true PIC on u-boot. This cannot handle
> > FIXUP so C code that depends on fixups before relocation to RAM
> > must use LINK_OFF to calculate the difference.
> ...
> > +   /*
> > +    * Copy GOT to cache and relocate it
> > +    * This assumes there is enough space at the end
> > +    * INIT_RAM to hold a copy of the GOT.
> > +    */
>
> Is it correct to assume that system will crash horribly if for some
> reason the GOT should not fit?

Yes ATM. I considered adding tests but it will cost more space and I
am not sure how to report the error this early.

>
> And this means that there is no way to adapt this approach to systems
> where initial RAM is restrictd and not big enough to hold a copy of
> the GOT?

Can't think of one, you need somewhere to put the GOT. You can work on
reducing the GOT size though.

      Jocke

[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF7F8BAE70.CE1486C7-ONC1257813.00727B82-C1257813.0072BE1B@transmode.se> you wrote:
>
> > But your new code does not reference CONFIG_SYS_MONITOR_BASE at all,
> > but uses CONFIG_SYS_FLASH_BASE instead.
> 
> You can't assume a fixed address when doing PIC therefore the change.

I understand this change is intentionally, then?

> > On which boards has this been tested?
> 
> Only on our custom 83xx boards.

Can you please try at least on one (or more) of the standard eval
boards 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
Conquest is easy. Control is not.
	-- Kirk, "Mirror, Mirror", stardate unknown

[U-Boot] [PATCH 1/4] mpc83xx: Make start.S true PIC

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/10 00:25:59:
>
> Dear Joakim Tjernlund,
>
> In message <OF7F8BAE70.CE1486C7-ONC1257813.00727B82-C1257813.0072BE1B@transmode.se> you wrote:
> >
> > > But your new code does not reference CONFIG_SYS_MONITOR_BASE at all,
> > > but uses CONFIG_SYS_FLASH_BASE instead.
> >
> > You can't assume a fixed address when doing PIC therefore the change.
>
> I understand this change is intentionally, then?

Yes it is needed, otherwise is isn't PIC as you assume a fixed start
address. The FLASH base is the same in both cases.

>
> > > On which boards has this been tested?
> >
> > Only on our custom 83xx boards.
>
> Can you please try at least on one (or more) of the standard eval
> boards as well?

Don't have one. We borrowed a board during early development but this board
has been returned long ago.

 Jocke

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Scott Wood]

On Sun, 9 Jan 2011 21:48:47 +0100
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:

> Wolfgang Denk <wd@denx.de> wrote on 2011/01/09 21:29:04:
> >
> > Dear Joakim Tjernlund,
> >
> > In message <1292838435-14958-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > > Only these 2 call sites depends on fixups for my mpc8321 based
> > > board.
> > >
> > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > > ---
> > >  arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
> > >  arch/powerpc/lib/board.c            |    2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > index 7a1cae7..88d9dd8 100644
> > > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > @@ -507,7 +507,7 @@ int prt_83xx_rsr(void)
> > >     sep = " ";
> > >     for (i = 0; i < n; i++)
> > >        if (rsr & bits[i].mask) {
> > > -         printf("%s%s", sep, bits[i].desc);
> > > +         printf("%s%s", sep, LINK_OFF(bits[i].desc));
> > >           sep = ", ";
> > >        }
> >
> >
> > Is my understanding correct that these changes are sufficient only for
> > your board, and only for your current configuration?  And that your
> > code would break (resp. require more LINK_OFF fixups) if you would -
> > for example - decide to enable CONFIG_DISPLAY_AER_FULL in your board
> > configuration (cf. print_83xx_arb_event() above in the same source
> > file) ?
> 
> It would break only if link address != load address. That is, if you
> want to use my new CONFIG_SYS_TRUE_PIC feature and be able to load
> u-boot at any address regardless of link address you would
> have to add LINK_OFF calls into print_83xx_arb_event() too if
> you want to use it.

Doesn't this add a requirement for future generic pre-relocation code to
comply with, to avoid breaking your board?

-Scott

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2011/01/10 19:24:02:
>
> On Sun, 9 Jan 2011 21:48:47 +0100
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> > Wolfgang Denk <wd@denx.de> wrote on 2011/01/09 21:29:04:
> > >
> > > Dear Joakim Tjernlund,
> > >
> > > In message <1292838435-14958-4-git-send-email-Joakim.Tjernlund@transmode.se> you wrote:
> > > > Only these 2 call sites depends on fixups for my mpc8321 based
> > > > board.
> > > >
> > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > > > ---
> > > >  arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
> > > >  arch/powerpc/lib/board.c            |    2 +-
> > > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > > index 7a1cae7..88d9dd8 100644
> > > > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c
> > > > @@ -507,7 +507,7 @@ int prt_83xx_rsr(void)
> > > >     sep = " ";
> > > >     for (i = 0; i < n; i++)
> > > >        if (rsr & bits[i].mask) {
> > > > -         printf("%s%s", sep, bits[i].desc);
> > > > +         printf("%s%s", sep, LINK_OFF(bits[i].desc));
> > > >           sep = ", ";
> > > >        }
> > >
> > >
> > > Is my understanding correct that these changes are sufficient only for
> > > your board, and only for your current configuration?  And that your
> > > code would break (resp. require more LINK_OFF fixups) if you would -
> > > for example - decide to enable CONFIG_DISPLAY_AER_FULL in your board
> > > configuration (cf. print_83xx_arb_event() above in the same source
> > > file) ?
> >
> > It would break only if link address != load address. That is, if you
> > want to use my new CONFIG_SYS_TRUE_PIC feature and be able to load
> > u-boot at any address regardless of link address you would
> > have to add LINK_OFF calls into print_83xx_arb_event() too if
> > you want to use it.
>
> Doesn't this add a requirement for future generic pre-relocation code to
> comply with, to avoid breaking your board?

Yes, but I don't mind if my board breaks from time to time. After all it isn't
in u-boot so I have had to deal with quite a few breakages already.
It is my hope this new feature will spread to other boards as time
pass.

 Jocke

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF6A573B77.9119DEB0-ONC1257814.0069A574-C1257814.007532C7@transmode.se> you wrote:
>
> > > It would break only if link address != load address. That is, if you
> > > want to use my new CONFIG_SYS_TRUE_PIC feature and be able to load
> > > u-boot at any address regardless of link address you would
> > > have to add LINK_OFF calls into print_83xx_arb_event() too if
> > > you want to use it.
> >
> > Doesn't this add a requirement for future generic pre-relocation code to
> > comply with, to avoid breaking your board?
> 
> Yes, but I don't mind if my board breaks from time to time. After all it isn't
> in u-boot so I have had to deal with quite a few breakages already.
> It is my hope this new feature will spread to other boards as time
> pass.

You mean you ask to add code that is not only highly fragile but even
known to be broken for other boards than yours, and the only board
that uses the feature and has been tested with it "isn't in u-boot" ?

Sorry, but it makes no sense to me to add all this.  NAK.

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 am pleased to see that we have differences.  May we together become
greater than the sum of both of us.
	-- Surak of Vulcan, "The Savage Curtain", stardate 5906.4

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/17 23:11:59:
>
> Dear Joakim Tjernlund,
>
> In message <OF6A573B77.9119DEB0-ONC1257814.0069A574-C1257814.007532C7@transmode.se> you wrote:
> >
> > > > It would break only if link address != load address. That is, if you
> > > > want to use my new CONFIG_SYS_TRUE_PIC feature and be able to load
> > > > u-boot at any address regardless of link address you would
> > > > have to add LINK_OFF calls into print_83xx_arb_event() too if
> > > > you want to use it.
> > >
> > > Doesn't this add a requirement for future generic pre-relocation code to
> > > comply with, to avoid breaking your board?
> >
> > Yes, but I don't mind if my board breaks from time to time. After all it isn't
> > in u-boot so I have had to deal with quite a few breakages already.
> > It is my hope this new feature will spread to other boards as time
> > pass.
>
> You mean you ask to add code that is not only highly fragile but even
> known to be broken for other boards than yours, and the only board
> that uses the feature and has been tested with it "isn't in u-boot" ?

No other board is broken. This new function is neutral to other boards.
I am merely saying as my board is the first user of this new feature I
expect minor breakage of my board from time to time when someone adds
a new function that needs LINK_OFF to work on my board but forgets
to actually add the LINK_OFF call. Once more boards uses my new
feature this problem goes away.

Wolfgang, once you indicated you were interested in such feature as I have
added but my first impl. had LINK_OFF calls all over the place, still you were
tempted to add the feature. Now that I have reduced the LINK_OFF calls to
a minimum you suddenly want to reject it even though >95% of the LINK_OFF calls
are gone. Why this change of heart?

  Jocke

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF71B40F2E.5C935BA8-ONC125781B.007CB9E6-C125781B.007E5726@transmode.se> you wrote:
>
> No other board is broken. This new function is neutral to other boards.

Well, I see this differntly.

> Wolfgang, once you indicated you were interested in such feature as I have
> added but my first impl. had LINK_OFF calls all over the place, still you were
> tempted to add the feature. Now that I have reduced the LINK_OFF calls to
> a minimum you suddenly want to reject it even though >95% of the LINK_OFF calls
> are gone. Why this change of heart?

Yes, I am definitely interested in this feature.

But I still consider the impact to make this really working to
heavy (and by working I mean working for all boards, out of the box,
without having to go through iterations of breakage to find out where
else a LINK_OFF needs to be added).

In the current form, your "simplification" results just from the fact
that you just added the bare minimum needed for your board. If - as
you hope - more and more boards would use this, more and more of these
now omitted LINK_OFFs would have to be added again.  The result would
be exactly the same mess as your original patch - it doesn't small
any better when you try to feed it to me in small bites.

And frankly, adding this stuff for an out-of-tree port is, um...,
sorry, but I don't find polite words atm.

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
An Elephant is a mouse with an Operating System.              - Knuth

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/18 00:42:10:
>
> Dear Joakim Tjernlund,
>
> In message <OF71B40F2E.5C935BA8-ONC125781B.007CB9E6-C125781B.007E5726@transmode.se> you wrote:
> >
> > No other board is broken. This new function is neutral to other boards.
>
> Well, I see this differntly.
>
> > Wolfgang, once you indicated you were interested in such feature as I have
> > added but my first impl. had LINK_OFF calls all over the place, still you were
> > tempted to add the feature. Now that I have reduced the LINK_OFF calls to
> > a minimum you suddenly want to reject it even though >95% of the LINK_OFF calls
> > are gone. Why this change of heart?
>
> Yes, I am definitely interested in this feature.

Good.

>
> But I still consider the impact to make this really working to
> heavy (and by working I mean working for all boards, out of the box,
> without having to go through iterations of breakage to find out where
> else a LINK_OFF needs to be added).

Yes, but the target area is fairly small. Only code the runs before
relocation which isn't that much.
How do you think a solution for all boards would look like?
The only other method is can think of is some MMU trickery and
I don't even see how you can make that work on all boards and
you would probably be locked to specific address ranges if
you use BATs as defined on PowerPC

>
> In the current form, your "simplification" results just from the fact
> that you just added the bare minimum needed for your board. If - as
> you hope - more and more boards would use this, more and more of these
> now omitted LINK_OFFs would have to be added again.  The result would
> be exactly the same mess as your original patch - it doesn't small
> any better when you try to feed it to me in small bites.

My first patch was pretty much only my board too and the result for all
boards would be MUCH smaller than the first patch approach.
I did only did one board because that was what I could test and I
wanted feedback from the list. Did you expect me to blindly
port this to every board?
The ground work for 83xx is done, a few more LINK_OFF calls is probably
needed for all 83xx boards, I can take a stab at the remaining
83xx boards once you embrace this method. The rest will have to
be up to the other board maintainers, if they want this feature.

>
> And frankly, adding this stuff for an out-of-tree port is, um...,
> sorry, but I don't find polite words atm.

Yet you are interested in such a feature. Sorry but I too
have a hard time finding polite words.

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Scott Wood]

On Tue, 18 Jan 2011 01:18:34 +0100
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:

> How do you think a solution for all boards would look like?
> The only other method is can think of is some MMU trickery and
> I don't even see how you can make that work on all boards and

You don't need to make the MMU trick work on all boards, just your board
(or cpu family), because it wouldn't be imposing anything on
the rest of the system.  Do we actually have someone who
needs this feature on a board without a suitable MMU, large lockable
cache or other SRAM, hardware bank switching, or other mechanism
that can be confined to early low-level board/cpu-specific code?

> you would probably be locked to specific address ranges if
> you use BATs as defined on PowerPC

You'd have alignment constraints, but it's good enough for bank
switching.

-Scott

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2011/01/18 18:27:49:
> On Tue, 18 Jan 2011 01:18:34 +0100
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> > How do you think a solution for all boards would look like?
> > The only other method is can think of is some MMU trickery and
> > I don't even see how you can make that work on all boards and
>
> You don't need to make the MMU trick work on all boards, just your board
> (or cpu family), because it wouldn't be imposing anything on
> the rest of the system.  Do we actually have someone who
> needs this feature on a board without a suitable MMU, large lockable
> cache or other SRAM, hardware bank switching, or other mechanism
> that can be confined to early low-level board/cpu-specific code?

Wolfgang seems to think so. As I read his reply he wants a solution for all
boards which I don't think is possible. I do think my approach comes closest though.
I did try BATs but I didn't get very far.

>
> > you would probably be locked to specific address ranges if
> > you use BATs as defined on PowerPC
>
> You'd have alignment constraints, but it's good enough for bank
> switching.

Alignment and a suitable bank size so you don't fall into your
environment. That may make BATs hard to use in general.

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OF287114D8.013B6E51-ONC125781C.00667DCD-C125781C.00673260@transmode.se> you wrote:
>
> > You don't need to make the MMU trick work on all boards, just your board
> > (or cpu family), because it wouldn't be imposing anything on
> > the rest of the system.  Do we actually have someone who
> > needs this feature on a board without a suitable MMU, large lockable
> > cache or other SRAM, hardware bank switching, or other mechanism
> > that can be confined to early low-level board/cpu-specific code?
> 
> Wolfgang seems to think so. As I read his reply he wants a solution for all

Please be aware that I don't think anything at all.  I just comment :-)

I'm not in your position, where I am focussing on "how can I implement
this".  I'm looking at the code from the outside, and I ask what does
it give to me, what of the things I'd like to have does it not give to
me, and at which price does it come.

> boards which I don't think is possible. I do think my approach comes closest though.
> I did try BATs but I didn't get very far.

If we are talking about _all_ boards we have to keep a wider view.
For example, on MPC8xx there are not BATs. Not to mention other
architectures.


Actually I was not asking for support for all boards, not even for
all boards of a specific architecture, or a specific CPU.  You
submitted this patch, and I learned that the code, as is, is only
useful for a single board, which appears to be maintained in an
out-of-tree port.  For all in-tree boards the code, as is, is broken
and unusable without further rework.  Needless to repeat that it is
completely untested for mainline.

So we have a patch that promises a feature, which cannot be used by
any of the mainline boards, but it makes the code more complex for
zero benefit.

It's a nice and appreciated RFC patch or even example implementation,
but I fail to see arguments why we should add this to mainline.

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
In the future, you're going to get computers as prizes  in  breakfast
cereals.  You'll  throw  them out because your house will be littered
with them.                                             - Robert Lucky

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Scott Wood]

On Tue, 18 Jan 2011 21:06:34 +0100
Wolfgang Denk <wd@denx.de> wrote:

> Dear Joakim Tjernlund,
> 
> In message <OF287114D8.013B6E51-ONC125781C.00667DCD-C125781C.00673260@transmode.se> you wrote:
> > boards which I don't think is possible. I do think my approach comes closest though.
> > I did try BATs but I didn't get very far.

What problems did you run into?

> 
> If we are talking about _all_ boards we have to keep a wider view.
> For example, on MPC8xx there are not BATs. Not to mention other
> architectures.

I don't see why MPC8xx's MMU couldn't be used for this, although it
might be a tight fit if you want to get everything into the pinned TLB
entries.

Should we be talking about all boards, if the only user of this so far
is on hardware that can do it in a less-intrusive way?

Another thing to keep in mind is that Joakim's approach also won't work
on all boards -- you need hardware that is capable of selecting from
different entry points, where the entry address itself changes rather
than flash banks being flipped around.

-Scott

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Scott Wood <scottwood@freescale.com> wrote on 2011/01/18 21:24:16:
> On Tue, 18 Jan 2011 21:06:34 +0100
> Wolfgang Denk <wd@denx.de> wrote:
>
> > Dear Joakim Tjernlund,
> >
> > In message <OF287114D8.013B6E51-ONC125781C.00667DCD-C125781C.00673260@transmode.se> you wrote:
> > > boards which I don't think is possible. I do think my approach comes closest though.
> > > I did try BATs but I didn't get very far.
>
> What problems did you run into?

It crashed and burned :) I never got to the details.

>
> >
> > If we are talking about _all_ boards we have to keep a wider view.
> > For example, on MPC8xx there are not BATs. Not to mention other
> > architectures.
>
> I don't see why MPC8xx's MMU couldn't be used for this, although it
> might be a tight fit if you want to get everything into the pinned TLB
> entries.

I think it is harder that that. You probably need a tlb miss handler.

>
> Should we be talking about all boards, if the only user of this so far
> is on hardware that can do it in a less-intrusive way?
>
> Another thing to keep in mind is that Joakim's approach also won't work
> on all boards -- you need hardware that is capable of selecting from
> different entry points, where the entry address itself changes rather
> than flash banks being flipped around.

No, you only need gcc support for msingle-pic-base and some extra RAM
to hold the GOT(about 8KB would do I think) while still in flash.
Ah, you mean how to select which image to select? That is a small
piece of SW that checks both images and chooses the valid one and jumps
there.

 Jocke

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/18 21:06:34:
> Dear Joakim Tjernlund,
>
> In message <OF287114D8.013B6E51-ONC125781C.00667DCD-C125781C.00673260@transmode.se> you wrote:
> >
> > > You don't need to make the MMU trick work on all boards, just your board
> > > (or cpu family), because it wouldn't be imposing anything on
> > > the rest of the system.  Do we actually have someone who
> > > needs this feature on a board without a suitable MMU, large lockable
> > > cache or other SRAM, hardware bank switching, or other mechanism
> > > that can be confined to early low-level board/cpu-specific code?
> >
> > Wolfgang seems to think so. As I read his reply he wants a solution for all
>
> Please be aware that I don't think anything at all.  I just comment :-)
>
> I'm not in your position, where I am focussing on "how can I implement
> this".  I'm looking at the code from the outside, and I ask what does
> it give to me, what of the things I'd like to have does it not give to
> me, and at which price does it come.
>
> > boards which I don't think is possible. I do think my approach comes closest though.
> > I did try BATs but I didn't get very far.
>
> If we are talking about _all_ boards we have to keep a wider view.
> For example, on MPC8xx there are not BATs. Not to mention other
> architectures.
>
>
> Actually I was not asking for support for all boards, not even for
> all boards of a specific architecture, or a specific CPU.  You
> submitted this patch, and I learned that the code, as is, is only
> useful for a single board, which appears to be maintained in an
> out-of-tree port.  For all in-tree boards the code, as is, is broken
> and unusable without further rework.  Needless to repeat that it is
> completely untested for mainline.

Ah, finally you make sense to me. I actually tested this with mainline
on my board so it is not completely untested in mainline.

>
> So we have a patch that promises a feature, which cannot be used by
> any of the mainline boards, but it makes the code more complex for
> zero benefit.

You can test that nothing breaks in other 83xx boards. That would be
valuable feedback to me.
I can try to complete all NOR based 83xx boards too, but someone needs
to test them.

>
> It's a nice and appreciated RFC patch or even example implementation,
> but I fail to see arguments why we should add this to mainline.

Well, you have to start somewhere and as this involves asm changes
in start.S it would be pretty dangerous add these without being able to
test. The idea is that once some version of this patch is in, interested
parties can apply the same concept on their boards too.

Finally, I would like to remind you about
 [PATCH] PowerPC: Move -fPIC flag to common place
 [PATCH] PowerPC: Add support for -msingle-pic-base
Those two stands on its own.

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Scott Wood,

In message <20110118142416.5892ca00@udp111988uds.am.freescale.net> you wrote:
>
> > If we are talking about _all_ boards we have to keep a wider view.
> > For example, on MPC8xx there are not BATs. Not to mention other
> > architectures.
> 
> I don't see why MPC8xx's MMU couldn't be used for this, although it
> might be a tight fit if you want to get everything into the pinned TLB
> entries.

I just wanted to point out that the "use BATs then" approach is also
not a solution that covers all systems.

> Should we be talking about all boards, if the only user of this so far
> is on hardware that can do it in a less-intrusive way?

In the first round of discussion the buzz word was to acchieve a fully
position independent U-Boot image.  This would obviously have a number
of nice use cases, including the often asked for "load test version to
free area in RAM and jump to it".

This is somehting that would indeed be useful for all boards, on all
architectures, all SoCs.  So it makes sense to me to include into the
reviewing  if a proposed solution is capable of being generalized to
other boards / systems or not.

> Another thing to keep in mind is that Joakim's approach also won't work
> on all boards -- you need hardware that is capable of selecting from
> different entry points, where the entry address itself changes rather
> than flash banks being flipped around.

There could be other mechanisms in place to determine which U-Boot
image should be started, like a tiny piece of software that jumps to
the selected image.

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
Unsichtbar macht sich die Dummheit, indem sie immer  gr??ere  Ausma?e
annimmt.                             -- Bertold Brecht: Der Tui-Roman

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFBBE91305.B19350D9-ONC125781C.007192FC-C125781C.007421BB@transmode.se> you wrote:
>
> Ah, finally you make sense to me. I actually tested this with mainline
> on my board so it is not completely untested in mainline.

As your board itself is not in mainline this _is_ completely untested
for mainline.

> > It's a nice and appreciated RFC patch or even example implementation,
> > but I fail to see arguments why we should add this to mainline.
> 
> Well, you have to start somewhere and as this involves asm changes
> in start.S it would be pretty dangerous add these without being able to
> test. The idea is that once some version of this patch is in, interested
> parties can apply the same concept on their boards too.

Such testing can be done anywhere, in some test branch. I don't think
mainline is the right place for such an intrusive and experiemental
feature.

> Finally, I would like to remind you about
>  [PATCH] PowerPC: Move -fPIC flag to common place

I have not seen any test reports on this, so I hesitate to apply it
(the move to a common place is no problem, of course, but I'm not
sure about changing -fPIC into -fpic). I think I remember problems
with this in the past; there have even been commits to "use '-fPIC'
_instead_ of '-mrelocatable'" in the past. I don't remember the
details, but I'd like to see some independent testing before this
goes in.

>  [PATCH] PowerPC: Add support for -msingle-pic-base

Same here. This hits a large number of boards, but I have seen zero
test reports.

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
Do you suppose the reason the ends of the `Intel Inside'  logo  don't
match up is that it was drawn on a Pentium?

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Scott Wood]

On Tue, 18 Jan 2011 21:39:23 +0100
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:

> Scott Wood <scottwood@freescale.com> wrote on 2011/01/18 21:24:16:
> > I don't see why MPC8xx's MMU couldn't be used for this, although it
> > might be a tight fit if you want to get everything into the pinned TLB
> > entries.
> 
> I think it is harder that that. You probably need a tlb miss handler.

Well, as I said, it might be a tight fit.  If you can't fit everything
within the pinned entries, then you'll need a TLB miss handler, but it
could be a simple handler that will insert a non-cacheable
identity-mapped entry for whatever address faulted (cacheable regions
are covered by pinned entries and won't fault in the first place).  Or
possibly have an above/below address threshold for determining
cacheability.

After relocation, you can turn off the MMU, so it's only things you
access before relocation that need to fit in the TLB.

> > Should we be talking about all boards, if the only user of this so far
> > is on hardware that can do it in a less-intrusive way?
> >
> > Another thing to keep in mind is that Joakim's approach also won't work
> > on all boards -- you need hardware that is capable of selecting from
> > different entry points, where the entry address itself changes rather
> > than flash banks being flipped around.
> 
> No, you only need gcc support for msingle-pic-base and some extra RAM
> to hold the GOT(about 8KB would do I think) while still in flash.
> Ah, you mean how to select which image to select? That is a small
> piece of SW that checks both images and chooses the valid one and jumps
> there.

OK, I thought you were using something like a DIP switch to toggle
low/high exception vectors, and getting different entry points that way.

-Scott

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Joakim Tjernlund]

Wolfgang Denk <wd@denx.de> wrote on 2011/01/18 22:25:04:
>
> Dear Joakim Tjernlund,
>
> In message <OFBBE91305.B19350D9-ONC125781C.007192FC-C125781C.007421BB@transmode.se> you wrote:
> >
> > Ah, finally you make sense to me. I actually tested this with mainline
> > on my board so it is not completely untested in mainline.
>
> As your board itself is not in mainline this _is_ completely untested
> for mainline.
>
> > > It's a nice and appreciated RFC patch or even example implementation,
> > > but I fail to see arguments why we should add this to mainline.
> >
> > Well, you have to start somewhere and as this involves asm changes
> > in start.S it would be pretty dangerous add these without being able to
> > test. The idea is that once some version of this patch is in, interested
> > parties can apply the same concept on their boards too.
>
> Such testing can be done anywhere, in some test branch. I don't think
> mainline is the right place for such an intrusive and experiemental
> feature.

Do you have such a branch where you can apply this?
>
> > Finally, I would like to remind you about
> >  [PATCH] PowerPC: Move -fPIC flag to common place
>
> I have not seen any test reports on this, so I hesitate to apply it
> (the move to a common place is no problem, of course, but I'm not
> sure about changing -fPIC into -fpic). I think I remember problems
> with this in the past; there have even been commits to "use '-fPIC'
> _instead_ of '-mrelocatable'" in the past. I don't remember the
> details, but I'd like to see some independent testing before this
> goes in.

Right, this can be confusing. Without -mrelocatable you don't
get fixups so this has to stay as fixups is a must nowadays.

Assuming you don't need fixups, -fpic needs what went into the linker
scripts a little while ago so -fpic had little chance before that.

>
> >  [PATCH] PowerPC: Add support for -msingle-pic-base
>
> Same here. This hits a large number of boards, but I have seen zero
> test reports.

Scott and Kim, can you give these 2 patches a spin?

[U-Boot] [PATCH 4/4] powerpc: Add LINK_OFF calls in early C-code.

[Wolfgang Denk]

Dear Joakim Tjernlund,

In message <OFA95D19C0.58C4CBC0-ONC125781C.00785FED-C125781C.0079E902@transmode.se> you wrote:
> 
> > Such testing can be done anywhere, in some test branch. I don't think
> > mainline is the right place for such an intrusive and experiemental
> > feature.
> 
> Do you have such a branch where you can apply this?

But I'm have no plans to test it.  I don't have time nor resources 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
"Get back to your stations!"
"We're beaming down to the planet, sir."
	-- Kirk and Mr. Leslie, "This Side of Paradise",
	   stardate 3417.3