Fix SPL build without CONFIG_SPL_SERIAL_SUPPORT

Attempting to build SPL without CONFIG_SPL_SERIAL_SUPPORT defined fails in assorted ways. This series fixes up those failures.

Green Travis build:

https://travis-ci.org/akiernan/u-boot/builds/367684056

Alex Kiernan (8):
Cleanup CONFIG_SPL_SERIAL_SUPPORT migration
spl: ti: Avoid preloader_console_init if !CONFIG_SPL_SERIAL_SUPPORT
spl: Add dependency on serial to Ymodem
spl: ti: Avoid serial calls when serial support is disabled
spl: Split sprintf, strto* from SPL serial in Kconfig
spl: Disable printf if not required
Consolidate __assert_failed into one implementation
spl: disk: usb: Add dependencies to sprintf/strto*

 arch/arm/mach-omap2/boot-common.c               |  3 ++-
 board/ti/am335x/board.c                         |  2 ++
 common/spl/Kconfig                              |  6 ++++++
 common/spl/spl.c                                |  2 ++
 configs/controlcenterdc_defconfig               |  1 +
 configs/ls1021aiot_sdcard_defconfig             |  1 +
 configs/ls1046aqds_nand_defconfig               |  1 +
 configs/ls1046aqds_sdcard_ifc_defconfig         |  1 +
 configs/ls1046aqds_sdcard_qspi_defconfig        |  1 +
 configs/ls1046ardb_emmc_defconfig               |  1 +
 configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig |  1 +
 configs/ls1046ardb_sdcard_defconfig             |  1 +
 disk/Kconfig                                    |  4 ++++
 drivers/usb/musb-new/Kconfig                    |  4 ++++
 include/configs/controlcenterdc.h               |  1 -
 include/configs/ls1021aiot.h                    |  1 -
 include/configs/ls1046a_common.h                |  2 --
 lib/Kconfig                                     | 22 ++++++++++++++++++++++
 lib/Makefile                                    | 15 ++++++---------
 lib/panic.c                                     | 11 +++++++++++
 lib/tiny-printf.c                               | 22 ++++++++--------------
 lib/vsprintf.c                                  | 12 +++---------
 22 files changed, 78 insertions(+), 37 deletions(-)

2.7.4

CONFIG_SPL_SERIAL_SUPPORT had already been migrated to Kconfig, but existed in some include files; fix those up here.

---

 configs/controlcenterdc_defconfig               | 1 +
 configs/ls1021aiot_sdcard_defconfig             | 1 +
 configs/ls1046aqds_nand_defconfig               | 1 +
 configs/ls1046aqds_sdcard_ifc_defconfig         | 1 +
 configs/ls1046aqds_sdcard_qspi_defconfig        | 1 +
 configs/ls1046ardb_emmc_defconfig               | 1 +
 configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig | 1 +
 configs/ls1046ardb_sdcard_defconfig             | 1 +
 include/configs/controlcenterdc.h               | 1 -
 include/configs/ls1021aiot.h                    | 1 -
 include/configs/ls1046a_common.h                | 2 --
 11 files changed, 8 insertions(+), 4 deletions(-)

index 5fe18c6..92d289e 100644
@@ -4,6 +4,7 @@ CONFIG_SYS_TEXT_BASE=0x00800000
 CONFIG_SPL_GPIO_SUPPORT=y
 CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_TARGET_CONTROLCENTERDC=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI_SUPPORT=y
 CONFIG_SPL=y

index 59592db..a34cb52 100644
@@ -1,6 +1,7 @@
 CONFIG_ARM=y
 CONFIG_TARGET_LS1021AIOT=y
 CONFIG_SYS_TEXT_BASE=0x82000000
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="ls1021a-iot-duart"
 CONFIG_SYS_EXTRA_OPTIONS="RAMBOOT_PBL,SPL_FSL_PBL,SD_BOOT,SD_BOOT_QSPI"

index 1d40a14..3a5c001 100644
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_TARGET_LS1046AQDS=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-qds-duart"
 CONFIG_DISTRO_DEFAULTS=y

