On 2/7/2012 6:21 PM, Graeme Russ wrote:
Hi Tony,

On Wed, Feb 8, 2012 at 11:49 AM, Troy Kisky
<troy.ki...@boundarydevices.com>  wrote:
On 2/7/2012 5:39 PM, Graeme Russ wrote:
Hi Troy,

On Wed, Feb 8, 2012 at 11:20 AM, Troy Kisky
<troy.ki...@boundarydevices.com>    wrote:
This makes adding linker defined tables easier
as only 1 lds file (include/u-boot-common.lds)
will need to be touched instead of 193 files.

Signed-off-by: Troy Kisky<troy.ki...@boundarydevices.com>
---

diff --git a/arch/arm/cpu/arm1136/u-boot.lds
b/arch/arm/cpu/arm1136/u-boot.lds
index d1e2851..fe07097 100644
--- a/arch/arm/cpu/arm1136/u-boot.lds
+++ b/arch/arm/cpu/arm1136/u-boot.lds
@@ -52,12 +52,7 @@ SECTIONS
                *(.data)
        }

-       . = ALIGN(4);
-
-       . = .;
-       __u_boot_cmd_start = .;
-       .u_boot_cmd : { *(.u_boot_cmd) }
-       __u_boot_cmd_end = .;
+#include<u-boot-common.lds>

        . = ALIGN(4);

Ick! A #define in the middle of a file :(

I would rather see the code block you are replacing #defined in
include/u-boot-common.lds.h and then use the define at the desired
location in the arch lds file. This will allow multiple 'functional
linker blocks' to be defined that the acrch scripts can arrange as they
see fit.

Also, ALIGN(4) may not always be a given, so maybe we should pass that in
as a parameter?

Hmmm....maybe something like (most likely not syntactically correct):

#define U_BOOT_LDS_SECTION(section, alignment) \
        . = ALIGN(alignment); \
        U_BOOT_LDS_SYMBOL(section ## _start) = .;
        . ## section : { KEEP(*(. ## section)) }
        U_BOOT_LDS_SYMBOL(section ## _end) = .;


So we can simply, in the arch lds:

        U_BOOT_LDS_SECTION(text, 4)
        U_BOOT_LDS_SECTION(u_boot_cmd, 4)
        U_BOOT_LDS_SECTION(rodata, 4)
        U_BOOT_LDS_SECTION(data, 4)
        U_BOOT_LDS_SECTION(dynsym, 4)

Regards,

Graeme

That works fine for existing sections.. i.e

U_BOOT_LDS_SECTION(u_boot_cmd, 4)


but what about the next patch in the series?
Do you want me to change all lds files again?
Ah, and therein lies the rub...

By adding 'phy_entry' into the common lds file, you have inflicted it on
everyone, and forced it to be after u_boot_cmd. What if I don't need the
phy_entry section (no network support) or don't want it hard-placed after
u_boot_cmd?

Inflicted? I doubt that I added any bytes at all to boards that don't use phylib.


I hope you have a better idea for how to handle patch 2/2.
Add  U_BOOT_LDS_SECTION(phy_entry, 4) to all the linker scripts

The entire reason for me for patch 1/2 was to support 2/2.

What about  #include <u-boot-comon.lds.h>

at the start of each lds file,

and

#include<u-boot-common.lds>


in the middle?



If people really want to control the relative ordering of u_boot_cmd, and 
phy_entry,
they are not forced to include u-boot-common.lds here.

I can even throw in an unnecessary ifdef in u-boot-common.lds

#ifdef CONFIG_PHYLIB
U_BOOT_LDS_SECTION(phy_entry, 4)
#endif


This will make it easier for developers to add a table without having
to change 192 lds files. If a few boards opt not to use
"#include<u-boot-common.lds>," that still means far fewer files
to change and less risk of typos.

Troy








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

Reply via email to