Patchwork [U-Boot,RFC,07/22] arm: Refactor bootm to reduce #ifdefs

login
register
mail settings
Submitter Simon Glass
Date Jan. 10, 2013, 2:58 p.m.
Message ID <1357829905-6579-8-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/211033/
State RFC
Headers show

Comments

Simon Glass - Jan. 10, 2013, 2:58 p.m.
With fewer #ifdefs the code is more readable and more of the code is
compiled for all boards. Add defines in the header file to control
what features are enabled, and then use if() instead of #ifdef.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 arch/arm/include/asm/bootm.h      |   54 +++++++++++++++++++++++-
 arch/arm/include/asm/u-boot-arm.h |    2 -
 arch/arm/lib/bootm.c              |   83 ++++++++----------------------------
 3 files changed, 71 insertions(+), 68 deletions(-)
Marek Vasut - Jan. 11, 2013, 11:10 a.m.
Dear Simon Glass,

> With fewer #ifdefs the code is more readable and more of the code is
> compiled for all boards. Add defines in the header file to control
> what features are enabled, and then use if() instead of #ifdef.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Did you try building it with ELDK 4.2 and check if there's no size growth?

Best regards,
Marek Vasut
Simon Glass - Jan. 11, 2013, 3:42 p.m.
Hi Marek,

On Fri, Jan 11, 2013 at 3:10 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> With fewer #ifdefs the code is more readable and more of the code is
>> compiled for all boards. Add defines in the header file to control
>> what features are enabled, and then use if() instead of #ifdef.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Did you try building it with ELDK 4.2 and check if there's no size growth?

buildman says:

08: arm: Refactor bootm to reduce #ifdefs
       arm: (285 boards)   spl/u-boot-spl:text -0.0   text -7.9   bss +1.0

so no size growth there with 4.4.1 (the numbers are changes in bytes
for that section averaged over all boards).

The code is basically equivalent, just using a compile-time check instead.

gcc version 4.4.1 (Sourcery G++ Lite 2010q1-202)

>
> Best regards,
> Marek Vasut

Regards,
Simon

Patch

diff --git a/arch/arm/include/asm/bootm.h b/arch/arm/include/asm/bootm.h
index db2ff94..2c4fa19 100644
--- a/arch/arm/include/asm/bootm.h
+++ b/arch/arm/include/asm/bootm.h
@@ -1,4 +1,7 @@ 
-/* Copyright (C) 2011
+/*
+ * Copyright (c) 2013, Google Inc.
+ *
+ * Copyright (C) 2011
  * Corscience GmbH & Co. KG - Simon Schwarz <schwarz@corscience.de>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -19,8 +22,55 @@ 
 #ifndef ARM_BOOTM_H
 #define ARM_BOOTM_H
 
-#ifdef CONFIG_USB_DEVICE
 extern void udc_disconnect(void);
+
+#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
+		defined(CONFIG_CMDLINE_TAG) || \
+		defined(CONFIG_INITRD_TAG) || \
+		defined(CONFIG_SERIAL_TAG) || \
+		defined(CONFIG_REVISION_TAG)
+# define BOOTM_ENABLE_TAGS		1
+#else
+# define BOOTM_ENABLE_TAGS		0
+#endif
+
+#ifdef CONFIG_SETUP_MEMORY_TAGS
+# define BOOTM_ENABLE_MEMORY_TAGS	1
+#else
+# define BOOTM_ENABLE_MEMORY_TAGS	0
+#endif
+
+#ifdef CONFIG_CMDLINE_TAG
+ #define BOOTM_ENABLE_CMDLINE_TAG	1
+#else
+ #define BOOTM_ENABLE_CMDLINE_TAG	0
+#endif
+
+#ifdef CONFIG_INITRD_TAG
+ #define BOOTM_ENABLE_INITRD_TAG	1
+#else
+ #define BOOTM_ENABLE_INITRD_TAG	0
+#endif
+
+#ifdef CONFIG_SERIAL_TAG
+ #define BOOTM_ENABLE_SERIAL_TAG	1
+void get_board_serial(struct tag_serialnr *serialnr);
+#else
+ #define BOOTM_ENABLE_SERIAL_TAG	0
+static inline void get_board_serial(struct tag_serialnr *serialnr)
+{
+}
+#endif
+
+#ifdef CONFIG_REVISION_TAG
+ #define BOOTM_ENABLE_REVISION_TAG	1
+u32 get_board_rev(void);
+#else
+ #define BOOTM_ENABLE_REVISION_TAG	0
+static inline u32 get_board_rev(void)
+{
+	return 0;
+}
 #endif
 
 #endif
diff --git a/arch/arm/include/asm/u-boot-arm.h b/arch/arm/include/asm/u-boot-arm.h
index 9f3cae5..9032c13 100644
--- a/arch/arm/include/asm/u-boot-arm.h
+++ b/arch/arm/include/asm/u-boot-arm.h
@@ -58,8 +58,6 @@  int	arch_early_init_r(void);
 int	board_init(void);
 int	dram_init (void);
 void	dram_init_banksize (void);
-void	setup_serial_tag (struct tag **params);
-void	setup_revision_tag (struct tag **params);
 
 /* cpu/.../interrupt.c */
 int	arch_interrupt_init	(void);
diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1bd2730..78fc013 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -38,13 +38,7 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
 static struct tag *params;
-#endif
 
 static ulong get_sp(void)
 {
@@ -110,11 +104,6 @@  static void announce_and_cleanup(void)
 	cleanup_before_linux();
 }
 
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
 static void setup_start_tag (bd_t *bd)
 {
 	params = (struct tag *)bd->bi_boot_params;
@@ -128,9 +117,7 @@  static void setup_start_tag (bd_t *bd)
 
 	params = tag_next (params);
 }
