[U-Boot] Weak symbols: request for comments

[U-Boot] Weak symbols: request for comments

[Sebastien Carlier]

Hello all,

I am looking for comments on the use of weak symbols in u-boot.

Some context: u-boot uses weak symbols in several places to provide
default definitions intended to be overriden in individual boards;
this feature is broken with recent toolchains (at least gcc 4.4.4,
binutils 2.20.1), and as a result only the default definitions are
used, while board-specific definitions are silently discarded.

The problem seems to arise because the weak definitions are seen by
the linker before the board-specific ones.  The linker will not look
in the board-specific library archive for strong symbols that would
override already defined weak symbols (this behavior is the one
specified by the System V gABI, so it is correct).

So, U-boot needs to be fixed.  I can see the following ways forward:

1.1) Stop using weak symbols; use pre-initialized function pointers
      instead (possibly grouped in a struct, for cleanliness).
      This has the benefit of offering a clear interface and being
      independent of toolchain details.

1.2) Use regular (non-weak) extern declarations for overridable stuff;
      collect all default weak symbols into a separate library archive,
      to be supplied last to the linker.

1.3) Stop using a library archive for the board specific stuff.
      Instead, collect and link all the object files to produce the
      output binary.  Only Makefile changes are involved, but correct
      behavior depends on all boards doing the right thing.

1.4) Link u-boot into a board-agnostic dynamic library, link the
      board-specific stuff into an executable embedding a dynamic
      linker, and package all this stuff somehow.

Are there better options?  Which one would you prefer to see
implemented?

For reference, here is the list of the definitions currently marked
weak in the u-boot code:

         _machine_restart
         arch_memory_failure_handle
         arch_memory_test_advance
         arch_memory_test_cleanup
         arch_memory_test_prepare
         board_hwconfig
         board_nand_init
         board_reset
         cpu_hwconfig
         do_bootelf_exec
         do_go_exec
         getDebugChar
         kgdb_flush_cache_all
         kgdb_flush_cache_range
         kgdb_interruptible
         kgdb_serial_init
         mg_get_drv_data
         putDebugChar
         putDebugStr
         read_fifo
         spi_cs_activate
         spi_cs_deactivate
         spi_cs_is_valid
         system_map

-- 
Sebastien Carlier

[U-Boot] Weak symbols: request for comments

[Andre Schwarz]

Sebastien,

[snip]
> So, U-boot needs to be fixed.  I can see the following ways forward:
>
> 1.1) Stop using weak symbols; use pre-initialized function pointers
>        instead (possibly grouped in a struct, for cleanliness).
>        This has the benefit of offering a clear interface and being
>        independent of toolchain details.
>    

yep - this is my favorite.
Main goal should always be to be as clear as possible.



Regards,
Andr?

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

[U-Boot] Weak symbols: request for comments

[Reinhard Meyer]

Dear Sebastien Carlier,
> Some context: u-boot uses weak symbols in several places to provide
> default definitions intended to be overriden in individual boards;
> this feature is broken with recent toolchains (at least gcc 4.4.4,
> binutils 2.20.1), and as a result only the default definitions are
> used, while board-specific definitions are silently discarded.
> 
> The problem seems to arise because the weak definitions are seen by
> the linker before the board-specific ones.  The linker will not look
> in the board-specific library archive for strong symbols that would
> override already defined weak symbols (this behavior is the one
> specified by the System V gABI, so it is correct).
> 
> So, U-boot needs to be fixed.  I can see the following ways forward:
> 
> 1.1) Stop using weak symbols; use pre-initialized function pointers
>       instead (possibly grouped in a struct, for cleanliness).
>       This has the benefit of offering a clear interface and being
>       independent of toolchain details.

Preferable 2nd. Don't use non standard stuff.

> 
> 1.2) Use regular (non-weak) extern declarations for overridable stuff;
>       collect all default weak symbols into a separate library archive,
>       to be supplied last to the linker.

