Graeme Russ wrote: > This patch makes all definitions, declarations and usages of weak functions > consistent. > > Signed-off-by: Graeme Russ <graeme.r...@gmail.com> > --- > > WARNING: This patch hits a _lot_ of arches - Please > > The following patch applies the following rules: > > - All functions are defined without __attribute__((weak)) in header files
Agreed. > - All weak functions are declared as __function() in the source file with > funtion() __attribute__((weak, alias("function"))); on the line immediately > after the closing brace of __function() - for example: > void __do_something (args) > { > ...some code... > } > do_something(args) __atttribute__((weak, alias("__do_something"))); Why do we need to being consistent? I don't see real/technical benefits to doing so. Using alias or not should be each developer's business. I completely disagree with forcing alias use here. > - There is no purely weak functions and therfore no longer code like: > if (do_something) > do_somthing(); > All instances have been replaced by empty functions with an alias. e.g. > void __do_something (args) {} > do_something(args) __atttribute__((weak, alias("__do_something"))); Agreed, please fix them. > diff --git a/board/incaip/incaip.c b/board/incaip/incaip.c > index 3b30970..3ee3ac9 100644 > --- a/board/incaip/incaip.c > +++ b/board/incaip/incaip.c > @@ -31,7 +31,7 @@ > > extern uint incaip_get_cpuclk(void); > > -void _machine_restart(void) > +void machine_restart(void) > { > *INCA_IP_WDT_RST_REQ = 0x3f; > } Why change the function name? It's derived from Linux/MIPS, and I'd like to keep consistent with with him. Please don't touch it. > diff --git a/board/purple/purple.c b/board/purple/purple.c > index 54bef65..c243487 100644 > --- a/board/purple/purple.c > +++ b/board/purple/purple.c > @@ -54,7 +54,7 @@ extern int asc_serial_getc (void); > extern int asc_serial_tstc (void); > extern void asc_serial_setbrg (void); > > -void _machine_restart(void) > +void machine_restart(void) > { > void (*f)(void) = (void *) 0xbfc00000; > > diff --git a/board/tb0229/tb0229.c b/board/tb0229/tb0229.c > index d3f05b2..f74573b 100644 > --- a/board/tb0229/tb0229.c > +++ b/board/tb0229/tb0229.c > @@ -16,7 +16,7 @@ > #include <asm/reboot.h> > #include <pci.h> > > -void _machine_restart(void) > +void machine_restart(void) > { > void (*f)(void) = (void *) 0xbfc00000; > Ditto. > diff --git a/cpu/mips/cpu.c b/cpu/mips/cpu.c > index b7180b0..84c4730 100644 > --- a/cpu/mips/cpu.c > +++ b/cpu/mips/cpu.c > @@ -38,13 +38,13 @@ > : \ > : "i" (op), "R" (*(unsigned char *)(addr))) > > -void __attribute__((weak)) _machine_restart(void) > -{ > -} > +void inline __machine_restart(void) {} > +void inline machine_restart (void) > + __attribute__((weak, alias("__machine_restart"))); > > int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) > { > - _machine_restart(); > + machine_restart(); > > fprintf(stderr, "*** reset failed ***\n"); > return 0; Why inline? It seems totally wrong to me. > diff --git a/include/asm-mips/reboot.h b/include/asm-mips/reboot.h > index 978d206..26885c0 100644 > --- a/include/asm-mips/reboot.h > +++ b/include/asm-mips/reboot.h > @@ -9,6 +9,6 @@ > #ifndef _ASM_REBOOT_H > #define _ASM_REBOOT_H > > -extern void _machine_restart(void); > +extern void machine_restart(void); > > #endif /* _ASM_REBOOT_H */ Ditto. Then I've read whole this thread, and will vote for Remy. > > >> >> All instances have been replaced by empty functions with an alias. e.g. >> >> void __do_something (args) {} >> >> do_something(args) __atttribute__((weak, alias("__do_something"))); > > > > Notice that in Linux, the 'alias' construction is not being used > > massively. Can it be removed here also, or is it somehow mandatory > > here? > > I don't think it is mandatory but it is in the majority in u-boot. If it's not mandatory, please drop all aliases. That saves source code size (not generated u-boot image size) a little bit. Majority or not does not make sense here. -- Shinya Kuribayashi NEC Electronics _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot