Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
Signed-off-by: Graeme Russ graeme.russ@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.