Not very practical, that would require that each driver etc. would
be in two parts, the main part and the "weak" part. It would no need
weak functions, however.

> 
> 1.3) Stop using a library archive for the board specific stuff.
>       Instead, collect and link all the object files to produce the
>       output binary.  Only Makefile changes are involved, but correct
>       behavior depends on all boards doing the right thing.

I don't like the "weak" concept :)

> 
> 1.4) Link u-boot into a board-agnostic dynamic library, link the
>       board-specific stuff into an executable embedding a dynamic
>       linker, and package all this stuff somehow.

That is too complex. Besides there are few board-agnostic parts in
u-boot, many functions rely in included defines that are board
specific.

> Are there better options?  Which one would you prefer to see
> implemented?

Yes. The old-fashioned #define CONFIG_BOARD_INIT_F and friends
method. I would prefer that one. Its not beautiful but still
widely used and bullet-proof.

Best Regards,
Reinhard

[U-Boot] Weak symbols: request for comments

[Sebastien Carlier]

Dear Reinhard,

On 11/05/2010 12:16 PM, Reinhard Meyer wrote:
>> 1.2) Use regular (non-weak) extern declarations for overridable stuff;
>>        collect all default weak symbols into a separate library archive,
>>        to be supplied last to the linker.
>>      
> Not very practical, that would require that each driver etc. would
> be in two parts, the main part and the "weak" part. It would no need
> weak functions, however.
>    

You are entirely correct.  It would be slightly inconvenient for drivers 
that provide overridable stuff, but no non-standard feature is needed 
and the benefit of static linking is preserved.

>> 1.3) Stop using a library archive for the board specific stuff.
>>        Instead, collect and link all the object files to produce the
>>        output binary.  Only Makefile changes are involved, but correct
>>        behavior depends on all boards doing the right thing.
>>      
> I don't like the "weak" concept :)
>    

It does seem like weak symbols were designed with other uses in mind, 
such as C++ class members defined within a class declaration, or to weak 
the dependencies between libraries... but not really to allow 
overridable definitions (what if two objects want to override the same 
weak symbol in different ways?).

>> 1.4) Link u-boot into a board-agnostic dynamic library, link the
>>        board-specific stuff into an executable embedding a dynamic
>>        linker, and package all this stuff somehow.
>>      
> That is too complex. Besides there are few board-agnostic parts in
> u-boot, many functions rely in included defines that are board
> specific.
>    

Agreed.

>> Are there better options?  Which one would you prefer to see
>> implemented?
>>      
> Yes. The old-fashioned #define CONFIG_BOARD_INIT_F and friends
> method. I would prefer that one. Its not beautiful but still
> widely used and bullet-proof.
>    

Could you please elaborate?  I have looked for things like this in the 
code base but I could not find what you are referring to.

Regards,

Sebastien Carlier

[U-Boot] Weak symbols: request for comments

[Wolfgang Denk]

Dear Sebastien Carlier,

In message <4CD3F58F.8090302@gmail.com> you wrote:
> 
> It does seem like weak symbols were designed with other uses in mind, 
> such as C++ class members defined within a class declaration, or to weak 
> the dependencies between libraries... but not really to allow 
> overridable definitions (what if two objects want to override the same 
> weak symbol in different ways?).

Well, but that's exactly why they are used in library code: to allow
overridable definitions.

> > Yes. The old-fashioned #define CONFIG_BOARD_INIT_F and friends
> > method. I would prefer that one. Its not beautiful but still
> > widely used and bullet-proof.
> 
> Could you please elaborate?  I have looked for things like this in the 
> code base but I could not find what you are referring to.

Don't bother looking for it. We are happy that we have eliminated a
bit of the #ifdef mess. We will not add it again.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I had the rare misfortune of being one of the first people to try and
implement a PL/1 compiler.                             -- T. Cheatham

[U-Boot] Weak symbols: request for comments

[Sebastien Carlier]

Dear Wolfgang,

