[U-Boot] [PATCH v4 05/13] Move memcpy(), memset() into new lib/membasic.c

[Simon Glass]

Hi Mike,

On Tue, Mar 6, 2012 at 8:20 AM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Tuesday 06 March 2012 01:34:24 Simon Glass wrote:
>> On Mon, Mar 5, 2012 at 9:05 PM, Mike Frysinger wrote:
>> > On Monday 20 February 2012 20:32:47 Simon Glass wrote:
>> >> These basic functions are needed by relocation. To avoid bringing in all
>> >> string.c functions (and the resulting code bloat for architectures where
>> >> -ffunction-sections is not used), move these into their own file.
>> >
>> > seems like fixing -f{data,function}-sections would be a more useful goal.
>> >  i don't know why arm doesn't support this today ... it's fairly easy to
>> > do.
>>
>> I did enable it for Tegra at one point with no ill effects except for
>> breaking warmboot, which relied on function ordering. I notice the
>> no-toplevel-reorder flag on x86 but not with ARM. But I assumed there
>> must be some reason it is not used and thought this patch safer, since
>> it is just a minor tweak on a series that has has some review.
>
> if we keep assuming that, then it'll never get fixed, and we'll sit on a steady
> stream of "small safe patches".  plus, fixing this yields general improvements
> in anti-bloat for everyone.
>
> function ordering sounds like something that should be imposed at the linker
> level ... for Blackfin, all i had to care about was the entry point was at the
> start of the .text, although warmbooting is fairly low level magic, and i
> handle that in the SPL-like core.

While I agree with you, I am not sure that I can enable that for ARM
this late in the game.

If I enable -ffunction-sections for the tx25 spl build, I get:

$ make clean && make tx25_config && make -j20 -s
Configuring for tx25 board...
Generating include/generated/asm-offsets.h
string.o: In function `skip_spaces':
/home/sjg/trunk/src/third_party/u-boot/files/lib/string.c:229:
undefined reference to `_ctype'
string.o: In function `strim':
/home/sjg/trunk/src/third_party/u-boot/files/lib/string.c:255:
undefined reference to `_ctype'
string.o: In function `strdup':
/home/sjg/trunk/src/third_party/u-boot/files/lib/string.c:292:
undefined reference to `malloc'
make[1]: *** [/home/sjg/trunk/src/third_party/u-boot/files/nand_spl/u-boot-spl]
Error 1
make: *** [nand_spl] Error 2
make: *** Waiting for unfinished jobs....

The string.c library is using malloc() and ctype calls which are not
in the build. If I add them to the build (and divide and various other
things that aren't really needed) and enable -ffunction-sections I get
a complete build but the result is a bit over 8KB instead of the
original 2KB. The build does complete, though. Maybe just building is
good enough for now, and we can deal any breakage next time?

Now I'm not sure if the tx25 can in fact deal with this, but it seems
risky at this stage.

So in summary I wonder whether we should run with this simple split
patch for now, and deal with moving ARM to -ffunction-sections in the
next merge window, assuming Albert is keen on that idea? Alternatively
I can update the tx25 patch (and perhaps1-2 others) to remove the need
for this patch, with the possibility that these few boards will build
but night not actually work.

Hmmm.

Regards,
Simon

> -mike