[U-Boot] [PATCH v1 0/8] 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(-)

CONFIG_SPL_SERIAL_SUPPORT had already been migrated to Kconfig, but existed in some include files; fix those up here.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
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(-)
diff --git a/configs/controlcenterdc_defconfig b/configs/controlcenterdc_defconfig index 5fe18c6..92d289e 100644 --- a/configs/controlcenterdc_defconfig +++ b/configs/controlcenterdc_defconfig @@ -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 diff --git a/configs/ls1021aiot_sdcard_defconfig b/configs/ls1021aiot_sdcard_defconfig index 59592db..a34cb52 100644 --- a/configs/ls1021aiot_sdcard_defconfig +++ b/configs/ls1021aiot_sdcard_defconfig @@ -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" diff --git a/configs/ls1046aqds_nand_defconfig b/configs/ls1046aqds_nand_defconfig index 1d40a14..3a5c001 100644 --- a/configs/ls1046aqds_nand_defconfig +++ b/configs/ls1046aqds_nand_defconfig @@ -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 diff --git a/configs/ls1046aqds_sdcard_ifc_defconfig b/configs/ls1046aqds_sdcard_ifc_defconfig index 4d0907c..2c5a9e9 100644 --- a/configs/ls1046aqds_sdcard_ifc_defconfig +++ b/configs/ls1046aqds_sdcard_ifc_defconfig @@ -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 diff --git a/configs/ls1046aqds_sdcard_qspi_defconfig b/configs/ls1046aqds_sdcard_qspi_defconfig index 8467148..66ddc35 100644 --- a/configs/ls1046aqds_sdcard_qspi_defconfig +++ b/configs/ls1046aqds_sdcard_qspi_defconfig @@ -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 diff --git a/configs/ls1046ardb_emmc_defconfig b/configs/ls1046ardb_emmc_defconfig index 3ca8f16..97de7b1 100644 --- a/configs/ls1046ardb_emmc_defconfig +++ b/configs/ls1046ardb_emmc_defconfig @@ -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 diff --git a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig index ee6320f..dfa0f01 100644 --- a/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig +++ b/configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig @@ -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 diff --git a/configs/ls1046ardb_sdcard_defconfig b/configs/ls1046ardb_sdcard_defconfig index 0fa6fa4..5a33dc6 100644 --- a/configs/ls1046ardb_sdcard_defconfig +++ b/configs/ls1046ardb_sdcard_defconfig @@ -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 diff --git a/include/configs/controlcenterdc.h b/include/configs/controlcenterdc.h index b6d15f6..89e57301 100644 --- a/include/configs/controlcenterdc.h +++ b/include/configs/controlcenterdc.h @@ -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 diff --git a/include/configs/ls1021aiot.h b/include/configs/ls1021aiot.h index f2a6837..dc1206f 100644 --- a/include/configs/ls1021aiot.h +++ b/include/configs/ls1021aiot.h @@ -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
diff --git a/include/configs/ls1046a_common.h b/include/configs/ls1046a_common.h index b9424e6..2851c84 100644 --- a/include/configs/ls1046a_common.h +++ b/include/configs/ls1046a_common.h @@ -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

If CONFIG_SPL_SERIAL_SUPPORT is disabled then the build fails because serial_init is undefined. Guard preloader_console_init() appropriately to fix this.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
arch/arm/mach-omap2/boot-common.c | 3 ++- common/spl/spl.c | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index db68a9d..f9ab5da 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -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 diff --git a/common/spl/spl.c b/common/spl/spl.c index 61d3071..794dbd0 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -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

Building with Ymodem support requires serial in SPL/TPL, add that dependency here.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
common/spl/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index 4d27565..c5d4b5e 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -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

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.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
board/ti/am335x/board.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/board/ti/am335x/board.c b/board/ti/am335x/board.c index c33bf58..896b9b6 100644 --- a/board/ti/am335x/board.c +++ b/board/ti/am335x/board.c @@ -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();

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().
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
common/spl/Kconfig | 4 ++++ lib/Kconfig | 22 ++++++++++++++++++++++ lib/Makefile | 15 ++++++--------- 3 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index c5d4b5e..259f966 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -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. diff --git a/lib/Kconfig b/lib/Kconfig index 436b90f..82c8fbc 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -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 diff --git a/lib/Makefile b/lib/Makefile index 35da570..13be8f4 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -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