-#endif
 
-#ifdef CONFIG_SETUP_MEMORY_TAGS
 static void setup_memory_tags(bd_t *bd)
 {
 	int i;
@@ -145,9 +132,7 @@  static void setup_memory_tags(bd_t *bd)
 		params = tag_next (params);
 	}
 }
-#endif
 
-#ifdef CONFIG_CMDLINE_TAG
 static void setup_commandline_tag(bd_t *bd, char *commandline)
 {
 	char *p;
@@ -172,9 +157,7 @@  static void setup_commandline_tag(bd_t *bd, char *commandline)
 
 	params = tag_next (params);
 }
-#endif
 
-#ifdef CONFIG_INITRD_TAG
 static void setup_initrd_tag(bd_t *bd, ulong initrd_start, ulong initrd_end)
 {
 	/* an ATAG_INITRD node tells the kernel where the compressed
@@ -188,14 +171,11 @@  static void setup_initrd_tag(bd_t *bd, ulong initrd_start, ulong initrd_end)
 
 	params = tag_next (params);
 }
-#endif
 
-#ifdef CONFIG_SERIAL_TAG
-void setup_serial_tag(struct tag **tmp)
+static void setup_serial_tag(struct tag **tmp)
 {
 	struct tag *params = *tmp;
 	struct tag_serialnr serialnr;
-	void get_board_serial(struct tag_serialnr *serialnr);
 
 	get_board_serial(&serialnr);
 	params->hdr.tag = ATAG_SERIAL;
@@ -205,13 +185,10 @@  void setup_serial_tag(struct tag **tmp)
 	params = tag_next (params);
 	*tmp = params;
 }
-#endif
 
-#ifdef CONFIG_REVISION_TAG
-void setup_revision_tag(struct tag **in_params)
+static void setup_revision_tag(struct tag **in_params)
 {
 	u32 rev = 0;
-	u32 get_board_rev(void);
 
 	rev = get_board_rev();
 	params->hdr.tag = ATAG_REVISION;
@@ -219,19 +196,12 @@  void setup_revision_tag(struct tag **in_params)
 	params->u.revision.rev = rev;
 	params = tag_next (params);
 }
-#endif
 
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
 static void setup_end_tag(bd_t *bd)
 {
 	params->hdr.tag = ATAG_NONE;
 	params->hdr.size = 0;
 }
-#endif
 
 #ifdef CONFIG_OF_LIBFDT
 static int create_fdt(bootm_headers_t *images)
@@ -275,50 +245,37 @@  __weak void setup_board_tags(struct tag **in_params) {}
 /* Subcommand: PREP */
 static void boot_prep_linux(bootm_headers_t *images)
 {
-#ifdef CONFIG_CMDLINE_TAG
 	char *commandline = getenv("bootargs");
-#endif
 
+	if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) {
 #ifdef CONFIG_OF_LIBFDT
-	if (images->ft_len) {
 		debug("using: FDT\n");
 		if (create_fdt(images)) {
 			printf("FDT creation failed! hanging...");
 			hang();
 		}
-	} else
 #endif
-	{
-#if defined(CONFIG_SETUP_MEMORY_TAGS) || \
-	defined(CONFIG_CMDLINE_TAG) || \
-	defined(CONFIG_INITRD_TAG) || \
-	defined(CONFIG_SERIAL_TAG) || \
-	defined(CONFIG_REVISION_TAG)
+	} else if (BOOTM_ENABLE_TAGS) {
 		debug("using: ATAGS\n");
 		setup_start_tag(gd->bd);
-#ifdef CONFIG_SERIAL_TAG
-		setup_serial_tag(&params);
-#endif
-#ifdef CONFIG_CMDLINE_TAG
-		setup_commandline_tag(gd->bd, commandline);
-#endif
-#ifdef CONFIG_REVISION_TAG
-		setup_revision_tag(&params);
-#endif
-#ifdef CONFIG_SETUP_MEMORY_TAGS
-		setup_memory_tags(gd->bd);
-#endif
-#ifdef CONFIG_INITRD_TAG
-		if (images->rd_start && images->rd_end)
-			setup_initrd_tag(gd->bd, images->rd_start,
-			images->rd_end);
-#endif
+		if (BOOTM_ENABLE_SERIAL_TAG)
+			setup_serial_tag(&params);
+		if (BOOTM_ENABLE_CMDLINE_TAG)
+			setup_commandline_tag(gd->bd, commandline);
+		if (BOOTM_ENABLE_REVISION_TAG)
+			setup_revision_tag(&params);
+		if (BOOTM_ENABLE_MEMORY_TAGS)
+			setup_memory_tags(gd->bd);
+		if (BOOTM_ENABLE_INITRD_TAG) {
+			if (images->rd_start && images->rd_end)
+				setup_initrd_tag(gd->bd, images->rd_start,
+				images->rd_end);
+		}
 		setup_board_tags(&params);
 		setup_end_tag(gd->bd);
-#else /* all tags */
+	} else {
 		printf("FDT and ATAGS support not compiled in - hanging\n");
 		hang();
-#endif /* all tags */
 	}
 }
 
@@ -343,11 +300,9 @@  static void boot_jump_linux(bootm_headers_t *images)
 	bootstage_mark(BOOTSTAGE_ID_RUN_OS);
 	announce_and_cleanup();
 
-#ifdef CONFIG_OF_LIBFDT
-	if (images->ft_len)
+	if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len)
 		r2 = (unsigned long)images->ft_addr;
 	else
-#endif
 		r2 = gd->bd->bi_boot_params;
 
 	kernel_entry(0, machid, r2);