[U-Boot] [RFC] arm/board.c: avoid ifdef using weak default functions

[U-Boot] [RFC] arm/board.c: avoid ifdef using weak default functions

[Alessandro Rubini]

From: Alessandro Rubini <rubini@gnudd.com>

While it's a matter of personal taste, I prefer to avoid ifdef when
possible.  For example, I don't like to add BOARD_LATE_INIT in the
config file just to have my board_late_init() function called.

This patch (not meant to be applied mainstram, jsut for discussion)
tries to simplify and make more readable the code in lib_arm/board.c.
If this is considered useful it can be done more seriously to all
platforms, and allow over time to remove defines in the class of
BOARD_LATE_INIT.

A serious reordering will definitely need more time, and this is just
a quick hack to show the idea; some things are suboptimal like the
arm_pci_init() thing which has to remain an ifdef and should be fixed
in a different way (I think all init function should return int
and print their own messages, to simplify this factoring out, but again
it's a matter of personal taste).

About the use of weak, I first converted .a to .o, but then found it
works nonetheless, and led functions are already weak ones in this file.

Is the idea worth pursuing? Or does it conflich with other work in
progress?

---
 lib_arm/board.c |   62 ++++++++++++++++++++++++++----------------------------
 1 files changed, 30 insertions(+), 32 deletions(-)

diff --git a/lib_arm/board.c b/lib_arm/board.c
index a44d308..50fe74b 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -61,10 +61,26 @@ DECLARE_GLOBAL_DATA_PTR;
 
 ulong monitor_flash_len;
 
-#ifdef CONFIG_HAS_DATAFLASH
-extern int  AT91F_DataflashInit(void);
-extern void dataflash_print_info(void);
-#endif
+/* Some functions may be there or not. Use a weak placeholder to avoid ifdef */
+int __normal_nop(void) { return 0; }
+void __void_nop(void) {}
+
+/* parts of init_sequence that may not be present */
+int arch_cpu_init(void) __attribute__((weak, alias("__normal_nop")));
+int interrupt_init(void) __attribute__((weak, alias("__normal_nop")));
+int print_cpuinfo(void) __attribute__((weak, alias("__normal_nop")));
+int checkboard(void) __attribute__((weak, alias("__normal_nop")));
+
+/* other functions that appear later and depend on config items */
+int AT91F_DataflashInit(void) __attribute__((weak, alias("__normal_nop")));
+void dataflash_print_info(void) __attribute__((weak, alias("__void_nop")));
+void serial_initialize(void) __attribute__((weak, alias("__void_nop")));
+void onenand_init(void) __attribute__((weak, alias("__void_nop")));
+int drv_vfd_init(void) __attribute__((weak, alias("__normal_nop")));
+void api_init(void)  __attribute__((weak, alias("__void_nop")));
+int arch_misc_init(void) __attribute__((weak, alias("__normal_nop")));
+int misc_init_r(void) __attribute__((weak, alias("__normal_nop")));
+int board_late_init(void) __attribute__((weak, alias("__normal_nop")));
 
 #ifndef CONFIG_IDENT_STRING
 #define CONFIG_IDENT_STRING ""
@@ -227,6 +243,11 @@ static int init_func_i2c (void)
 	puts ("ready\n");
 	return (0);
 }
+#else
+static int init_func_i2c (void)
+{
+	return 0;
+}
 #endif
 
 #if defined(CONFIG_CMD_PCI) || defined (CONFIG_PCI)
@@ -236,6 +257,11 @@ static int arm_pci_init(void)
 	pci_init();
 	return 0;
 }
+#else
+static int arm_pci_init(void)
+{
+	return 0;
+}
 #endif /* CONFIG_CMD_PCI || CONFIG_PCI */
 
 /*
@@ -266,32 +292,20 @@ typedef int (init_fnc_t) (void);
 int print_cpuinfo (void);
 
 init_fnc_t *init_sequence[] = {
-#if defined(CONFIG_ARCH_CPU_INIT)
 	arch_cpu_init,		/* basic arch cpu dependent setup */
-#endif
 	board_init,		/* basic board dependent setup */
