[U-Boot] [PATCH v2 1/2] RFC: create u-boot-common.lds

[Troy Kisky]

On 2/7/2012 6:21 PM, Graeme Russ wrote:
> Hi Tony,
>
> On Wed, Feb 8, 2012 at 11:49 AM, Troy Kisky
> <troy.kisky at 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.kisky at 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.kisky at 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