index 4d0907c..2c5a9e9 100644
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_TARGET_LS1046AQDS=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-qds-duart"
 CONFIG_DISTRO_DEFAULTS=y

index 8467148..66ddc35 100644
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_TARGET_LS1046AQDS=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-qds-duart"
 CONFIG_DISTRO_DEFAULTS=y

index 3ca8f16..97de7b1 100644
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_TARGET_LS1046ARDB=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-rdb"
 CONFIG_DISTRO_DEFAULTS=y

index ee6320f..dfa0f01 100644
@@ -3,6 +3,7 @@ CONFIG_TARGET_LS1046ARDB=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_SECURE_BOOT=y
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-rdb"
 CONFIG_DISTRO_DEFAULTS=y

index 0fa6fa4..5a33dc6 100644
@@ -2,6 +2,7 @@ CONFIG_ARM=y
 CONFIG_TARGET_LS1046ARDB=y
 CONFIG_SYS_TEXT_BASE=0x82000000
 CONFIG_FSL_LS_PPA=y
+CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
 CONFIG_DEFAULT_DEVICE_TREE="fsl-ls1046a-rdb"
 CONFIG_DISTRO_DEFAULTS=y

index b6d15f6..89e57301 100644
@@ -112,7 +112,6 @@
 
 #define CONFIG_SPL_LIBCOMMON_SUPPORT
 #define CONFIG_SPL_LIBGENERIC_SUPPORT
-#define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_I2C_SUPPORT
 
 #if CONFIG_SPL_BOOT_DEVICE == SPL_BOOT_SPI_NOR_FLASH

index f2a6837..dc1206f 100644
@@ -67,7 +67,6 @@
 #define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
 #define CONFIG_SPL_I2C_SUPPORT
 #define CONFIG_SPL_WATCHDOG_SUPPORT
-#define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_MMC_SUPPORT
 #define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0xe8
 

index b9424e6..2851c84 100644
@@ -67,7 +67,6 @@
 #define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
 #define CONFIG_SPL_WATCHDOG_SUPPORT
 #define CONFIG_SPL_I2C_SUPPORT
-#define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_DRIVERS_MISC_SUPPORT
 
 #define CONFIG_SPL_MMC_SUPPORT

@@ -104,7 +103,6 @@
 #define CONFIG_SPL_ENV_SUPPORT
 #define CONFIG_SPL_WATCHDOG_SUPPORT
 #define CONFIG_SPL_I2C_SUPPORT
-#define CONFIG_SPL_SERIAL_SUPPORT
 #define CONFIG_SPL_MPC8XXX_INIT_DDR_SUPPORT
 
 #define CONFIG_SPL_NAND_SUPPORT

2.7.4

If CONFIG_SPL_SERIAL_SUPPORT is disabled then the build fails because serial_init is undefined. Guard preloader_console_init() appropriately to fix this.

---

 arch/arm/mach-omap2/boot-common.c | 3 ++-
 common/spl/spl.c                  | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

index db68a9d..f9ab5da 100644
@@ -196,9 +196,10 @@ u32 spl_boot_mode(const u32 boot_device)
 
 void spl_board_init(void)
 {
+#ifdef CONFIG_SPL_SERIAL_SUPPORT
 	/* Prepare console output */
 	preloader_console_init();
-
+#endif
 #if defined(CONFIG_SPL_NAND_SUPPORT) || defined(CONFIG_SPL_ONENAND_SUPPORT)
 	gpmc_init();
 #endif

index 61d3071..794dbd0 100644
@@ -471,6 +471,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	jump_to_image_no_args(&spl_image);
 }
 