Now we have a guard for printf, disable it in the build if it's not selected.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
lib/panic.c | 3 +++ lib/tiny-printf.c | 13 ++++++++----- lib/vsprintf.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/lib/panic.c b/lib/panic.c index e2b8b74..3ca6421 100644 --- a/lib/panic.c +++ b/lib/panic.c @@ -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(); } diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 0b04813..9b97aed 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -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) { diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 5f7a5f1..bb0c573 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -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)

On 18/04/2018 11:02, Alex Kiernan wrote:
Now we have a guard for printf, disable it in the build if it's not selected.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com
lib/panic.c | 3 +++ lib/tiny-printf.c | 13 ++++++++----- lib/vsprintf.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/lib/panic.c b/lib/panic.c index e2b8b74..3ca6421 100644 --- a/lib/panic.c +++ b/lib/panic.c @@ -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)
Why not use only CONFIG_IS_ENABLED(PRINTF) ?
va_list args; va_start(args, fmt); vprintf(fmt, args); va_end(args); +#endif panic_finish(); } diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 0b04813..9b97aed 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -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)
ditto.
+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) { diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 5f7a5f1..bb0c573 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -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)
ditto.
JJ
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)

On Wed, Apr 18, 2018 at 10:27 AM, Jean-Jacques Hiblot jjhiblot@ti.com wrote:
On 18/04/2018 11:02, Alex Kiernan wrote:
Now we have a guard for printf, disable it in the build if it's not selected.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com
lib/panic.c | 3 +++ lib/tiny-printf.c | 13 ++++++++----- lib/vsprintf.c | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/lib/panic.c b/lib/panic.c index e2b8b74..3ca6421 100644 --- a/lib/panic.c +++ b/lib/panic.c @@ -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)
Why not use only CONFIG_IS_ENABLED(PRINTF) ?
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.

We had two implementations of __assert_failed which were almost identical, combine them into one.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
lib/panic.c | 8 ++++++++ lib/tiny-printf.c | 9 --------- lib/vsprintf.c | 8 -------- 3 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/lib/panic.c b/lib/panic.c index 3ca6421..328cfae 100644 --- a/lib/panic.c +++ b/lib/panic.c @@ -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); +} diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c index 9b97aed..030aef5 100644 --- a/lib/tiny-printf.c +++ b/lib/tiny-printf.c @@ -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(); -} diff --git a/lib/vsprintf.c b/lib/vsprintf.c index bb0c573..9295d5f 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -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 */

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.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com ---
disk/Kconfig | 4 ++++ drivers/usb/musb-new/Kconfig | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/disk/Kconfig b/disk/Kconfig index 8af2a95..f9f8416 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -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: diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig index ea5bae2..46ae794 100644 --- a/drivers/usb/musb-new/Kconfig +++ b/drivers/usb/musb-new/Kconfig @@ -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.

On Wed, Apr 18, 2018 at 09:02:23AM +0000, Alex Kiernan wrote:
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.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com
disk/Kconfig | 4 ++++ drivers/usb/musb-new/Kconfig | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/disk/Kconfig b/disk/Kconfig index 8af2a95..f9f8416 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -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:
diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig index ea5bae2..46ae794 100644 --- a/drivers/usb/musb-new/Kconfig +++ b/drivers/usb/musb-new/Kconfig @@ -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
I guess that's only true if the support is enabled in the SPL / TPL in the first place?
Maxime

On Wed, Apr 18, 2018 at 10:11 AM, Maxime Ripard maxime.ripard@bootlin.com wrote:
On Wed, Apr 18, 2018 at 09:02:23AM +0000, Alex Kiernan wrote:
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.
Signed-off-by: Alex Kiernan alex.kiernan@gmail.com
disk/Kconfig | 4 ++++ drivers/usb/musb-new/Kconfig | 4 ++++ 2 files changed, 8 insertions(+)
diff --git a/disk/Kconfig b/disk/Kconfig index 8af2a95..f9f8416 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -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:
diff --git a/drivers/usb/musb-new/Kconfig b/drivers/usb/musb-new/Kconfig index ea5bae2..46ae794 100644 --- a/drivers/usb/musb-new/Kconfig +++ b/drivers/usb/musb-new/Kconfig @@ -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
I guess that's only true if the support is enabled in the SPL / TPL in the first place?
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.
participants (3)
-
Alex Kiernan
-
Jean-Jacques Hiblot
-
Maxime Ripard