[U-Boot] [RFC][PATCH] Code Clean-up (weak functions)
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
- 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")));
- If the weak alias decleration exceeds 80 columns, the __attribute__ is placed on the following line, indented by one tab
- 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")));
board/incaip/incaip.c | 2 +- board/purple/purple.c | 2 +- board/tb0229/tb0229.c | 2 +- .../xilinx/ppc405-generic/xilinx_ppc405_generic.c | 1 - common/cmd_boot.c | 6 ++-- common/cmd_bootm.c | 6 +++- common/cmd_elf.c | 4 +- common/cmd_ide.c | 6 ++-- common/cmd_log.c | 3 +- common/main.c | 3 +- common/serial.c | 3 +- cpu/at32ap/cpu.c | 6 +++- cpu/blackfin/cpu.h | 2 +- cpu/blackfin/reset.c | 6 +++- cpu/mips/cpu.c | 8 +++--- cpu/mpc8xxx/ddr/util.c | 5 ++- cpu/ppc4xx/44x_spd_ddr.c | 3 +- cpu/ppc4xx/44x_spd_ddr2.c | 3 +- cpu/ppc4xx/4xx_pci.c | 3 +- cpu/ppc4xx/4xx_pcie.c | 2 +- cpu/ppc4xx/fdt.c | 3 +- drivers/mtd/cfi_flash.c | 24 +++++++++++++------ drivers/net/mcfmii.c | 3 +- include/asm-avr32/arch-at32ap700x/clk.h | 2 +- include/asm-mips/reboot.h | 2 +- include/common.h | 2 +- lib_arm/board.c | 21 +++++++++++----- lib_ppc/board.c | 6 +++- lib_ppc/interrupts.c | 4 +- 29 files changed, 86 insertions(+), 57 deletions(-)
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; } 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;
diff --git a/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c b/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c index 9bd1770..57ffdc8 100644 --- a/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c +++ b/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c @@ -25,7 +25,6 @@ ulong __get_PCI_freq(void) { return 0; } - ulong get_PCI_freq(void) __attribute__((weak, alias("__get_PCI_freq")));
int __board_pre_init(void) diff --git a/common/cmd_boot.c b/common/cmd_boot.c index 6024ffe..7d77b8d 100644 --- a/common/cmd_boot.c +++ b/common/cmd_boot.c @@ -28,12 +28,12 @@ #include <command.h> #include <net.h>
-/* Allow ports to override the default behavior */ -__attribute__((weak)) -unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) +unsigned long __do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { return entry (argc, argv); } +unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) + __attribute__((weak, alias("__do_go_exec")));
int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a8f85e9..137cf65 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -159,13 +159,15 @@ void __board_lmb_reserve(struct lmb *lmb) { /* please define platform specific board_lmb_reserve() */ } -void board_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__board_lmb_reserve"))); +void board_lmb_reserve(struct lmb *lmb) + __attribute__((weak, alias("__board_lmb_reserve")));
void __arch_lmb_reserve(struct lmb *lmb) { /* please define platform specific arch_lmb_reserve() */ } -void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve"))); +void arch_lmb_reserve(struct lmb *lmb) + __attribute__((weak, alias("__arch_lmb_reserve")));
#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM diff --git a/common/cmd_elf.c b/common/cmd_elf.c index 27a4b73..4cd2975 100644 --- a/common/cmd_elf.c +++ b/common/cmd_elf.c @@ -27,8 +27,6 @@ DECLARE_GLOBAL_DATA_PTR; int valid_elf_image (unsigned long addr); unsigned long load_elf_image (unsigned long addr);
-/* Allow ports to override the default behavior */ -__attribute__((weak)) unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { unsigned long ret; @@ -52,6 +50,8 @@ unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *ar
return ret; } +unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) + __attribute__((weak, alias("__do_bootelf_exec")));
/* ====================================================================== * Interpreter command to boot an arbitrary ELF image from memory. diff --git a/common/cmd_ide.c b/common/cmd_ide.c index db05f76..0b890f0 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -529,7 +529,7 @@ __ide_outb(int dev, int port, unsigned char val) outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port))); } void inline ide_outb (int dev, int port, unsigned char val) - __attribute__((weak, alias("__ide_outb"))); + __attribute__((weak, alias("__ide_outb")));
unsigned char inline __ide_inb(int dev, int port) @@ -541,7 +541,7 @@ __ide_inb(int dev, int port) return val; } unsigned char inline ide_inb(int dev, int port) - __attribute__((weak, alias("__ide_inb"))); + __attribute__((weak, alias("__ide_inb")));
#ifdef CONFIG_TUNE_PIO int inline @@ -550,7 +550,7 @@ __ide_set_piomode(int pio_mode) return 0; } int inline ide_set_piomode(int pio_mode) - __attribute__((weak, alias("__ide_set_piomode"))); + __attribute__((weak, alias("__ide_set_piomode"))); #endif
void ide_init (void) diff --git a/common/cmd_log.c b/common/cmd_log.c index febdb90..854c30d 100644 --- a/common/cmd_log.c +++ b/common/cmd_log.c @@ -70,7 +70,8 @@ unsigned long __logbuffer_base(void) { return CONFIG_SYS_SDRAM_BASE + gd->bd->bi_memsize - LOGBUFF_LEN; } -unsigned long logbuffer_base (void) __attribute__((weak, alias("__logbuffer_base"))); +unsigned long logbuffer_base (void) + __attribute__((weak, alias("__logbuffer_base")));
void logbuff_init_ptrs (void) { diff --git a/common/main.c b/common/main.c index a999a5d..5b9da3b 100644 --- a/common/main.c +++ b/common/main.c @@ -48,7 +48,8 @@ DECLARE_GLOBAL_DATA_PTR; * Board-specific Platform code can reimplement show_boot_progress () if needed */ void inline __show_boot_progress (int val) {} -void inline show_boot_progress (int val) __attribute__((weak, alias("__show_boot_progress"))); +void inline show_boot_progress (int val) + __attribute__((weak, alias("__show_boot_progress")));
#if defined(CONFIG_BOOT_RETRY_TIME) && defined(CONFIG_RESET_TO_RETRY) extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* for do_reset() prototype */ diff --git a/common/serial.c b/common/serial.c index b38d1e7..a7c6946 100644 --- a/common/serial.c +++ b/common/serial.c @@ -75,7 +75,8 @@ struct serial_device *__default_serial_console (void) #endif }
-struct serial_device *default_serial_console(void) __attribute__((weak, alias("__default_serial_console"))); +struct serial_device *default_serial_console(void) + __attribute__((weak, alias("__default_serial_console"))); #endif
int serial_register (struct serial_device *dev) diff --git a/cpu/at32ap/cpu.c b/cpu/at32ap/cpu.c index f92d3e2..4e7eded 100644 --- a/cpu/at32ap/cpu.c +++ b/cpu/at32ap/cpu.c @@ -43,6 +43,9 @@
DECLARE_GLOBAL_DATA_PTR;
+void inline __gclk_init(void) {} +void inline gclk_init (void) __attribute__((weak, alias("__gclk_init"))); + int cpu_init(void) { extern void _evba(void); @@ -65,8 +68,7 @@ int cpu_init(void) sysreg_write(EVBA, (unsigned long)&_evba); asm volatile("csrf %0" : : "i"(SYSREG_EM_OFFSET));
- if(gclk_init) - gclk_init(); + gclk_init();
return 0; } diff --git a/cpu/blackfin/cpu.h b/cpu/blackfin/cpu.h index 0a13c28..5aa1787 100644 --- a/cpu/blackfin/cpu.h +++ b/cpu/blackfin/cpu.h @@ -27,7 +27,7 @@
#include <command.h>
-void board_reset(void) __attribute__((__weak__)); +void board_reset(void); void bfin_reset_or_hang(void) __attribute__((__noreturn__)); void bfin_panic(struct pt_regs *reg); void dump(struct pt_regs *regs); diff --git a/cpu/blackfin/reset.c b/cpu/blackfin/reset.c index d1e34b3..e89b980 100644 --- a/cpu/blackfin/reset.c +++ b/cpu/blackfin/reset.c @@ -65,6 +65,9 @@ void bfin_reset(void) } }
+void inline __board_reset(void) {} +void inline board_reset (void) __attribute__((weak, alias("__board_reset"))); + /* We need to trampoline ourselves up into L1 since our linker * does not have relaxtion support and will only generate a * PC relative call with a 25 bit immediate. This is not enough @@ -73,8 +76,7 @@ void bfin_reset(void) __attribute__ ((__noreturn__)) static inline void bfin_reset_trampoline(void) { - if (board_reset) - board_reset(); + board_reset(); while (1) asm("jump (%0);" : : "a" (bfin_reset)); } 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; diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c index 27c135b..56de559 100644 --- a/cpu/mpc8xxx/ddr/util.c +++ b/cpu/mpc8xxx/ddr/util.c @@ -97,7 +97,8 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params, } }
-__attribute__((weak, alias("__fsl_ddr_set_lawbar"))) void +void fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params, unsigned int memctl_interleaved, - unsigned int ctrl_num); + unsigned int ctrl_num) + __attribute__((weak, alias("__fsl_ddr_set_lawbar"))); diff --git a/cpu/ppc4xx/44x_spd_ddr.c b/cpu/ppc4xx/44x_spd_ddr.c index 153391e..79976a3 100644 --- a/cpu/ppc4xx/44x_spd_ddr.c +++ b/cpu/ppc4xx/44x_spd_ddr.c @@ -79,7 +79,8 @@ void __spd_ddr_init_hang (void) { hang (); } -void spd_ddr_init_hang (void) __attribute__((weak, alias("__spd_ddr_init_hang"))); +void spd_ddr_init_hang (void) + __attribute__((weak, alias("__spd_ddr_init_hang")));
/*-----------------------------------------------------------------------------+ | General Definition diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c index b40e4b1..d456b97 100644 --- a/cpu/ppc4xx/44x_spd_ddr2.c +++ b/cpu/ppc4xx/44x_spd_ddr2.c @@ -171,7 +171,8 @@ void __spd_ddr_init_hang (void) { hang (); } -void spd_ddr_init_hang (void) __attribute__((weak, alias("__spd_ddr_init_hang"))); +void spd_ddr_init_hang (void) + __attribute__((weak, alias("__spd_ddr_init_hang")));
/* * To provide an interface for board specific config values in this common diff --git a/cpu/ppc4xx/4xx_pci.c b/cpu/ppc4xx/4xx_pci.c index e8871fc..b1f90b8 100644 --- a/cpu/ppc4xx/4xx_pci.c +++ b/cpu/ppc4xx/4xx_pci.c @@ -89,7 +89,8 @@ int __pci_pre_init(struct pci_controller *hose) { return 1; } -int pci_pre_init(struct pci_controller *hose) __attribute__((weak, alias("__pci_pre_init"))); +int pci_pre_init(struct pci_controller *hose) + __attribute__((weak, alias("__pci_pre_init")));
#if defined(CONFIG_405GP) || defined(CONFIG_405EP)
diff --git a/cpu/ppc4xx/4xx_pcie.c b/cpu/ppc4xx/4xx_pcie.c index fd40d8a..970932b 100644 --- a/cpu/ppc4xx/4xx_pcie.c +++ b/cpu/ppc4xx/4xx_pcie.c @@ -729,7 +729,7 @@ int __ppc4xx_init_pcie_port_hw(int port, int rootport) #endif /* CONFIG_405EX */
int ppc4xx_init_pcie_port_hw(int port, int rootport) -__attribute__((weak, alias("__ppc4xx_init_pcie_port_hw"))); + __attribute__((weak, alias("__ppc4xx_init_pcie_port_hw")));
/* * We map PCI Express configuration access into the 512MB regions diff --git a/cpu/ppc4xx/fdt.c b/cpu/ppc4xx/fdt.c index c55e1cf..cd6ed58 100644 --- a/cpu/ppc4xx/fdt.c +++ b/cpu/ppc4xx/fdt.c @@ -72,7 +72,8 @@ void __ft_board_setup(void *blob, bd_t *bd) fdt_strerror(rc)); } } -void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup"))); +void ft_board_setup(void *blob, bd_t *bd + __attribute__((weak, alias("__ft_board_setup")));
/* * Fixup all PCIe nodes by setting the device_type property diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index e8afe99..5055274 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -251,14 +251,22 @@ static u64 __flash_read64(void *addr) }
#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS -void flash_write8(u8 value, void *addr)__attribute__((weak, alias("__flash_write8"))); -void flash_write16(u16 value, void *addr)__attribute__((weak, alias("__flash_write16"))); -void flash_write32(u32 value, void *addr)__attribute__((weak, alias("__flash_write32"))); -void flash_write64(u64 value, void *addr)__attribute__((weak, alias("__flash_write64"))); -u8 flash_read8(void *addr)__attribute__((weak, alias("__flash_read8"))); -u16 flash_read16(void *addr)__attribute__((weak, alias("__flash_read16"))); -u32 flash_read32(void *addr)__attribute__((weak, alias("__flash_read32"))); -u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); +void flash_write8(u8 value, void *addr) + __attribute__((weak, alias("__flash_write8"))); +void flash_write16(u16 value, void *addr) + __attribute__((weak, alias("__flash_write16"))); +void flash_write32(u32 value, void *addr) + __attribute__((weak, alias("__flash_write32"))); +void flash_write64(u64 value, void *addr) + __attribute__((weak, alias("__flash_write64"))); +u8 flash_read8(void *addr) + __attribute__((weak, alias("__flash_read8"))); +u16 flash_read16(void *addr) + __attribute__((weak, alias("__flash_read16"))); +u32 flash_read32(void *addr) + __attribute__((weak, alias("__flash_read32"))); +u64 flash_read64(void *addr) + __attribute__((weak, alias("__flash_read64"))); #else #define flash_write8 __flash_write8 #define flash_write16 __flash_write16 diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 2b733c6..41e1c94 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -218,8 +218,6 @@ int mii_discover_phy(struct eth_device *dev) } #endif /* CONFIG_SYS_DISCOVER_PHY */
-void mii_init(void) __attribute__((weak,alias("__mii_init"))); - void __mii_init(void) { FEC_INFO_T *info; @@ -269,6 +267,7 @@ void __mii_init(void) info->dup_spd = miiphy_duplex(dev->name, info->phy_addr) << 16; info->dup_spd |= miiphy_speed(dev->name, info->phy_addr); } +void mii_init(void) __attribute__((weak,alias("__mii_init")));
/* * Read and write a MII PHY register, routines used by MII Utilities diff --git a/include/asm-avr32/arch-at32ap700x/clk.h b/include/asm-avr32/arch-at32ap700x/clk.h index 7817572..7a23fd8 100644 --- a/include/asm-avr32/arch-at32ap700x/clk.h +++ b/include/asm-avr32/arch-at32ap700x/clk.h @@ -82,7 +82,7 @@ static inline unsigned long get_spi_clk_rate(unsigned int dev_id) #endif
extern void clk_init(void); -extern void gclk_init(void) __attribute__((weak)); +extern void gclk_init(void);
/* Board code may need the SDRAM base clock as a compile-time constant */ #define SDRAMC_BUS_HZ (MAIN_CLK_RATE >> CONFIG_SYS_CLKDIV_HSB) 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 */ diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /* * Board-specific Platform code can reimplement show_boot_progress () if needed */ -void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
#ifdef CONFIG_INIT_CRITICAL #error CONFIG_INIT_CRITICAL is deprecated! diff --git a/lib_arm/board.c b/lib_arm/board.c index 2358beb..67b9469 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -123,19 +123,26 @@ void *sbrk (ptrdiff_t increment) * May be supplied by boards if desired */ void inline __coloured_LED_init (void) {} -void inline coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init"))); +void inline coloured_LED_init (void) + __attribute__((weak, alias("__coloured_LED_init"))); void inline __red_LED_on (void) {} -void inline red_LED_on (void) __attribute__((weak, alias("__red_LED_on"))); +void inline red_LED_on (void) + __attribute__((weak, alias("__red_LED_on"))); void inline __red_LED_off(void) {} -void inline red_LED_off(void) __attribute__((weak, alias("__red_LED_off"))); +void inline red_LED_off(void) + __attribute__((weak, alias("__red_LED_off"))); void inline __green_LED_on(void) {} -void inline green_LED_on(void) __attribute__((weak, alias("__green_LED_on"))); +void inline green_LED_on(void) + __attribute__((weak, alias("__green_LED_on"))); void inline __green_LED_off(void) {} -void inline green_LED_off(void)__attribute__((weak, alias("__green_LED_off"))); +void inline green_LED_off(void) + __attribute__((weak, alias("__green_LED_off"))); void inline __yellow_LED_on(void) {} -void inline yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on"))); +void inline yellow_LED_on(void) + __attribute__((weak, alias("__yellow_LED_on"))); void inline __yellow_LED_off(void) {} -void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off"))); +void inline yellow_LED_off(void) + __attribute__((weak, alias("__yellow_LED_off")));
/************************************************************************ * Init Utilities * diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 289a32a..caebc2b 100644 --- a/lib_ppc/board.c +++ b/lib_ppc/board.c @@ -206,7 +206,8 @@ void __board_add_ram_info(int use_default) { /* please define platform specific board_add_ram_info() */ } -void board_add_ram_info(int) __attribute__((weak, alias("__board_add_ram_info"))); +void board_add_ram_info(int) + __attribute__((weak, alias("__board_add_ram_info")));
static int init_func_ram (void) @@ -643,7 +644,8 @@ int __is_sata_supported(void) * board have such issue.*/ return 1; } -int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported"))); +int is_sata_supported(void) + __attribute__((weak, alias("__is_sata_supported")));
/************************************************************************ * diff --git a/lib_ppc/interrupts.c b/lib_ppc/interrupts.c index f603170..696259b 100644 --- a/lib_ppc/interrupts.c +++ b/lib_ppc/interrupts.c @@ -32,12 +32,12 @@ #endif
#ifdef CONFIG_SHOW_ACTIVITY -void board_show_activity (ulong) __attribute__((weak, alias("__board_show_activity"))); - void __board_show_activity (ulong dummy) { return; } +void board_show_activity (ulong) + __attribute__((weak, alias("__board_show_activity"))); #endif /* CONFIG_SHOW_ACTIVITY */
#ifndef CONFIG_SYS_WATCHDOG_FREQ -- 1.6.0.2.GIT
Hello Graeme,
2008/12/13 Graeme Russ graeme.russ@gmail.com:
This patch makes all definitions, declarations and usages of weak functions consistent.
Signed-off-by: Graeme Russ graeme.russ@gmail.com
Just curious: What is the relation of this patch to the problem discussed earlier: http://www.mail-archive.com/u-boot@lists.denx.de/msg05367.html Does this patch repair anything, or could it maybe break things? I ask this because the weak linking seemed not work as many expected, and I guess you cannot test all architectures/boards...
- If the weak alias decleration exceeds 80 columns, the __attribute__ is placed
on the following line, indented by one tab
The benefit on 1 line would be that it is easy recognisable with grep which instance of a function is actually being used (or in fact should be used due to the fuzzy behaviour of weak in U-boot...) without having to go through all the code. That benefit is gone when it is moved across different lines.
Maybe it is an idea to move the attribute(weak,...) construction to a macro, like in Linux: #define __weak __attribute__((weak))
In that case it can be changed easier when a non-GCC compiler is being used, and would keep the lines shorter, such that it still fits on 1 line?
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? Removing it would keep the lines short as well...
Kind Regards,
Remy
- There is no purely weak functions and therfore no longer code like: if (do_something) do_somthing();
board/incaip/incaip.c | 2 +- board/purple/purple.c | 2 +- board/tb0229/tb0229.c | 2 +- .../xilinx/ppc405-generic/xilinx_ppc405_generic.c | 1 - common/cmd_boot.c | 6 ++-- common/cmd_bootm.c | 6 +++- common/cmd_elf.c | 4 +- common/cmd_ide.c | 6 ++-- common/cmd_log.c | 3 +- common/main.c | 3 +- common/serial.c | 3 +- cpu/at32ap/cpu.c | 6 +++- cpu/blackfin/cpu.h | 2 +- cpu/blackfin/reset.c | 6 +++- cpu/mips/cpu.c | 8 +++--- cpu/mpc8xxx/ddr/util.c | 5 ++- cpu/ppc4xx/44x_spd_ddr.c | 3 +- cpu/ppc4xx/44x_spd_ddr2.c | 3 +- cpu/ppc4xx/4xx_pci.c | 3 +- cpu/ppc4xx/4xx_pcie.c | 2 +- cpu/ppc4xx/fdt.c | 3 +- drivers/mtd/cfi_flash.c | 24 +++++++++++++------ drivers/net/mcfmii.c | 3 +- include/asm-avr32/arch-at32ap700x/clk.h | 2 +- include/asm-mips/reboot.h | 2 +- include/common.h | 2 +- lib_arm/board.c | 21 +++++++++++----- lib_ppc/board.c | 6 +++- lib_ppc/interrupts.c | 4 +- 29 files changed, 86 insertions(+), 57 deletions(-)
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; } 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;
diff --git a/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c b/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c index 9bd1770..57ffdc8 100644 --- a/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c +++ b/board/xilinx/ppc405-generic/xilinx_ppc405_generic.c @@ -25,7 +25,6 @@ ulong __get_PCI_freq(void) { return 0; }
ulong get_PCI_freq(void) __attribute__((weak, alias("__get_PCI_freq")));
int __board_pre_init(void) diff --git a/common/cmd_boot.c b/common/cmd_boot.c index 6024ffe..7d77b8d 100644 --- a/common/cmd_boot.c +++ b/common/cmd_boot.c @@ -28,12 +28,12 @@ #include <command.h> #include <net.h>
-/* Allow ports to override the default behavior */ -__attribute__((weak)) -unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) +unsigned long __do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { return entry (argc, argv); } +unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
__attribute__((weak, alias("__do_go_exec")));int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a8f85e9..137cf65 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -159,13 +159,15 @@ void __board_lmb_reserve(struct lmb *lmb) { /* please define platform specific board_lmb_reserve() */ } -void board_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__board_lmb_reserve"))); +void board_lmb_reserve(struct lmb *lmb)
__attribute__((weak, alias("__board_lmb_reserve")));void __arch_lmb_reserve(struct lmb *lmb) { /* please define platform specific arch_lmb_reserve() */ } -void arch_lmb_reserve(struct lmb *lmb) __attribute__((weak, alias("__arch_lmb_reserve"))); +void arch_lmb_reserve(struct lmb *lmb)
__attribute__((weak, alias("__arch_lmb_reserve")));#if defined(__ARM__) #define IH_INITRD_ARCH IH_ARCH_ARM diff --git a/common/cmd_elf.c b/common/cmd_elf.c index 27a4b73..4cd2975 100644 --- a/common/cmd_elf.c +++ b/common/cmd_elf.c @@ -27,8 +27,6 @@ DECLARE_GLOBAL_DATA_PTR; int valid_elf_image (unsigned long addr); unsigned long load_elf_image (unsigned long addr);
-/* Allow ports to override the default behavior */ -__attribute__((weak)) unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { unsigned long ret; @@ -52,6 +50,8 @@ unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *ar
return ret;} +unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
__attribute__((weak, alias("__do_bootelf_exec")));/* ======================================================================
- Interpreter command to boot an arbitrary ELF image from memory.
diff --git a/common/cmd_ide.c b/common/cmd_ide.c index db05f76..0b890f0 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -529,7 +529,7 @@ __ide_outb(int dev, int port, unsigned char val) outb(val, (ATA_CURR_BASE(dev)+CONFIG_SYS_ATA_PORT_ADDR(port))); } void inline ide_outb (int dev, int port, unsigned char val)
__attribute__((weak, alias("__ide_outb")));
__attribute__((weak, alias("__ide_outb")));unsigned char inline __ide_inb(int dev, int port) @@ -541,7 +541,7 @@ __ide_inb(int dev, int port) return val; } unsigned char inline ide_inb(int dev, int port)
__attribute__((weak, alias("__ide_inb")));
__attribute__((weak, alias("__ide_inb")));#ifdef CONFIG_TUNE_PIO int inline @@ -550,7 +550,7 @@ __ide_set_piomode(int pio_mode) return 0; } int inline ide_set_piomode(int pio_mode)
__attribute__((weak, alias("__ide_set_piomode")));
__attribute__((weak, alias("__ide_set_piomode")));#endif
void ide_init (void) diff --git a/common/cmd_log.c b/common/cmd_log.c index febdb90..854c30d 100644 --- a/common/cmd_log.c +++ b/common/cmd_log.c @@ -70,7 +70,8 @@ unsigned long __logbuffer_base(void) { return CONFIG_SYS_SDRAM_BASE + gd->bd->bi_memsize - LOGBUFF_LEN; } -unsigned long logbuffer_base (void) __attribute__((weak, alias("__logbuffer_base"))); +unsigned long logbuffer_base (void)
__attribute__((weak, alias("__logbuffer_base")));void logbuff_init_ptrs (void) { diff --git a/common/main.c b/common/main.c index a999a5d..5b9da3b 100644 --- a/common/main.c +++ b/common/main.c @@ -48,7 +48,8 @@ DECLARE_GLOBAL_DATA_PTR;
- Board-specific Platform code can reimplement show_boot_progress () if needed
*/ void inline __show_boot_progress (int val) {} -void inline show_boot_progress (int val) __attribute__((weak, alias("__show_boot_progress"))); +void inline show_boot_progress (int val)
__attribute__((weak, alias("__show_boot_progress")));#if defined(CONFIG_BOOT_RETRY_TIME) && defined(CONFIG_RESET_TO_RETRY) extern int do_reset (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]); /* for do_reset() prototype */ diff --git a/common/serial.c b/common/serial.c index b38d1e7..a7c6946 100644 --- a/common/serial.c +++ b/common/serial.c @@ -75,7 +75,8 @@ struct serial_device *__default_serial_console (void) #endif }
-struct serial_device *default_serial_console(void) __attribute__((weak, alias("__default_serial_console"))); +struct serial_device *default_serial_console(void)
__attribute__((weak, alias("__default_serial_console")));#endif
int serial_register (struct serial_device *dev) diff --git a/cpu/at32ap/cpu.c b/cpu/at32ap/cpu.c index f92d3e2..4e7eded 100644 --- a/cpu/at32ap/cpu.c +++ b/cpu/at32ap/cpu.c @@ -43,6 +43,9 @@
DECLARE_GLOBAL_DATA_PTR;
+void inline __gclk_init(void) {} +void inline gclk_init (void) __attribute__((weak, alias("__gclk_init")));
int cpu_init(void) { extern void _evba(void); @@ -65,8 +68,7 @@ int cpu_init(void) sysreg_write(EVBA, (unsigned long)&_evba); asm volatile("csrf %0" : : "i"(SYSREG_EM_OFFSET));
if(gclk_init)gclk_init();
gclk_init(); return 0;} diff --git a/cpu/blackfin/cpu.h b/cpu/blackfin/cpu.h index 0a13c28..5aa1787 100644 --- a/cpu/blackfin/cpu.h +++ b/cpu/blackfin/cpu.h @@ -27,7 +27,7 @@
#include <command.h>
-void board_reset(void) __attribute__((__weak__)); +void board_reset(void); void bfin_reset_or_hang(void) __attribute__((__noreturn__)); void bfin_panic(struct pt_regs *reg); void dump(struct pt_regs *regs); diff --git a/cpu/blackfin/reset.c b/cpu/blackfin/reset.c index d1e34b3..e89b980 100644 --- a/cpu/blackfin/reset.c +++ b/cpu/blackfin/reset.c @@ -65,6 +65,9 @@ void bfin_reset(void) } }
+void inline __board_reset(void) {} +void inline board_reset (void) __attribute__((weak, alias("__board_reset")));
/* We need to trampoline ourselves up into L1 since our linker
- does not have relaxtion support and will only generate a
- PC relative call with a 25 bit immediate. This is not enough
@@ -73,8 +76,7 @@ void bfin_reset(void) __attribute__ ((__noreturn__)) static inline void bfin_reset_trampoline(void) {
if (board_reset)board_reset();
board_reset(); while (1) asm("jump (%0);" : : "a" (bfin_reset));} 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;diff --git a/cpu/mpc8xxx/ddr/util.c b/cpu/mpc8xxx/ddr/util.c index 27c135b..56de559 100644 --- a/cpu/mpc8xxx/ddr/util.c +++ b/cpu/mpc8xxx/ddr/util.c @@ -97,7 +97,8 @@ __fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params, } }
-__attribute__((weak, alias("__fsl_ddr_set_lawbar"))) void +void fsl_ddr_set_lawbar(const common_timing_params_t *memctl_common_params, unsigned int memctl_interleaved,
unsigned int ctrl_num);
unsigned int ctrl_num)__attribute__((weak, alias("__fsl_ddr_set_lawbar")));diff --git a/cpu/ppc4xx/44x_spd_ddr.c b/cpu/ppc4xx/44x_spd_ddr.c index 153391e..79976a3 100644 --- a/cpu/ppc4xx/44x_spd_ddr.c +++ b/cpu/ppc4xx/44x_spd_ddr.c @@ -79,7 +79,8 @@ void __spd_ddr_init_hang (void) { hang (); } -void spd_ddr_init_hang (void) __attribute__((weak, alias("__spd_ddr_init_hang"))); +void spd_ddr_init_hang (void)
__attribute__((weak, alias("__spd_ddr_init_hang")));/*-----------------------------------------------------------------------------+ | General Definition diff --git a/cpu/ppc4xx/44x_spd_ddr2.c b/cpu/ppc4xx/44x_spd_ddr2.c index b40e4b1..d456b97 100644 --- a/cpu/ppc4xx/44x_spd_ddr2.c +++ b/cpu/ppc4xx/44x_spd_ddr2.c @@ -171,7 +171,8 @@ void __spd_ddr_init_hang (void) { hang (); } -void spd_ddr_init_hang (void) __attribute__((weak, alias("__spd_ddr_init_hang"))); +void spd_ddr_init_hang (void)
__attribute__((weak, alias("__spd_ddr_init_hang")));/*
- To provide an interface for board specific config values in this common
diff --git a/cpu/ppc4xx/4xx_pci.c b/cpu/ppc4xx/4xx_pci.c index e8871fc..b1f90b8 100644 --- a/cpu/ppc4xx/4xx_pci.c +++ b/cpu/ppc4xx/4xx_pci.c @@ -89,7 +89,8 @@ int __pci_pre_init(struct pci_controller *hose) { return 1; } -int pci_pre_init(struct pci_controller *hose) __attribute__((weak, alias("__pci_pre_init"))); +int pci_pre_init(struct pci_controller *hose)
__attribute__((weak, alias("__pci_pre_init")));#if defined(CONFIG_405GP) || defined(CONFIG_405EP)
diff --git a/cpu/ppc4xx/4xx_pcie.c b/cpu/ppc4xx/4xx_pcie.c index fd40d8a..970932b 100644 --- a/cpu/ppc4xx/4xx_pcie.c +++ b/cpu/ppc4xx/4xx_pcie.c @@ -729,7 +729,7 @@ int __ppc4xx_init_pcie_port_hw(int port, int rootport) #endif /* CONFIG_405EX */
int ppc4xx_init_pcie_port_hw(int port, int rootport) -__attribute__((weak, alias("__ppc4xx_init_pcie_port_hw")));
__attribute__((weak, alias("__ppc4xx_init_pcie_port_hw")));/*
- We map PCI Express configuration access into the 512MB regions
diff --git a/cpu/ppc4xx/fdt.c b/cpu/ppc4xx/fdt.c index c55e1cf..cd6ed58 100644 --- a/cpu/ppc4xx/fdt.c +++ b/cpu/ppc4xx/fdt.c @@ -72,7 +72,8 @@ void __ft_board_setup(void *blob, bd_t *bd) fdt_strerror(rc)); } } -void ft_board_setup(void *blob, bd_t *bd) __attribute__((weak, alias("__ft_board_setup"))); +void ft_board_setup(void *blob, bd_t *bd
__attribute__((weak, alias("__ft_board_setup")));/*
- Fixup all PCIe nodes by setting the device_type property
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index e8afe99..5055274 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -251,14 +251,22 @@ static u64 __flash_read64(void *addr) }
#ifdef CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS -void flash_write8(u8 value, void *addr)__attribute__((weak, alias("__flash_write8"))); -void flash_write16(u16 value, void *addr)__attribute__((weak, alias("__flash_write16"))); -void flash_write32(u32 value, void *addr)__attribute__((weak, alias("__flash_write32"))); -void flash_write64(u64 value, void *addr)__attribute__((weak, alias("__flash_write64"))); -u8 flash_read8(void *addr)__attribute__((weak, alias("__flash_read8"))); -u16 flash_read16(void *addr)__attribute__((weak, alias("__flash_read16"))); -u32 flash_read32(void *addr)__attribute__((weak, alias("__flash_read32"))); -u64 flash_read64(void *addr)__attribute__((weak, alias("__flash_read64"))); +void flash_write8(u8 value, void *addr)
__attribute__((weak, alias("__flash_write8")));+void flash_write16(u16 value, void *addr)
__attribute__((weak, alias("__flash_write16")));+void flash_write32(u32 value, void *addr)
__attribute__((weak, alias("__flash_write32")));+void flash_write64(u64 value, void *addr)
__attribute__((weak, alias("__flash_write64")));+u8 flash_read8(void *addr)
__attribute__((weak, alias("__flash_read8")));+u16 flash_read16(void *addr)
__attribute__((weak, alias("__flash_read16")));+u32 flash_read32(void *addr)
__attribute__((weak, alias("__flash_read32")));+u64 flash_read64(void *addr)
__attribute__((weak, alias("__flash_read64")));#else #define flash_write8 __flash_write8 #define flash_write16 __flash_write16 diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 2b733c6..41e1c94 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -218,8 +218,6 @@ int mii_discover_phy(struct eth_device *dev) } #endif /* CONFIG_SYS_DISCOVER_PHY */
-void mii_init(void) __attribute__((weak,alias("__mii_init")));
void __mii_init(void) { FEC_INFO_T *info; @@ -269,6 +267,7 @@ void __mii_init(void) info->dup_spd = miiphy_duplex(dev->name, info->phy_addr) << 16; info->dup_spd |= miiphy_speed(dev->name, info->phy_addr); } +void mii_init(void) __attribute__((weak,alias("__mii_init")));
/*
- Read and write a MII PHY register, routines used by MII Utilities
diff --git a/include/asm-avr32/arch-at32ap700x/clk.h b/include/asm-avr32/arch-at32ap700x/clk.h index 7817572..7a23fd8 100644 --- a/include/asm-avr32/arch-at32ap700x/clk.h +++ b/include/asm-avr32/arch-at32ap700x/clk.h @@ -82,7 +82,7 @@ static inline unsigned long get_spi_clk_rate(unsigned int dev_id) #endif
extern void clk_init(void); -extern void gclk_init(void) __attribute__((weak)); +extern void gclk_init(void);
/* Board code may need the SDRAM base clock as a compile-time constant */ #define SDRAMC_BUS_HZ (MAIN_CLK_RATE >> CONFIG_SYS_CLKDIV_HSB) 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 */ diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /*
- Board-specific Platform code can reimplement show_boot_progress () if needed
*/ -void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
#ifdef CONFIG_INIT_CRITICAL #error CONFIG_INIT_CRITICAL is deprecated! diff --git a/lib_arm/board.c b/lib_arm/board.c index 2358beb..67b9469 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -123,19 +123,26 @@ void *sbrk (ptrdiff_t increment)
- May be supplied by boards if desired
*/ void inline __coloured_LED_init (void) {} -void inline coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init"))); +void inline coloured_LED_init (void)
__attribute__((weak, alias("__coloured_LED_init")));void inline __red_LED_on (void) {} -void inline red_LED_on (void) __attribute__((weak, alias("__red_LED_on"))); +void inline red_LED_on (void)
__attribute__((weak, alias("__red_LED_on")));void inline __red_LED_off(void) {} -void inline red_LED_off(void) __attribute__((weak, alias("__red_LED_off"))); +void inline red_LED_off(void)
__attribute__((weak, alias("__red_LED_off")));void inline __green_LED_on(void) {} -void inline green_LED_on(void) __attribute__((weak, alias("__green_LED_on"))); +void inline green_LED_on(void)
__attribute__((weak, alias("__green_LED_on")));void inline __green_LED_off(void) {} -void inline green_LED_off(void)__attribute__((weak, alias("__green_LED_off"))); +void inline green_LED_off(void)
__attribute__((weak, alias("__green_LED_off")));void inline __yellow_LED_on(void) {} -void inline yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on"))); +void inline yellow_LED_on(void)
__attribute__((weak, alias("__yellow_LED_on")));void inline __yellow_LED_off(void) {} -void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off"))); +void inline yellow_LED_off(void)
__attribute__((weak, alias("__yellow_LED_off")));/************************************************************************
- Init Utilities *
diff --git a/lib_ppc/board.c b/lib_ppc/board.c index 289a32a..caebc2b 100644 --- a/lib_ppc/board.c +++ b/lib_ppc/board.c @@ -206,7 +206,8 @@ void __board_add_ram_info(int use_default) { /* please define platform specific board_add_ram_info() */ } -void board_add_ram_info(int) __attribute__((weak, alias("__board_add_ram_info"))); +void board_add_ram_info(int)
__attribute__((weak, alias("__board_add_ram_info")));static int init_func_ram (void) @@ -643,7 +644,8 @@ int __is_sata_supported(void) * board have such issue.*/ return 1; } -int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported"))); +int is_sata_supported(void)
__attribute__((weak, alias("__is_sata_supported")));/************************************************************************
diff --git a/lib_ppc/interrupts.c b/lib_ppc/interrupts.c index f603170..696259b 100644 --- a/lib_ppc/interrupts.c +++ b/lib_ppc/interrupts.c @@ -32,12 +32,12 @@ #endif
#ifdef CONFIG_SHOW_ACTIVITY -void board_show_activity (ulong) __attribute__((weak, alias("__board_show_activity")));
void __board_show_activity (ulong dummy) { return; } +void board_show_activity (ulong)
__attribute__((weak, alias("__board_show_activity")));#endif /* CONFIG_SHOW_ACTIVITY */
#ifndef CONFIG_SYS_WATCHDOG_FREQ
1.6.0.2.GIT
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Remy Bohmer wrote:
Hello Graeme,
2008/12/13 Graeme Russ graeme.russ@gmail.com:
This patch makes all definitions, declarations and usages of weak functions consistent.
Signed-off-by: Graeme Russ graeme.russ@gmail.com
Just curious: What is the relation of this patch to the problem discussed earlier: http://www.mail-archive.com/u-boot@lists.denx.de/msg05367.html Does this patch repair anything, or could it maybe break things?
The problem discussed in the above thread is related to overriding weak functions not working if the override is the only function in a module (.o) within a library (.a) - This patch does not alter this behavior
I ask this because the weak linking seemed not work as many expected, and I guess you cannot test all architectures/boards...
This patch is intended to make the usage of weak functions consistent so that when anyone goes looking in the code for an example to implement their own code, they will not be presented with 4 or 5 options. This patch modifies the small handful that were inconsistent on the assumption that the (majority) worked. For example, this patch should resolve the show_boot_progress() issue as per:
http://www.mail-archive.com/u-boot@lists.denx.de/msg05998.html
- If the weak alias decleration exceeds 80 columns, the __attribute__ is placed
on the following line, indented by one tab
The benefit on 1 line would be that it is easy recognisable with grep
true - I thought about that but decided against breaking the 80 column rule - Wolfgang, what are your thoughts on this?
which instance of a function is actually being used (or in fact should be used due to the fuzzy behaviour of weak in U-boot...) without having to go through all the code. That benefit is gone when it is moved across different lines.
I didn't think about that - good point
Maybe it is an idea to move the attribute(weak,...) construction to a macro, like in Linux: #define __weak __attribute__((weak))
Yes, but it would need the alias as well - most that already exceed 80 columns would still with the macro
In that case it can be changed easier when a non-GCC compiler is being used, and would keep the lines shorter, such that it still fits on 1 line?
Potentially, but we would have to go through and touch all the weak functions, not just the small subset hit by this patch
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.
Removing it would keep the lines short as well...
Kind Regards,
Remy
Thanks for the comments,
Regards,
Graeme
On Saturday 13 December 2008 00:26:26 Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
a quick glance shows that it breaks things (the ELF and Blackfin stuff certainly appears to be wrong). i'm guessing you focused on style for the RFC part rather than the result actually being correct ... -mike
Mike,
On Mon, Dec 22, 2008 at 8:05 PM, Mike Frysinger vapier@gentoo.org wrote:
On Saturday 13 December 2008 00:26:26 Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
a quick glance shows that it breaks things (the ELF and Blackfin stuff certainly appears to be wrong). i'm guessing you focused on style for the RFC part rather than the result actually being correct ... -mike
Thanks for having a look at this. Would it be possible for you to be a bit more specific about 'wrong' if for nothing other that me gaining a better understanding of how it works, and how it breaks
I was, honestly, hoping that the changes would not break things.
Thanks,
Graeme
On Monday 22 December 2008 04:16:33 Graeme Russ wrote:
On Mon, Dec 22, 2008 at 8:05 PM, Mike Frysinger wrote:
On Saturday 13 December 2008 00:26:26 Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
a quick glance shows that it breaks things (the ELF and Blackfin stuff certainly appears to be wrong). i'm guessing you focused on style for the RFC part rather than the result actually being correct ... -mike
Thanks for having a look at this. Would it be possible for you to be a bit more specific about 'wrong' if for nothing other that me gaining a better understanding of how it works, and how it breaks
you set the aliases to functions that do not exist -mike
On Mon, Dec 22, 2008 at 10:44 PM, Mike Frysinger vapier@gentoo.org wrote:
On Monday 22 December 2008 04:16:33 Graeme Russ wrote:
On Mon, Dec 22, 2008 at 8:05 PM, Mike Frysinger wrote:
On Saturday 13 December 2008 00:26:26 Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
a quick glance shows that it breaks things (the ELF and Blackfin stuff certainly appears to be wrong). i'm guessing you focused on style for the RFC part rather than the result actually being correct ... -mike
Thanks for having a look at this. Would it be possible for you to be a bit more specific about 'wrong' if for nothing other that me gaining a better understanding of how it works, and how it breaks
you set the aliases to functions that do not exist -mike
Ack - I can see that for the ELF - the main function needs to be renamed __do_bootelf_exec ()
I cannot see the problem with Blackfin - I will freely admit that Blackfin has been fundamentally changed (any therefore needs thorough testing), but unless the Blackfin toolchain treats weak function linking differently, it _should_ "just still work"(tm)
Maybe I am not seeing the obvious?
Regards,
Graeme
On Monday 22 December 2008 06:20:27 Graeme Russ wrote:
On Mon, Dec 22, 2008 at 10:44 PM, Mike Frysinger vapier@gentoo.org wrote:
On Monday 22 December 2008 04:16:33 Graeme Russ wrote:
On Mon, Dec 22, 2008 at 8:05 PM, Mike Frysinger wrote:
On Saturday 13 December 2008 00:26:26 Graeme Russ wrote:
This patch makes all definitions, declarations and usages of weak functions consistent.
a quick glance shows that it breaks things (the ELF and Blackfin stuff certainly appears to be wrong). i'm guessing you focused on style for the RFC part rather than the result actually being correct ... -mike
Thanks for having a look at this. Would it be possible for you to be a bit more specific about 'wrong' if for nothing other that me gaining a better understanding of how it works, and how it breaks
you set the aliases to functions that do not exist
Ack - I can see that for the ELF - the main function needs to be renamed __do_bootelf_exec ()
I cannot see the problem with Blackfin - I will freely admit that Blackfin has been fundamentally changed (any therefore needs thorough testing), but unless the Blackfin toolchain treats weak function linking differently, it _should_ "just still work"(tm)
Maybe I am not seeing the obvious?
sorry, you're right, you added the new function. that said, i dont think marking those functions as "inline" is correct or makes sense. -mike
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.
- 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")));
Curious as to why you removed such code? Was it because it didn't work? If so I might have an answer for that. See my post: "Re: [U-Boot] [PATCH V4] cmd_bdinfo: move implementation to arch instead of common"
Jocke
On Wed, Dec 24, 2008 at 12:14 PM, Shinya Kuribayashi shinya.kuribayashi@necel.com wrote:
Graeme Russ wrote:
[Snip]
- 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.
Consistent code is easier to maintain - If we eliminate inconsistencies early, it stops them being spread and resulting in spaghetti code
[snip]
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.
For consistency :) - But I 100% agree that if the code is coupled to another source and meets that sources rules then it is easier and more maintainable to keep their rules - I have no objection to retaining the original names
[snip]
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.
I wasn't aiming to change the semantics of whether functions where inline or not (only weak / aliased). I agree that inline does not make sense here
[snip]
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.
I wanted to make the impact of the patch as small as possible - I agree that this would make the code cleaner, simpler and smaller but would require more extensive testing across a lot of build-tools to make sure they all fully support weak functions in this way. I am more than happy to create a patch, but it will require a _lot_ of regression testing
Regards,
Graeme
participants (5)
-
Graeme Russ -
Joakim Tjernlund -
Mike Frysinger -
Remy Bohmer -
Shinya Kuribayashi