+#ifdef CONFIG_SPL_SERIAL_SUPPORT
 /*
  * This requires UART clocks to be enabled.  In order for this to work the
  * caller must ensure that the gd pointer is valid.

@@ -491,6 +492,7 @@ void preloader_console_init(void)
 	spl_display_print();
 #endif
 }
+#endif
 
 /**
  * spl_relocate_stack_gd() - Relocate stack ready for board_init_r() execution

2.7.4

Building with Ymodem support requires serial in SPL/TPL, add that dependency here.

---

 common/spl/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

index 4d27565..c5d4b5e 100644
@@ -751,6 +751,7 @@ config SPL_WATCHDOG_SUPPORT
 
 config SPL_YMODEM_SUPPORT
 	bool "Support loading using Ymodem"
+	depends on SPL_SERIAL_SUPPORT
 	help
 	  While loading from serial is slow it can be a useful backup when
 	  there is no other option. The Ymodem protocol provides a reliable

@@ -951,6 +952,7 @@ config TPL_SPI_SUPPORT
 
 config TPL_YMODEM_SUPPORT
 	bool "Support loading using Ymodem"
+	depends on TPL_SERIAL_SUPPORT
 	help
 	  While loading from serial is slow it can be a useful backup when
 	  there is no other option. The Ymodem protocol provides a reliable

2.7.4

If CONFIG_SPL_SERIAL_SUPPORT is not set, then the build will fail:

board/ti/am335x/built-in.o: In function `spl_start_uboot':
board/ti/am335x/board.c:247: undefined reference to serial_tstc' board/ti/am335x/board.c:247: undefined reference to serial_getc'

Avoid the calls to the serial functions in that case.

---

 board/ti/am335x/board.c | 2 ++
 1 file changed, 2 insertions(+)

index c33bf58..896b9b6 100644
@@ -243,9 +243,11 @@ static struct emif_regs ddr3_icev2_emif_reg_data = {
 #ifdef CONFIG_SPL_OS_BOOT
 int spl_start_uboot(void)
 {
+#ifdef CONFIG_SPL_SERIAL_SUPPORT
 	/* break into full u-boot on 'c' */
 	if (serial_tstc() && serial_getc() == 'c')
 		return 1;
+#endif
 
 #ifdef CONFIG_SPL_ENV_SUPPORT
 	env_init();

2.7.4

When SPL serial is disabled, callers who need sprintf or strtoul fail because their inclusion is guarded by CONFIG_SPL_SERIAL_SUPPORT/ CONFIG_TPL_SERIAL_SUPPORT.

Split printf, sprintf and strto into their own entries and then select all of them if SERIAL_SUPPORT is enabled to match the current behaviour.

Include panic.o unconditionally as it can be called from anywhere which uses BUG_ON().

---

 common/spl/Kconfig |  4 ++++
 lib/Kconfig        | 22 ++++++++++++++++++++++
 lib/Makefile       | 15 ++++++---------
 3 files changed, 32 insertions(+), 9 deletions(-)

index c5d4b5e..259f966 100644
@@ -626,6 +626,8 @@ config SPL_SATA_SUPPORT
 
 config SPL_SERIAL_SUPPORT
 	bool "Support serial"
+	select SPL_PRINTF
+	select SPL_STRTO
 	help
 	  Enable support for serial in SPL. This allows use of a serial UART
 	  for displaying messages while SPL is running. It also brings in

@@ -927,6 +929,8 @@ config TPL_RAM_DEVICE
 
 config TPL_SERIAL_SUPPORT
 	bool "Support serial"
+	select TPL_PRINTF
+	select TPL_STRTO
 	help
 	  Enable support for serial in TPL. See SPL_SERIAL_SUPPORT for
 	  details.

index 436b90f..82c8fbc 100644
@@ -27,6 +27,28 @@ config HAVE_PRIVATE_LIBGCC
 config LIB_UUID
 	bool
 
+config SPL_PRINTF
+	bool
+	select SPL_SPRINTF
+	select SPL_STRTO if !USE_TINY_PRINTF
+
+config TPL_PRINTF
+	bool
+	select TPL_SPRINTF
+	select TPL_STRTO if !USE_TINY_PRINTF
+
+config SPL_SPRINTF
+	bool
+
+config TPL_SPRINTF
+	bool
+
+config SPL_STRTO
+	bool
+
+config TPL_STRTO
+	bool
+
 config USE_PRIVATE_LIBGCC
 	bool "Use private libgcc"
 	depends on HAVE_PRIVATE_LIBGCC