On 11/05/2010 01:23 PM, Wolfgang Denk wrote:
> Dear Sebastien Carlier,
>
> In message<4CD3F58F.8090302@gmail.com>  you wrote:
>    
>> It does seem like weak symbols were designed with other uses in mind,
>> such as C++ class members defined within a class declaration, or to weak
>> the dependencies between libraries... but not really to allow
>> overridable definitions (what if two objects want to override the same
>> weak symbol in different ways?).
>>      
> Well, but that's exactly why they are used in library code: to allow
> overridable definitions.
>    

If this usage were specifically designed for, do you think the linker 
would silently allow multiple yet conflicting weak definitions for the 
same symbol?  It is true that weak symbols can be used to support 
overridable definitions, but they seem more suitable for other uses.

Regards,

Sebastien Carlier

[U-Boot] [PATCH] Switch from library archives to partial linking

[Sebastien Carlier]

Hello all,

My previous patch missed some places that create library archives.  A 
new 100% $(AR)-free version is available here:

     http://io.oiioiio.com/~sebc/0001-Use-partial-linking-v2.patch

I have tested this patch with MAKEALL -A arm and checked that it does 
not break any build on this architecture.  I have not yet tested the 
resulting binaries, but in theory they should work.  :-)  Feedback on 
other architectures is also welcome.

For a few boards (balloon3, palmld, palmtc, pleb2, zipitz2) that disable 
CONFIG_CMD_NET, this patch also disables CONFIG_CMD_NFS to prevent 
net/nfs.o from being compiled and causing undefined symbols.

-- 
Sebastien Carlier

[U-Boot] [PATCH] Switch from library archives to partial linking

[Albert ARIBAUD]

Le 06/11/2010 15:28, Sebastien Carlier a ?crit :
> Hello all,
>
> My previous patch missed some places that create library archives.  A
> new 100% $(AR)-free version is available here:
>
>       http://io.oiioiio.com/~sebc/0001-Use-partial-linking-v2.patch
>
> I have tested this patch with MAKEALL -A arm and checked that it does
> not break any build on this architecture.  I have not yet tested the
> resulting binaries, but in theory they should work.  :-)  Feedback on
> other architectures is also welcome.
>
> For a few boards (balloon3, palmld, palmtc, pleb2, zipitz2) that disable
> CONFIG_CMD_NET, this patch also disables CONFIG_CMD_NFS to prevent
> net/nfs.o from being compiled and causing undefined symbols.

I guess if this patch is ready to be pulled in a git repo, you should 
submit it using git format-patch / git send-email, ideally as a patchset 
with each patch dealing with one lib, because clearly each ex-library 
will require its own set of custodian ack(s), thus require its own patch 
in the set.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] Switch from library archives to partial linking

[Peter Tyser]

On Sat, 2010-11-06 at 18:21 +0100, Albert ARIBAUD wrote:
> Le 06/11/2010 15:28, Sebastien Carlier a ?crit :
> > Hello all,
> >
> > My previous patch missed some places that create library archives.  A
> > new 100% $(AR)-free version is available here:
> >
> >       http://io.oiioiio.com/~sebc/0001-Use-partial-linking-v2.patch
> >
> > I have tested this patch with MAKEALL -A arm and checked that it does
> > not break any build on this architecture.  I have not yet tested the
> > resulting binaries, but in theory they should work.  :-)  Feedback on
> > other architectures is also welcome.
> >
> > For a few boards (balloon3, palmld, palmtc, pleb2, zipitz2) that disable
> > CONFIG_CMD_NET, this patch also disables CONFIG_CMD_NFS to prevent
> > net/nfs.o from being compiled and causing undefined symbols.
> 
> I guess if this patch is ready to be pulled in a git repo, you should 
> submit it using git format-patch / git send-email, ideally as a patchset 
> with each patch dealing with one lib, because clearly each ex-library 
> will require its own set of custodian ack(s), thus require its own patch 
> in the set.

