[U-Boot] [PATCH 1/4] ARM: fix u-boot.lds for -ffunction-sections/-fdata-sections

[Albert ARIBAUD]

Hi Stephen,

On Thu, 18 Oct 2012 15:17:52 -0600, Stephen Warren
<swarren at wwwdotorg.org> wrote:

> On 10/18/2012 02:58 PM, Stephen Warren wrote:
> > On 10/18/2012 02:36 PM, Albert ARIBAUD wrote:
> >> Hi Stephen,
> >>
> >> On Wed, 17 Oct 2012 21:17:45 -0600, Stephen Warren
> >> <swarren at wwwdotorg.org> wrote:
> >>
> >>> On 10/17/2012 05:58 PM, Simon Glass wrote:
> >>>> Hi Stephen,
> >>>>
> >>>> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> >>>>> From: Stephen Warren <swarren at nvidia.com>
> >>>>>
> >>>>> When -ffunction-sections or -fdata-section are used, symbols are placed
> >>>>> into sections such as .data.eserial1_device and .bss.serial_current.
> >>>>> Update the linker script to explicitly include these. Without this
> >>>>> change (at least with my gcc-4.5.3 built using crosstool-ng), I see that
> >>>>> the sections do end up being included, but __bss_end__ gets set to the
> >>>>> same value as __bss_start.
> >>>>>
> >>>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> >>>>> ---
> >>>>> This series fixes an SPL size overflow problem on Tegra. Tom Warren is
> >>>>> out on vacation until Oct 25th, so he certainly won't be able to review
> >>>>> this. Perhaps it could be applied directly to the ARM tree if enough
> >>>>> Tegra people ack the series?
> >>>>>
> >>>>> Note that this series is not enough to make Tegra support work; either
> >>>>> you must hack ./arch/arm/cpu/arm720t/tegra-common/spl.c to call
> >>>>> serial_initialize() right before serial_init() in preloader_console_init()
> >>>>> or wait for Allen Martin to rework Tegra's SPL support using the common
> >>>>> SPL code.
> >>>>
> >>>> Are you going to submit a patch to enable function-sections, or is
> >>>> that a separate discussion?
> >>>
> >>> For the SPL on Tegra, those flags were already on; this patch fixes a
> >>> bug rather than prepares for new functionality.
> >>>
> >>>>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> >>>
> >>>>> -       .u_boot_cmd : { *(.u_boot_cmd) }
> >>>>> +       .u_boot_cmd : { *(.u_boot_cmd*) }
> >>>>
> >>>> I don't think this line is needed?
> >>>>
> >>> ...
> >>>>> -               *(.dynsym)
> >>>>> +               *(.dynsym*)
> >>>>
> >>>> Nor this one?
> >>>
> >>> Possibly. I changed all the section names to be future-proof. Perhaps a
> >>> more targeted patch is warranted.
> >>
> >> Has this been (at least build-)tested on all boards which have
> >> -ffunction-sections or -fdata-sections?
> > 
> > Yes; those flags both appear to be turned on when building the SPL for
> > Tegra (although strangely, not when building the main U-Boot):
> > 
> > xxx-gcc -M -g  -Os   -fno-common -ffixed-r8 -msoft-float  -D__KERNEL__
> > -ffunction-sections -fdata-sections -DCONFIG_SYS_TEXT_BASE=0x0010c000
> > -DCONFIG_SPL_TEXT_BASE=0x00108000 -DCONFIG_SPL_BUILD
> > -I/home/swarren/shared/git_wa/u-boot/include -fno-builtin -ffreestanding
> > -nostdinc -isystem xxx -pipe -DCONFIG_ARM -D__ARM__ -marm
> > -mno-thumb-interwork -mabi=aapcs-linux -march=armv4 -mtune=arm7tdmi -MQ
> > xxx/spl/arch/arm/cpu/arm720t/interrupts.o interrupts.c >
> > xxx/spl/arch/arm/cpu/arm720t/.depend.interrupts
> 
> I tried to test this more widely. I found that only
> arch/arm/cpu/ixp/config.mk enables this on ARM, with the comment:
> 
> > # -fdata-sections triggers "section .bss overlaps section .rel.dyn" linker error
> > PLATFORM_RELFLAGS += -ffunction-sections
> > LDFLAGS_u-boot += --gc-sections
> 
> Indeed, if I edit arch/arm/cpu/armv7/config.mk to enable both
> -ffunction-sections -fdata-sections, I do see that same section overlap
> error building for Tegra.
> 
> Then, if I apply the .lds patch we're discussing, Tegra builds (and
> runs) OK with those linker flags enabled. So, this patch seems to fix
> two issues not just one - bonus:-)

Unless someone opposes, I will pick this one in u-boot-arm/master later
today, as I will be doing some .lds merging on ARM (and then some
start.S merging). Once this is done, the Tegra tree (Tom already Cc:)
should merge u-boot-arm/master to get this first commit in, then apply
commits 2 to 5.

Amicalement,
-- 
Albert.