[U-Boot] [TESTING PATCH] ppc: Relocation test patch

[Joakim Tjernlund]

Peter Tyser <ptyser at xes-inc.com> wrote on 18/09/2009 18:24:48:
>
>
> > > > Sorry, I don't have an example. Just a guess, weak function references:
> > > >
> > > > void weak_fun(void) __attribute__ ((weak));
> > > > if (weak_fun)
> > > >    weak_fun();
> > >
> > > Using default weak functions as well as overridden weak functions both
> > > definitely work.  So the pointers must be being updated correctly.  I
> > > guess I'm not sure where specifically a problem could arise.  Let me
> > > know if you have any additional details.  I'm hoping to send the patches
> > > out later today, maybe some review/testing will make things clearer.
> >
> > This does not work:
> >
> > void weak_fun(void) __attribute__ ((weak));
> > printf("weak_fun:%p\n", weak_fun);
> >
> > prints "weak 17f9c000" after relocation
> > for me, should be NULL when weak_fun is undefined.
>
> Ahh, I see.  I see the same thing.  In general U-Boot declares weak
> functions by either using the 'alias' attribute:
>
> static int __def_eth_init(bd_t *bis)
> {
>    return -1;
> }
> int cpu_eth_init(bd_t *bis) __attribute__((weak, alias("__def_eth_init")));
>
> or by declaring a function as weak:
>
> void __attribute__((weak)) _machine_restart(void)
> {
> }
>
> Both these scenarios work with the current relocation fixup scheme.
> What is a real world scenario (such as your example) when someone would
> declare a weak function, but not actually implement a default.  Doesn't
> that defeat the purpose of having a weak function in the first place?
> Eg why would someone use your example of:
>
> void weak_fun(void) __attribute__ ((weak));
> ...
>    if (weak_fun)
>       weak_fun();
> ...
>
> over:
>
> void weak_fun(void) __attribute__ ((weak))
> {
> };
> ...
>    weak_fun();
> ...
> (or the alias implementation)

Instead of defining empty board function you could have in generic code
if (board_init_whatever)
   board_init_whatever()

>
> I'm trying to grasp the limitations of the current relocation mechanism
> as I'm afraid I don't have time to dig through all PPC architectures'
> start.S files to fix their relocation code right now:)

Sure, I am not saying that you need to do this to make current u-boot
work. I am just saying that the reloc funs are not complete and will
bug if someone in the future wants to use weak functions( or something else
that needs the same) in the above manner, it won't work. So these should be fixed at
some point but it should be a separate patch as it has nothing to do with your reloc patch,
it is broken as is even without your patch. I just mentioned it because I remembered it now
that you were working on relocation in general. I also think the relocation funs should be
in C ( as I posted ) as this is much easier to understand. Instead of just fixing all
the reloc assembler one should replace these with calls to generic C code.
If you want to work on making u-boot fully relocatable, I have some hints for you.

 Jocke