You shouldn't need to send the patch using "git send-email".  The patch
is greater than U-Boot's mailing list limit (100k) and posting the patch
on a website is perfectly acceptable.  Also, it shouldn't be necessary
to split the patch into each separate patch's to address each lib.  It'd
be a lot of work on Sebastien's part to do this and not break bisection,
and most maintainers can either ack this patch, or probably don't need
to since its more of a build change, not low-level change that a
maintainer has insight into.

I had a couple of comments though:
- You need to add your "Signed-of-by: " line to the patch.
- A patch description illustrating why this approach is better than the
current approach would be appreciated.
- You shouldn't be making changes to stuff like CONFIG_CMD_NFS in this
patch.  Its unrelated, and should be dealt with in another patch.  eg
your patches could be:
1/2: Fix boards with CONFIG_CMD_NFS but !CONFIG_CMD_NET
2/2: Switch from library archives to partial linking

I just tried compiling for PowerPC and ran into this:
Configuring for cmi_mpc5xx board...
net/libnet.o: In function `rpc_req':
/home/ptyser/u-boot/u-boot/net/nfs.c:193: undefined reference to `NetEthHdrSize'
/home/ptyser/u-boot/u-boot/net/nfs.c:202: undefined reference to `NetSendUDPPacket'
net/libnet.o: In function `NfsStart':
/home/ptyser/u-boot/u-boot/net/nfs.c:741: undefined reference to `NetSetTimeout'
/home/ptyser/u-boot/u-boot/net/nfs.c:742: undefined reference to `NetSetHandler'
net/libnet.o: In function `NfsHandler':
/home/ptyser/u-boot/u-boot/net/nfs.c:656: undefined reference to `NetSetTimeout'
net/libnet.o: In function `NfsTimeout':
/home/ptyser/u-boot/u-boot/net/nfs.c:574: undefined reference to `NetStartAgain'
/home/ptyser/u-boot/u-boot/net/nfs.c:577: undefined reference to `NetSetTimeout'
net/libnet.o:(.got2+0x8): undefined reference to `NetTxPacket'
net/libnet.o:(.got2+0xc): undefined reference to `NetServerEther'
net/libnet.o:(.got2+0x18): undefined reference to `NetServerIP'
net/libnet.o:(.got2+0x1c): undefined reference to `BootFile'
net/libnet.o:(.got2+0x20): undefined reference to `NetOurIP'
net/libnet.o:(.got2+0x30): undefined reference to `NetOurGatewayIP'
net/libnet.o:(.got2+0x34): undefined reference to `NetOurSubnetMask'
net/libnet.o:(.got2+0x40): undefined reference to `NetBootFileSize'
net/libnet.o:(.got2+0x64): undefined reference to `NetState'
net/libnet.o:(.got2+0x7c): undefined reference to `NetBootFileXferSize'
make: *** [u-boot] Error 1
powerpc-linux-size: './u-boot': No such file

I'm guessing lots of boards will have this same issue.  I imagine its
due to include/config_cmd_defaults.h, so maybe if you fix the issue in
that one place all the compile issues will go away.

Best,
Peter

[U-Boot] [PATCH] Switch from library archives to partial linking

[Sebastien Carlier]

Dear Peter,

On 11/07/2010 03:16 PM, Peter Tyser wrote:
> You shouldn't need to send the patch using "git send-email".  The patch
> is greater than U-Boot's mailing list limit (100k) and posting the patch
> on a website is perfectly acceptable.  Also, it shouldn't be necessary
> to split the patch into each separate patch's to address each lib.  It'd
> be a lot of work on Sebastien's part to do this and not break bisection,
> and most maintainers can either ack this patch, or probably don't need
> to since its more of a build change, not low-level change that a
> maintainer has insight into.
>    

That makes a lot of sense.

> I had a couple of comments though:
> - You need to add your "Signed-of-by: " line to the patch.
>    

Okay.  As you may have guessed, I am a first-time git user.

> - A patch description illustrating why this approach is better than the
> current approach would be appreciated.
>    

Will do.

> - You shouldn't be making changes to stuff like CONFIG_CMD_NFS in this
> patch.  Its unrelated, and should be dealt with in another patch.  eg
> your patches could be:
> 1/2: Fix boards with CONFIG_CMD_NFS but !CONFIG_CMD_NET
> 2/2: Switch from library archives to partial linking
>    

Sounds good, will do.

> I'm guessing lots of boards will have this same issue.  I imagine its
> due to include/config_cmd_defaults.h, so maybe if you fix the issue in
> that one place all the compile issues will go away.
>    

The generic fix is to include the following lines somewhere at the end 
of the config.h generated in the mkconfig script:

#ifndef CONFIG_CMD_NET
# undef CONFIG_CMD_NFS
#endif

These lines should probable be put in a new header file; would 
config_checks.h be an ok name for it?  I suppose there might be other 
cases where a module (that is included by default) needs to be excluded 
when one of its dependencies is disabled.

Regards,

Sebastien Carlier

[U-Boot] [PATCH] Switch from library archives to partial linking

[Andreas Bießmann]

Dear Sebastien Carlier,
Am 07.11.2010 um 16:11 schrieb Sebastien Carlier:

> Dear Peter,
> 
> On 11/07/2010 03:16 PM, Peter Tyser wrote:

>> - You shouldn't be making changes to stuff like CONFIG_CMD_NFS in this
>> patch.  Its unrelated, and should be dealt with in another patch.  eg
>> your patches could be:
>> 1/2: Fix boards with CONFIG_CMD_NFS but !CONFIG_CMD_NET
>> 2/2: Switch from library archives to partial linking
>> 
> 
> Sounds good, will do.
> 
>> I'm guessing lots of boards will have this same issue.  I imagine its
>> due to include/config_cmd_defaults.h, so maybe if you fix the issue in
>> that one place all the compile issues will go away.
>> 
> 
> The generic fix is to include the following lines somewhere at the end 
> of the config.h generated in the mkconfig script:
> 
> #ifndef CONFIG_CMD_NET
> # undef CONFIG_CMD_NFS
> #endif
> 
> These lines should probable be put in a new header file; would 
> config_checks.h be an ok name for it?  I suppose there might be other 
> cases where a module (that is included by default) needs to be excluded 
> when one of its dependencies is disabled.
> 

I guess the boards are broken before your library changes too. So yes you need to split these patches.
But two points regarding your described approach.
 - The respective boards need a fix, if they do (conditionally) disable CMD_NET and miss CMD_NFS it is their fault and the respective boards config should be fixed
 - The build for net commands need a fix if they will build NFS stuff without NET stuff

I dunno if it is required to have some config_checks.h, this may grow up to unexpected complexity.

regards

Andreas Bie?mann

[U-Boot] [PATCH] Switch from library archives to partial linking

[Sebastien Carlier]

Dear Andreas,

On 11/07/2010 04:30 PM, Andreas Bie?mann wrote:
> I guess the boards are broken before your library changes too.

The boards in question do actually build with library archives.  The
final binary does not make use of the NFS stuff, therefore the linker
does not need to resolve the undefined symbols in it.

> So yes you need to split these patches.

Understood, I will post separate patches.

> But two points regarding your described approach.
>  - The respective boards need a fix, if they do (conditionally)
> disable CMD_NET and miss CMD_NFS it is their fault and the respective
> boards config should be fixed

The boards unconditionally disable CMD_NET and miss CMD_NFS.  Although
this is not a meaningful configuration, it used to build...

>  - The build for net commands need a fix if they will build NFS stuff
> without NET stuff

It is possible for net/Makefile to disable all modules if CMD_NET is
disabled.  I do not know if this is desirable, because it would allow
situations where the C code believes that some features will be included
and the build system does something else.  It seems better to ensure
consistency at the board config level, since autoconf.mk is
automatically generated from that.

> I dunno if it is required to have some config_checks.h, this may grow
> up to unexpected complexity.

How are dependencies between u-boot modules handled currently?  Are they
documented or available in any form that could be used to automatically
check that a configuration is meaningful?

Best regards,

Sebastien Carlier

[U-Boot] [PATCH] Switch from library archives to partial linking

[Andreas Bießmann]

Dear Sebastien Carlier,

Am 07.11.2010 um 17:18 schrieb Sebastien Carlier:

> Dear Andreas,
> 
> On 11/07/2010 04:30 PM, Andreas Bie?mann wrote:
>> I guess the boards are broken before your library changes too.
> 
> The boards in question do actually build with library archives.  The
> final binary does not make use of the NFS stuff, therefore the linker
> does not need to resolve the undefined symbols in it.

I see .. so I guess the build system for net should be fixed.

>> But two points regarding your described approach.
>> - The respective boards need a fix, if they do (conditionally)
>> disable CMD_NET and miss CMD_NFS it is their fault and the respective
>> boards config should be fixed
> 
> The boards unconditionally disable CMD_NET and miss CMD_NFS.  Although
> this is not a meaningful configuration, it used to build...

Indeed this configuration makes no sense but the information "let NFS stuff out" is implicit, isn't it?
To get your new linking fixed to work with this configuration you could introduce a new 'check service' to fixup those situations. But I think this should be fixed in the makefiles for net. And with that point of view those changes could go into the same patch which does the new linking. Cause you do not fixup boards to get them built but you make your linking more robust.

>> - The build for net commands need a fix if they will build NFS stuff
>> without NET stuff
> 
> It is possible for net/Makefile to disable all modules if CMD_NET is
> disabled.  

In my eyes this is the better way to go for your concrete issue, but please let some others comment. I'm a fairly new contributor to u-boot and do not know about those questions. This is only my point of view.

regards

Andreas Bie?mann

[U-Boot] [PATCH] Switch from library archives to partial linking

[Mike Frysinger]

On Sunday, November 07, 2010 10:11:14 Sebastien Carlier wrote:
> On 11/07/2010 03:16 PM, Peter Tyser wrote:
> > I'm guessing lots of boards will have this same issue.  I imagine its
> > due to include/config_cmd_defaults.h, so maybe if you fix the issue in
> > that one place all the compile issues will go away.
> 
> The generic fix is to include the following lines somewhere at the end
> of the config.h generated in the mkconfig script:
> 
> #ifndef CONFIG_CMD_NET
> # undef CONFIG_CMD_NFS
> #endif
> 
> These lines should probable be put in a new header file; would
> config_checks.h be an ok name for it?  I suppose there might be other
> cases where a module (that is included by default) needs to be excluded
> when one of its dependencies is disabled.

i dont like this sort of magc undoing of defaults.  update the board configs 
instead to undef NFS.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20101109/1965ebf5/attachment.pgp 

[U-Boot] [PATCH] Switch from library archives to partial linking

[Wolfgang Denk]

Dear Sebastien Carlier,

In message <4CD56618.4010603@gmail.com> you wrote:
>
> My previous patch missed some places that create library archives.  A 
> new 100% $(AR)-free version is available here:

Please post your patch on the mailing list for review.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You might not be as stupid as you look. This is not hard. Let's think
about this. I mean ... I'll think about this, and  you  can  join  in
when you know the words.             - Terry Pratchett, _Men at Arms_

[U-Boot] [PATCH] Switch from library archives to partial linking

[Peter Tyser]

Hi Wolfgang,

> In message <4CD56618.4010603@gmail.com> you wrote:
> >
> > My previous patch missed some places that create library archives.  A 
> > new 100% $(AR)-free version is available here:
> 
> Please post your patch on the mailing list for review.

For what its worth, Sebastien's patch is significantly larger than the
100K mailing list limit and I assume he was following the instructions
at http://www.denx.de/wiki/U-Boot/Patches.

He's been told conflicting stories about how to submit his patch - I
just wanted to clarify for him (and me) what the current preferred
method really is.

Thanks,
Peter

[U-Boot] [PATCH] Switch from library archives to partial linking

[Wolfgang Denk]

Dear Peter Tyser,

In message <1289168695.3511.11.camel@ptyser-laptop> you wrote:
>
> > > new 100% $(AR)-free version is available here:
> > 
> > Please post your patch on the mailing list for review.
> 
> For what its worth, Sebastien's patch is significantly larger than the
> 100K mailing list limit and I assume he was following the instructions
> at http://www.denx.de/wiki/U-Boot/Patches.
> 
> He's been told conflicting stories about how to submit his patch - I
> just wanted to clarify for him (and me) what the current preferred
> method really is.

Well, I would at least expect a description of the patch being posted.

Something like "new 100% $(AR)-free version is available" does not
tell me anything. It does not explain the rationale of the patch, nor
what was changed (list of files etc.), or why. It does not even
mention why the patch has not been posted.

Oh, yes, by now I reas that other posting as well, but there was no
threading used, so all this was completely out of context.

Sebastien: For a new version, I expect at least to see the full
commit message and diff-stat information posted (i. e. all what git
format-patch gives, except the actual patch data).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A fanatic is a person who can't change his mind and won't change  the
subject.                                          - Winston Churchill

[U-Boot] Weak symbols: request for comments

[Reinhard Meyer]

Dear Sebastien,
>>> Are there better options?  Which one would you prefer to see
>>> implemented?
>>>      
>> Yes. The old-fashioned #define CONFIG_BOARD_INIT_F and friends
>> method. I would prefer that one. Its not beautiful but still
>> widely used and bullet-proof.
>>    
> 
> Could you please elaborate?  I have looked for things like this in the
> code base but I could not find what you are referring to.

extracts from arch/arm/lib/board.c:

#if defined(CONFIG_ARCH_CPU_INIT)
	arch_cpu_init,		/* basic arch cpu dependent setup */
#endif
#if defined(CONFIG_BOARD_EARLY_INIT_F)
	board_early_init_f,
#endif
#if defined(CONFIG_ARCH_MISC_INIT)
	/* miscellaneous arch dependent initialisations */
	arch_misc_init ();
#endif
#if defined(CONFIG_MISC_INIT_R)
	/* miscellaneous platform dependent initialisations */
	misc_init_r ();
#endif
#ifdef BOARD_LATE_INIT
	board_late_init ();
#endif
#if defined(CONFIG_RESET_PHY_R)
	debug ("Reset Ethernet PHY\n");
	reset_phy();
#endif

Just a few that can be "enabled" by board specific defines.

"xxxboard".h #defines them and "xxxboard.c" has to implement them.

Best Regards,
Reinhard

[U-Boot] Weak symbols: request for comments

[Andreas Bießmann]

Dear Sebastien Carlier,

Am 05.11.2010 11:39, schrieb Sebastien Carlier:
> Hello all,

> So, U-boot needs to be fixed.  I can see the following ways forward:
> 
> 1.1) Stop using weak symbols; use pre-initialized function pointers
>       instead (possibly grouped in a struct, for cleanliness).
>       This has the benefit of offering a clear interface and being
>       independent of toolchain details.

sounds good to me despite of grouping. Isn't grouping tough due to
different weak functions for each architecture?

> 1.2) Use regular (non-weak) extern declarations for overridable stuff;
>       collect all default weak symbols into a separate library archive,
>       to be supplied last to the linker.

sounds messy to me. How about different weak symbols for different
architectures?

regards

Andreas Bie?mann

[U-Boot] Weak symbols: request for comments

[Joakim Tjernlund]

>
> Dear Sebastien Carlier,
>
> Am 05.11.2010 11:39, schrieb Sebastien Carlier:
> > Hello all,
>
> > So, U-boot needs to be fixed.  I can see the following ways forward:
> >
> > 1.1) Stop using weak symbols; use pre-initialized function pointers
> >       instead (possibly grouped in a struct, for cleanliness).
> >       This has the benefit of offering a clear interface and being
> >       independent of toolchain details.
>
> sounds good to me despite of grouping. Isn't grouping tough due to
> different weak functions for each architecture?

This will build more size and relocations and you get fixups too
which are/will be bad for code that runs before relocation.

 Jocke

[U-Boot] Weak symbols: request for comments

[Wolfgang Denk]

Dear Sebastien Carlier,

In message <4CD3DEFC.7010104@gmail.com> you wrote:
> 
> 1.1) Stop using weak symbols; use pre-initialized function pointers
>       instead (possibly grouped in a struct, for cleanliness).
>       This has the benefit of offering a clear interface and being
>       independent of toolchain details.

And where would the "pre-initialized function pointers" come from?
Without adding a hell of #ifdef's ?

> 1.2) Use regular (non-weak) extern declarations for overridable stuff;
>       collect all default weak symbols into a separate library archive,
>       to be supplied last to the linker.

Not realy practicable, as the code is distributed all over the place,
and should remain where it logically belongs.

> 1.3) Stop using a library archive for the board specific stuff.
>       Instead, collect and link all the object files to produce the
>       output binary.  Only Makefile changes are involved, but correct
>       behavior depends on all boards doing the right thing.

Close. I think stop using a library archives and do what Linux does
instead is the way to go.

> 1.4) Link u-boot into a board-agnostic dynamic library, link the
>       board-specific stuff into an executable embedding a dynamic
>       linker, and package all this stuff somehow.

Sounds pretty much complicated.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Chapter 1 -- The story so  far:
In the beginning the Universe was created. This has  made  a  lot  of
people very angry and been widely regarded as a bad move.

[U-Boot] Weak symbols: request for comments

[Sebastien Carlier]

Dear Wolfgang,

On 11/05/2010 01:14 PM, Wolfgang Denk wrote:
>> 1.1) Stop using weak symbols; use pre-initialized function pointers
>>        instead (possibly grouped in a struct, for cleanliness).
>>        This has the benefit of offering a clear interface and being
>>        independent of toolchain details.
>>      
> And where would the "pre-initialized function pointers" come from?
> Without adding a hell of #ifdef's ?
>    

It would not be pretty, and, as pointed out by Joakim Tjernlund, it 
would not work before relocation.

>> 1.2) Use regular (non-weak) extern declarations for overridable stuff;
>>        collect all default weak symbols into a separate library archive,
>>        to be supplied last to the linker.
>>      
> Not realy practicable, as the code is distributed all over the place,
> and should remain where it logically belongs.
>    

Each module could have its own "defaults.c" for overridable 
implementations, and the build system could collect all of these.  It is 
not a pain-free solution.

>> 1.3) Stop using a library archive for the board specific stuff.
>>        Instead, collect and link all the object files to produce the
>>        output binary.  Only Makefile changes are involved, but correct
>>        behavior depends on all boards doing the right thing.
>>      
> Close. I think stop using a library archives and do what Linux does
> instead is the way to go.
>    

Partial linking with ld -r ?  That does seem like a fairly simple change.

Regards,

Sebastien Carlier

[U-Boot] Weak symbols: request for comments

[Sebastien Carlier]

Dear Wolfgang,

I have implemented this solution:

On 11/05/2010 01:14 PM, Wolfgang Denk wrote:
> I think stop using a library archives and do what Linux does
> instead is the way to go.
>    

You will find the patch here:

     
http://io.oiioiio.com/~sebc/0001-Use-partial-linking-instead-of-building-library-arch.patch.bz2

I am not posting the patch directly to the list because it is rather large.

Feedback is very welcome!

Best regards,
Sebastien Carlier