Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
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 Kiskytroy.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 = .;
+#includeu-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?
Hmmmmaybe 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?
I hope you have a better idea for how to handle patch 2/2.
Thanks for the feedback.
Troy
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
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 Kiskytroy.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 = .;
+#includeu-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?
Hmmmmaybe 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?
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
Hmm, and this gets me to thinking that this process is going to be very
useful for my INIT_CALL series - At the moment, I collect all the
INIT_CALL declerations in a section, but I need a dedicated linker script
to create a file which only includes that section - With a slight tweak to
U_BOOT_LDS_SECTION, I should be able to selectively switch between the two
link modes (INIT_CALL versus 'final' link)
Regards,
Graeme
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
Hi Troy, On Wed, Feb 8, 2012 at 12:21 PM, Graeme Russ graeme.r...@gmail.com wrote: Hi Tony, Oops, sorry - forgot to proof read Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
Hi Troy, On Wed, Feb 8, 2012 at 12:21 PM, Graeme Russ graeme.r...@gmail.com wrote: Add U_BOOT_LDS_SECTION(phy_entry, 4) to all the linker scripts Oh, and it should have an #ifdef around it - I know it's probably harmless, but I dislike having 'fluff' floating around :) Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
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.comwrote:
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 Kiskytroy.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 = .;
+#includeu-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?
Hmmmmaybe 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
#includeu-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
#includeu-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
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
Hi Troy, On Wed, Feb 8, 2012 at 12:56 PM, Troy Kisky troy.ki...@boundarydevices.com wrote: 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: 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. You are correct, it will not add anything, but we tend to have an aversion to dead code :) 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 #includeu-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 #includeu-boot-common.lds, that still means far fewer files to change and less risk of typos. That looks like a good compromise to me As I mentioned, the U_BOOT_LDS_SECTION macro will be very useful for me later on The next biggie is where to define all the externs exported from the linker script as a result of using the U_BOOT_LDS_SECTION macro. I'm half tempted to think we could collect all the usages of U_BOOT_LDS_SECTION in a header (for the common case, that is essentially u-boot-common.lds) and by setting a #define you switch between full macro expansion (for the linker) and 'extern generation' for including in C files. That way, when you add a new section, everything happen automagically :) Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
On 2/7/2012 7:24 PM, Graeme Russ wrote: Hi Troy, On Wed, Feb 8, 2012 at 12:56 PM, Troy Kisky troy.ki...@boundarydevices.com wrote: 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.comwrote: 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. You are correct, it will not add anything, but we tend to have an aversion to dead code :) 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 #includeu-boot-comon.lds.h at the start of each lds file, and #includeu-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 #includeu-boot-common.lds, that still means far fewer files to change and less risk of typos. That looks like a good compromise to me As I mentioned, the U_BOOT_LDS_SECTION macro will be very useful for me later on The next biggie is where to define all the externs exported from the linker script as a result of using the U_BOOT_LDS_SECTION macro. I'm half tempted to think we could collect all the usages of U_BOOT_LDS_SECTION in a header (for the common case, that is essentially u-boot-common.lds) and by setting a #define you switch between full macro expansion (for the linker) and 'extern generation' for including in C files. That way, when you add a new section, everything happen automagically :) Regards, Graeme So do you think this is wrong in patch 2/2? diff --git a/include/phy.h b/include/phy.h index bc522d5..f0eb502 100644 --- a/include/phy.h +++ b/include/phy.h @@ -23,6 +23,7 @@ #ifndef _PHY_H #define _PHY_H +#include linux/compiler.h #include linux/list.h #include linux/mii.h #include linux/ethtool.h @@ -231,4 +232,7 @@ int phy_vitesse_init(void); /* PHY UIDs for various PHYs that are referenced in external code */ #define PHY_UID_TN2020 0x00a19410 +#define __phy_entry __attribute__((section(.phy_entry))) __used __aligned(4) +extern struct phy_driver __phy_entry_start, __phy_entry_end; + #endif I don't see how this can be automatically generated. Troy ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds
Hi Troy, On Wed, Feb 8, 2012 at 1:46 PM, Troy Kisky troy.ki...@boundarydevices.com wrote: On 2/7/2012 7:24 PM, Graeme Russ wrote: The next biggie is where to define all the externs exported from the linker script as a result of using the U_BOOT_LDS_SECTION macro. I'm half tempted to think we could collect all the usages of U_BOOT_LDS_SECTION in a header (for the common case, that is essentially u-boot-common.lds) and by setting a #define you switch between full macro expansion (for the linker) and 'extern generation' for including in C files. That way, when you add a new section, everything happen automagically :) Regards, Graeme So do you think this is wrong in patch 2/2? diff --git a/include/phy.h b/include/phy.h index bc522d5..f0eb502 100644 --- a/include/phy.h +++ b/include/phy.h @@ -23,6 +23,7 @@ #ifndef _PHY_H #define _PHY_H +#include linux/compiler.h #include linux/list.h #include linux/mii.h #include linux/ethtool.h @@ -231,4 +232,7 @@ int phy_vitesse_init(void); /* PHY UIDs for various PHYs that are referenced in external code */ #define PHY_UID_TN2020 0x00a19410 +#define __phy_entry __attribute__((section(.phy_entry))) __used __aligned(4) +extern struct phy_driver __phy_entry_start, __phy_entry_end; + Aha! Thanks - putting the extern there makes much more sense :) #endif I don't see how this can be automatically generated. Yeah, I wouldn't bother either now I see above Regards, Graeme ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