-#if defined(CONFIG_USE_IRQ)
 	interrupt_init,		/* set up exceptions */
-#endif
 	timer_init,		/* initialize timer */
 	env_init,		/* initialize environment */
 	init_baudrate,		/* initialze baudrate settings */
 	serial_init,		/* serial communications setup */
 	console_init_f,		/* stage 1 init of console */
 	display_banner,		/* say that we are here */
-#if defined(CONFIG_DISPLAY_CPUINFO)
 	print_cpuinfo,		/* display cpu info (and speed) */
-#endif
-#if defined(CONFIG_DISPLAY_BOARDINFO)
 	checkboard,		/* display board info */
-#endif
-#if defined(CONFIG_HARD_I2C) || defined(CONFIG_SOFT_I2C)
 	init_func_i2c,
-#endif
 	dram_init,		/* configure available RAM banks */
-#if defined(CONFIG_CMD_PCI) || defined (CONFIG_PCI)
 	arm_pci_init,
-#endif
 	display_dram_config,
 	NULL,
 };
@@ -365,26 +379,18 @@ void start_armboot (void)
 	nand_init();		/* go init the NAND */
 #endif
 
-#if defined(CONFIG_CMD_ONENAND)
 	onenand_init();
-#endif
 
-#ifdef CONFIG_HAS_DATAFLASH
 	AT91F_DataflashInit();
 	dataflash_print_info();
-#endif
 
 	/* initialize environment */
 	env_relocate ();
 
-#ifdef CONFIG_VFD
 	/* must do this after the framebuffer is allocated */
 	drv_vfd_init();
-#endif /* CONFIG_VFD */
 
-#ifdef CONFIG_SERIAL_MULTI
 	serial_initialize();
-#endif
 
 	/* IP Address */
 	gd->bd->bi_ip_addr = getenv_IPaddr ("ipaddr");
@@ -393,21 +399,15 @@ void start_armboot (void)
 
 	jumptable_init ();
 
-#if defined(CONFIG_API)
 	/* Initialize API */
 	api_init ();
-#endif
 
 	console_init_r ();	/* fully init console as a device */
 
-#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
 
 	/* enable exceptions */
 	enable_interrupts ();
@@ -447,9 +447,7 @@ extern void davinci_eth_set_mac_addr (const u_int8_t *addr);
 	}
 #endif
 
-#ifdef BOARD_LATE_INIT
 	board_late_init ();
-#endif
 
 #ifdef CONFIG_GENERIC_MMC
 	puts ("MMC:   ");
-- 
1.6.0.2

[U-Boot] [RFC] arm/board.c: avoid ifdef using weak default functions

[Jean-Christophe PLAGNIOL-VILLARD]

On 19:19 Wed 22 Jul     , Alessandro Rubini wrote:
> From: Alessandro Rubini <rubini@gnudd.com>
> 
> While it's a matter of personal taste, I prefer to avoid ifdef when
> possible.  For example, I don't like to add BOARD_LATE_INIT in the
> config file just to have my board_late_init() function called.
> 
> This patch (not meant to be applied mainstram, jsut for discussion)
> tries to simplify and make more readable the code in lib_arm/board.c.
> If this is considered useful it can be done more seriously to all
> platforms, and allow over time to remove defines in the class of
> BOARD_LATE_INIT.
> 
> A serious reordering will definitely need more time, and this is just
> a quick hack to show the idea; some things are suboptimal like the
> arm_pci_init() thing which has to remain an ifdef and should be fixed
> in a different way (I think all init function should return int
> and print their own messages, to simplify this factoring out, but again
> it's a matter of personal taste).
> 
> About the use of weak, I first converted .a to .o, but then found it
> works nonetheless, and led functions are already weak ones in this file.
> 
> Is the idea worth pursuing? Or does it conflich with other work in
> progress?
I've already send an RFC about initcall

I'm not in favor a weak function as it will increase the size of U-Boot and
will not be easy to use as we seen a lots of time due to compiler issue

so I think the initcall mecansim will be more generic, more easier to use and
smaller than the current implementation (already tested)

Best Regards,
J.