index 35da570..13be8f4 100644
@@ -88,22 +88,19 @@ obj-y += time.o
 obj-$(CONFIG_TRACE) += trace.o
 obj-$(CONFIG_LIB_UUID) += uuid.o
 obj-$(CONFIG_LIB_RAND) += rand.o
+obj-y += panic.o
 
-ifdef CONFIG_SPL_BUILD
-ifdef CONFIG_TPL_BUILD
-SERIAL_SUPPORT := $(CONFIG_TPL_SERIAL_SUPPORT)
-else
-SERIAL_SUPPORT := $(CONFIG_SPL_SERIAL_SUPPORT)
-endif
+ifeq ($(CONFIG_$(SPL_TPL_)BUILD),y)
 # SPL U-Boot may use full-printf, tiny-printf or none at all
 ifdef CONFIG_USE_TINY_PRINTF
-obj-$(SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o
+obj-$(CONFIG_$(SPL_TPL_)SPRINTF) += tiny-printf.o
 else
-obj-$(SERIAL_SUPPORT) += vsprintf.o panic.o strto.o strmhz.o
+obj-$(CONFIG_$(SPL_TPL_)SPRINTF) += vsprintf.o strmhz.o
 endif
+obj-$(CONFIG_$(SPL_TPL_)STRTO) += strto.o
 else
 # Main U-Boot always uses the full printf support
-obj-y += vsprintf.o panic.o strto.o strmhz.o
+obj-y += vsprintf.o strto.o strmhz.o
 endif
 
 subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2

2.7.4

Now we have a guard for printf, disable it in the build if it's not selected.

---

 lib/panic.c       |  3 +++
 lib/tiny-printf.c | 13 ++++++++-----
 lib/vsprintf.c    |  4 +++-
 3 files changed, 14 insertions(+), 6 deletions(-)

index e2b8b74..3ca6421 100644
@@ -37,9 +37,12 @@ void panic_str(const char *str)
 
 void panic(const char *fmt, ...)
 {
+#if !(IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)) || \
+	CONFIG_IS_ENABLED(PRINTF)
 	va_list args;
 	va_start(args, fmt);
 	vprintf(fmt, args);
 	va_end(args);
+#endif
 	panic_finish();
 }

index 0b04813..9b97aed 100644
@@ -23,11 +23,6 @@ struct printf_info {
 	void (*putc)(struct printf_info *info, char ch);
 };
 
-static void putc_normal(struct printf_info *info, char ch)
-{
-	putc(ch);
-}
-
 static void out(struct printf_info *info, char c)
 {
 	*info->bf++ = c;

@@ -321,6 +316,13 @@ abort:
 	return 0;
 }
 
+#if !(IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)) || \
+	CONFIG_IS_ENABLED(PRINTF)
+static void putc_normal(struct printf_info *info, char ch)
+{
+	putc(ch);
+}
+
 int vprintf(const char *fmt, va_list va)
 {
 	struct printf_info info;

@@ -343,6 +345,7 @@ int printf(const char *fmt, ...)
 
 	return ret;
 }
+#endif
 
 static void putc_outstr(struct printf_info *info, char ch)
 {

index 5f7a5f1..bb0c573 100644
@@ -783,6 +783,8 @@ int sprintf(char *buf, const char *fmt, ...)
 	return i;
 }
 
+#if !(IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)) || \
+	CONFIG_IS_ENABLED(PRINTF)
 int printf(const char *fmt, ...)
 {
 	va_list args;

@@ -824,7 +826,7 @@ int vprintf(const char *fmt, va_list args)
 	puts(printbuffer);
 	return i;
 }
-
+#endif
 
 void __assert_fail(const char *assertion, const char *file, unsigned line,
 		   const char *function)

2.7.4

Because I don't have a CONFIG_PRINTF, only CONFIG_SPL_PRINTF/CONFIG_TPL_PRINTF, so you'd end up with it disabled.

But very happy to add a CONFIG_PRINTF which is default y and would allow this to be cleaned up.

Alex Kiernan

We had two implementations of __assert_failed which were almost identical, combine them into one.

---

 lib/panic.c       | 8 ++++++++
 lib/tiny-printf.c | 9 ---------
 lib/vsprintf.c    | 8 --------
 3 files changed, 8 insertions(+), 17 deletions(-)

index 3ca6421..328cfae 100644
@@ -46,3 +46,11 @@ void panic(const char *fmt, ...)
 #endif
 	panic_finish();
 }
+
+void __assert_fail(const char *assertion, const char *file, unsigned int line,
+		   const char *function)
+{
+	/* This will not return */
+	panic("%s:%u: %s: Assertion `%s' failed.", file, line, function,
+	      assertion);
+}

index 9b97aed..030aef5 100644
@@ -384,12 +384,3 @@ int snprintf(char *buf, size_t size, const char *fmt, ...)
 
 	return ret;
 }
-
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-		   const char *function)
-{
-	/* This will not return */
-	printf("%s:%u: %s: Assertion `%s' failed.", file, line, function,
-	       assertion);
-	hang();
-}

index bb0c573..9295d5f 100644
@@ -828,14 +828,6 @@ int vprintf(const char *fmt, va_list args)
 }
 #endif
 
-void __assert_fail(const char *assertion, const char *file, unsigned line,
-		   const char *function)
-{
-	/* This will not return */
-	panic("%s:%u: %s: Assertion `%s' failed.", file, line, function,
-	      assertion);
-}
-
 char *simple_itoa(ulong i)
 {
 	/* 21 digits plus null terminator, good for 64-bit or smaller ints */

2.7.4

If SPL serial support is disabled nothing brings in sprintf, snprintf or simple_strtoul:

env/built-in.o: In function `regex_callback':
env/attr.c:128: undefined reference to sprintf' disk/built-in.o: In function blk_get_device_by_str':
disk/part.c:386: undefined reference to simple_strtoul' disk/part.c:395: undefined reference to simple_strtoul' disk/built-in.o: In function `blk_get_device_part_str':
disk/part.c:522: undefined reference to simple_strtoul' disk/built-in.o: In function part_set_generic_name':
disk/part.c:704: undefined reference to sprintf' drivers/built-in.o: In function init_peripheral_ep':
drivers/usb/musb-new/musb_gadget.c:1826: undefined reference to sprintf' drivers/built-in.o: In function musb_core_init':
drivers/usb/musb-new/musb_core.c:1451: undefined reference to `snprintf'

Add those dependencies here.

---

 disk/Kconfig                 | 4 ++++
 drivers/usb/musb-new/Kconfig | 4 ++++
 2 files changed, 8 insertions(+)

index 8af2a95..f9f8416 100644
@@ -4,6 +4,10 @@ menu "Partition Types"
 config PARTITIONS
 	bool "Enable Partition Labels (disklabels) support"
 	default y
+	select SPL_SPRINTF
+	select TPL_SPRINTF
+	select SPL_STRTO
+	select TPL_STRTO
 	help
 	  Partition Labels (disklabels) Supported:
 	  Zero or more of the following:

index ea5bae2..46ae794 100644
@@ -5,12 +5,16 @@ comment "MUSB Controller Driver"
 
 config USB_MUSB_HOST
 	bool "MUSB host mode support"
+	select SPL_SPRINTF
+	select TPL_SPRINTF
 	help
 	  Enables the MUSB USB dual-role controller in host mode.
 
 config USB_MUSB_GADGET
 	bool "MUSB gadget mode support"
 	select USB_GADGET_DUALSPEED
+	select SPL_SPRINTF
+	select TPL_SPRINTF
 	help
 	  Enables the MUSB USB dual-role controller in gadget mode.
 

2.7.4

We've a mix today of "select SPL_..." and "select SPL_... if SPL" - roughly three times as many without the "... if SPL", which was why I picked that style, but looking more closely you do get those symbols then appearing in non-SPL builds.

The guards I have only apply if you are SPL/TPL, but the more I stare at it, the more correct it feels to have them guarded here against SPL/TPL.

Alex Kiernan