[PATCH 00/11] Reduce usage of timestamp macros
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
arch/arm/mach-rockchip/px30-board-tpl.c | 1 - arch/arm/mach-rockchip/tpl.c | 4 ++++ arch/m68k/cpu/mcf5227x/start.S | 6 ------ arch/m68k/cpu/mcf523x/start.S | 6 ------ arch/m68k/cpu/mcf52x2/start.S | 6 ------ arch/m68k/cpu/mcf530x/start.S | 8 ------- arch/m68k/cpu/mcf532x/start.S | 6 ------ arch/m68k/cpu/mcf5445x/start.S | 7 ------- arch/nios2/cpu/start.S | 1 - arch/powerpc/cpu/mpc83xx/start.S | 10 +++------ arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++----- arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++---- arch/x86/cpu/quark/mrc.c | 4 +--- arch/x86/lib/acpi_table.c | 1 - board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c | 1 - board/cssi/MCR3000/u-boot.lds | 2 ++ board/ge/b1x5v2/b1x5v2.c | 2 +- board/ge/bx50v3/bx50v3.c | 2 +- board/ge/mx53ppd/mx53ppd.c | 2 +- board/l+g/vinco/vinco.c | 1 - board/renesas/grpeach/lowlevel_init.S | 1 - .../work_92105/work_92105_display.c | 1 + cmd/version.c | 7 ++++++- common/main.c | 2 +- common/spl/spl.c | 4 ++++ doc/develop/version.rst | 21 +++++++++++-------- drivers/rtc/emul_rtc.c | 2 +- drivers/usb/gadget/f_rockusb.c | 1 - drivers/video/cfb_console.c | 3 +-- include/configs/bcmstb.h | 1 - include/version.h | 8 ------- include/version_string.h | 8 +++++++ lib/display_options.c | 2 +- lib/efi_loader/efi_tcg2.c | 7 +++---- net/cdp.c | 3 --- test/print_ut.c | 2 +- 40 files changed, 75 insertions(+), 102 deletions(-) create mode 100644 include/version_string.h
Signed-off-by: Pali Rohár pali@kernel.org --- arch/m68k/cpu/mcf5445x/start.S | 1 - net/cdp.c | 3 --- 2 files changed, 4 deletions(-)
diff --git a/arch/m68k/cpu/mcf5445x/start.S b/arch/m68k/cpu/mcf5445x/start.S index 7007d78c83fa..40c497436570 100644 --- a/arch/m68k/cpu/mcf5445x/start.S +++ b/arch/m68k/cpu/mcf5445x/start.S @@ -10,7 +10,6 @@ #include <common.h> #include <asm-offsets.h> #include <config.h> -#include <timestamp.h> #include "version.h" #include <asm/cache.h>
diff --git a/net/cdp.c b/net/cdp.c index fac020468194..a8f890e75226 100644 --- a/net/cdp.c +++ b/net/cdp.c @@ -11,9 +11,6 @@
#include <common.h> #include <net.h> -#if defined(CONFIG_CDP_VERSION) -#include <timestamp.h> -#endif
#include "cdp.h"
On Mon, Aug 02, 2021 at 03:18:28PM +0200, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
On Mon, Aug 02, 2021 at 03:18:28PM +0200, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
Signed-off-by: Pali Rohár pali@kernel.org --- arch/arm/mach-rockchip/px30-board-tpl.c | 1 - arch/nios2/cpu/start.S | 1 - arch/x86/lib/acpi_table.c | 1 - board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c | 1 - board/l+g/vinco/vinco.c | 1 - board/renesas/grpeach/lowlevel_init.S | 1 - drivers/usb/gadget/f_rockusb.c | 1 - include/configs/bcmstb.h | 1 - 8 files changed, 8 deletions(-)
diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c b/arch/arm/mach-rockchip/px30-board-tpl.c index 085e65062011..637a5e1b18b8 100644 --- a/arch/arm/mach-rockchip/px30-board-tpl.c +++ b/arch/arm/mach-rockchip/px30-board-tpl.c @@ -9,7 +9,6 @@ #include <init.h> #include <ram.h> #include <spl.h> -#include <version.h> #include <asm/io.h> #include <asm/arch-rockchip/bootrom.h> #include <asm/arch-rockchip/sdram_px30.h> diff --git a/arch/nios2/cpu/start.S b/arch/nios2/cpu/start.S index f5ad184e8d0b..acb8ca686ed9 100644 --- a/arch/nios2/cpu/start.S +++ b/arch/nios2/cpu/start.S @@ -6,7 +6,6 @@
#include <asm-offsets.h> #include <config.h> -#include <version.h>
/* * icache and dcache configuration used only for start.S. diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c index 5ec31301d0b8..3f847711e2be 100644 --- a/arch/x86/lib/acpi_table.c +++ b/arch/x86/lib/acpi_table.c @@ -16,7 +16,6 @@ #include <dm/uclass-internal.h> #include <mapmem.h> #include <serial.h> -#include <version.h> #include <acpi/acpigen.h> #include <acpi/acpi_device.h> #include <acpi/acpi_table.h> diff --git a/board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c b/board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c index a6937e7d5243..2a2439c53ae4 100644 --- a/board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c +++ b/board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c @@ -10,7 +10,6 @@ #include <i2c.h> #include <init.h> #include <nand.h> -#include <version.h> #include <asm/global_data.h> #include <asm/io.h> #include <asm/arch/at91_common.h> diff --git a/board/l+g/vinco/vinco.c b/board/l+g/vinco/vinco.c index f221f05261ae..9c4c5fdc4a53 100644 --- a/board/l+g/vinco/vinco.c +++ b/board/l+g/vinco/vinco.c @@ -30,7 +30,6 @@ #include <netdev.h> #include <nand.h> #include <spi.h> -#include <version.h>
DECLARE_GLOBAL_DATA_PTR;
diff --git a/board/renesas/grpeach/lowlevel_init.S b/board/renesas/grpeach/lowlevel_init.S index 9a66dfa6c693..b83c4e868670 100644 --- a/board/renesas/grpeach/lowlevel_init.S +++ b/board/renesas/grpeach/lowlevel_init.S @@ -4,7 +4,6 @@ * Copyright (C) 2017 Chris Brandt */ #include <config.h> -#include <version.h> #include <asm/macro.h>
/* Watchdog Registers */ diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c index bd846ce9a77b..98a7ffa2a752 100644 --- a/drivers/usb/gadget/f_rockusb.c +++ b/drivers/usb/gadget/f_rockusb.c @@ -17,7 +17,6 @@ #include <linux/usb/gadget.h> #include <linux/usb/composite.h> #include <linux/compiler.h> -#include <version.h> #include <g_dnl.h> #include <asm/arch-rockchip/f_rockusb.h>
diff --git a/include/configs/bcmstb.h b/include/configs/bcmstb.h index 2660d18f35a9..81c2bbe46f22 100644 --- a/include/configs/bcmstb.h +++ b/include/configs/bcmstb.h @@ -10,7 +10,6 @@ #ifndef __BCMSTB_H #define __BCMSTB_H
-#include "version.h" #include <linux/sizes.h>
#ifndef __ASSEMBLY__
On Mon, Aug 02, 2021 at 03:18:29PM +0200, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
On Mon, Aug 02, 2021 at 03:18:29PM +0200, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
Macro U_BOOT_VERSION_STRING is already stored in variable version_string. So use directly this variable instead of storing U_BOOT_VERSION_STRING into temporary variable.
Signed-off-by: Pali Rohár pali@kernel.org --- lib/efi_loader/efi_tcg2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 1319a8b37868..14353ae71c9b 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1275,11 +1275,10 @@ free_pool: static efi_status_t efi_append_scrtm_version(struct udevice *dev) { struct tpml_digest_values digest_list; - u8 ver[] = U_BOOT_VERSION_STRING; const int pcr_index = 0; efi_status_t ret;
- ret = tcg2_create_digest(ver, sizeof(ver), &digest_list); + ret = tcg2_create_digest(version_string, strlen(version_string) + 1, &digest_list); if (ret != EFI_SUCCESS) goto out;
@@ -1288,7 +1287,7 @@ static efi_status_t efi_append_scrtm_version(struct udevice *dev) goto out;
ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list, - sizeof(ver), ver); + strlen(version_string) + 1, version_string);
out: return ret;
+cc Ilias
On 8/2/21 3:18 PM, Pali Rohár wrote:
Macro U_BOOT_VERSION_STRING is already stored in variable version_string. So use directly this variable instead of storing U_BOOT_VERSION_STRING into temporary variable.
Signed-off-by: Pali Rohár pali@kernel.org
lib/efi_loader/efi_tcg2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 1319a8b37868..14353ae71c9b 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1275,11 +1275,10 @@ free_pool: static efi_status_t efi_append_scrtm_version(struct udevice *dev) { struct tpml_digest_values digest_list;
u8 ver[] = U_BOOT_VERSION_STRING; const int pcr_index = 0; efi_status_t ret;
ret = tcg2_create_digest(ver, sizeof(ver), &digest_list);
- ret = tcg2_create_digest(version_string, strlen(version_string) + 1, &digest_list); if (ret != EFI_SUCCESS) goto out;
@@ -1288,7 +1287,7 @@ static efi_status_t efi_append_scrtm_version(struct udevice *dev) goto out;
ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list,
sizeof(ver), ver);
strlen(version_string) + 1, version_string);out: return ret;
Hi Pali,
On Mon, 2 Aug 2021 at 23:04, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
+cc Ilias
On 8/2/21 3:18 PM, Pali Rohár wrote:
Macro U_BOOT_VERSION_STRING is already stored in variable version_string. So use directly this variable instead of storing U_BOOT_VERSION_STRING into temporary variable.
Looks ok Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Signed-off-by: Pali Rohár pali@kernel.org
lib/efi_loader/efi_tcg2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 1319a8b37868..14353ae71c9b 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1275,11 +1275,10 @@ free_pool: static efi_status_t efi_append_scrtm_version(struct udevice *dev) { struct tpml_digest_values digest_list;
u8 ver[] = U_BOOT_VERSION_STRING; const int pcr_index = 0; efi_status_t ret;ret = tcg2_create_digest(ver, sizeof(ver), &digest_list);
ret = tcg2_create_digest(version_string, strlen(version_string) + 1, &digest_list); if (ret != EFI_SUCCESS) goto out;@@ -1288,7 +1287,7 @@ static efi_status_t efi_append_scrtm_version(struct udevice *dev) goto out;
ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list,
sizeof(ver), ver);
strlen(version_string) + 1, version_string);out: return ret;
On Mon, Aug 02, 2021 at 03:18:30PM +0200, Pali Rohár wrote:
Macro U_BOOT_VERSION_STRING is already stored in variable version_string. So use directly this variable instead of storing U_BOOT_VERSION_STRING into temporary variable.
Signed-off-by: Pali Rohár pali@kernel.org Acked-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Applied to u-boot/next, thanks!
More C files do not use compile time timestamp macros and do not have to be recompiled every time when SOURCE_DATE_EPOCH changes.
This patch moves version_string[] from version.h to version_string.h and updates other C files which only needs version_string[] string to include version_string.h instead of version.h. After applying this patch these files are not recompiled every time when SOURCE_DATE_EPOCH changes.
Signed-off-by: Pali Rohár pali@kernel.org --- board/ge/b1x5v2/b1x5v2.c | 2 +- board/ge/bx50v3/bx50v3.c | 2 +- board/ge/mx53ppd/mx53ppd.c | 2 +- cmd/version.c | 1 + common/main.c | 2 +- drivers/video/cfb_console.c | 3 +-- include/version.h | 3 --- include/version_string.h | 8 ++++++++ lib/display_options.c | 2 +- lib/efi_loader/efi_tcg2.c | 2 +- test/print_ut.c | 2 +- 11 files changed, 17 insertions(+), 12 deletions(-) create mode 100644 include/version_string.h
diff --git a/board/ge/b1x5v2/b1x5v2.c b/board/ge/b1x5v2/b1x5v2.c index de4cb0d5afaa..a2cbd1512e92 100644 --- a/board/ge/b1x5v2/b1x5v2.c +++ b/board/ge/b1x5v2/b1x5v2.c @@ -30,7 +30,7 @@ #include <panel.h> #include <rtc.h> #include <spi_flash.h> -#include <version.h> +#include <version_string.h>
#include "../common/vpd_reader.h"
diff --git a/board/ge/bx50v3/bx50v3.c b/board/ge/bx50v3/bx50v3.c index 7fcebba02600..ed700f4e1da5 100644 --- a/board/ge/bx50v3/bx50v3.c +++ b/board/ge/bx50v3/bx50v3.c @@ -34,7 +34,7 @@ #include <power/pmic.h> #include <input.h> #include <pwm.h> -#include <version.h> +#include <version_string.h> #include <stdlib.h> #include <dm/root.h> #include "../common/ge_rtc.h" diff --git a/board/ge/mx53ppd/mx53ppd.c b/board/ge/mx53ppd/mx53ppd.c index 6174125e728a..f540fe1f3668 100644 --- a/board/ge/mx53ppd/mx53ppd.c +++ b/board/ge/mx53ppd/mx53ppd.c @@ -33,7 +33,7 @@ #include <fsl_pmic.h> #include <linux/fb.h> #include <ipu_pixfmt.h> -#include <version.h> +#include <version_string.h> #include <watchdog.h> #include "ppd_gpio.h" #include <stdlib.h> diff --git a/cmd/version.c b/cmd/version.c index 685b458ce262..965ac2e2144d 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -7,6 +7,7 @@ #include <common.h> #include <command.h> #include <version.h> +#include <version_string.h> #include <linux/compiler.h> #ifdef CONFIG_SYS_COREBOOT #include <asm/cb_sysinfo.h> diff --git a/common/main.c b/common/main.c index ae5bcdb32f8b..3f5214fd44b8 100644 --- a/common/main.c +++ b/common/main.c @@ -15,7 +15,7 @@ #include <env.h> #include <init.h> #include <net.h> -#include <version.h> +#include <version_string.h> #include <efi_loader.h>
static void run_preboot_environment_command(void) diff --git a/drivers/video/cfb_console.c b/drivers/video/cfb_console.c index 1f491a48d6a7..a19c96755681 100644 --- a/drivers/video/cfb_console.c +++ b/drivers/video/cfb_console.c @@ -71,7 +71,7 @@ #include <fdtdec.h> #include <gzip.h> #include <log.h> -#include <version.h> +#include <version_string.h> #include <malloc.h> #include <video.h> #include <asm/global_data.h> @@ -108,7 +108,6 @@ * Console device */
-#include <version.h> #include <linux/types.h> #include <stdio_dev.h> #include <video_font.h> diff --git a/include/version.h b/include/version.h index 2d24451569d5..0a3b29adb89a 100644 --- a/include/version.h +++ b/include/version.h @@ -16,7 +16,4 @@ #define U_BOOT_VERSION_STRING U_BOOT_VERSION " (" U_BOOT_DATE " - " \ U_BOOT_TIME " " U_BOOT_TZ ")" CONFIG_IDENT_STRING
-#ifndef __ASSEMBLY__ -extern const char version_string[]; -#endif /* __ASSEMBLY__ */ #endif /* __VERSION_H__ */ diff --git a/include/version_string.h b/include/version_string.h new file mode 100644 index 000000000000..a89a6e43705e --- /dev/null +++ b/include/version_string.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#ifndef __VERSION_STRING_H__ +#define __VERSION_STRING_H__ + +extern const char version_string[]; + +#endif /* __VERSION_STRING_H__ */ diff --git a/lib/display_options.c b/lib/display_options.c index c08a87e31629..e096ef8f4e45 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -8,7 +8,7 @@ #include <compiler.h> #include <console.h> #include <div64.h> -#include <version.h> +#include <version_string.h> #include <linux/ctype.h> #include <asm/io.h>
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 14353ae71c9b..b845e5531280 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -14,7 +14,7 @@ #include <efi_tcg2.h> #include <log.h> #include <malloc.h> -#include <version.h> +#include <version_string.h> #include <tpm-v2.h> #include <u-boot/hash-checksum.h> #include <u-boot/sha1.h> diff --git a/test/print_ut.c b/test/print_ut.c index e2bcfbef0078..11d8580e55c5 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -9,7 +9,7 @@ #include <display_options.h> #include <log.h> #include <mapmem.h> -#include <version.h> +#include <version_string.h> #include <test/suites.h> #include <test/test.h> #include <test/ut.h>
On Mon, Aug 02, 2021 at 03:18:31PM +0200, Pali Rohár wrote:
More C files do not use compile time timestamp macros and do not have to be recompiled every time when SOURCE_DATE_EPOCH changes.
This patch moves version_string[] from version.h to version_string.h and updates other C files which only needs version_string[] string to include version_string.h instead of version.h. After applying this patch these files are not recompiled every time when SOURCE_DATE_EPOCH changes.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
On Mon, Aug 02, 2021 at 03:18:31PM +0200, Pali Rohár wrote:
More C files do not use compile time timestamp macros and do not have to be recompiled every time when SOURCE_DATE_EPOCH changes.
This patch moves version_string[] from version.h to version_string.h and updates other C files which only needs version_string[] string to include version_string.h instead of version.h. After applying this patch these files are not recompiled every time when SOURCE_DATE_EPOCH changes.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
There is no need to overload version_string at the end of start.S files. Common implementation of version_string should be fine.
Signed-off-by: Pali Rohár pali@kernel.org --- arch/m68k/cpu/mcf5227x/start.S | 6 ------ arch/m68k/cpu/mcf523x/start.S | 6 ------ arch/m68k/cpu/mcf52x2/start.S | 6 ------ arch/m68k/cpu/mcf530x/start.S | 8 -------- arch/m68k/cpu/mcf532x/start.S | 6 ------ arch/m68k/cpu/mcf5445x/start.S | 6 ------ 6 files changed, 38 deletions(-)
diff --git a/arch/m68k/cpu/mcf5227x/start.S b/arch/m68k/cpu/mcf5227x/start.S index 86c93ba3faff..c49f59e9aabf 100644 --- a/arch/m68k/cpu/mcf5227x/start.S +++ b/arch/m68k/cpu/mcf5227x/start.S @@ -6,7 +6,6 @@
#include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -488,8 +487,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION_STRING, "\0" -.align 4 diff --git a/arch/m68k/cpu/mcf523x/start.S b/arch/m68k/cpu/mcf523x/start.S index 8c5a16495523..060e0eecbb56 100644 --- a/arch/m68k/cpu/mcf523x/start.S +++ b/arch/m68k/cpu/mcf523x/start.S @@ -6,7 +6,6 @@
#include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -252,8 +251,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION_STRING, "\0" -.align 4 diff --git a/arch/m68k/cpu/mcf52x2/start.S b/arch/m68k/cpu/mcf52x2/start.S index 747a518f6cd5..bf1cdc76f152 100644 --- a/arch/m68k/cpu/mcf52x2/start.S +++ b/arch/m68k/cpu/mcf52x2/start.S @@ -6,7 +6,6 @@
#include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -334,8 +333,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION_STRING, "\0" -.align 4 diff --git a/arch/m68k/cpu/mcf530x/start.S b/arch/m68k/cpu/mcf530x/start.S index 32356d875ec1..ae4df00b9eb0 100644 --- a/arch/m68k/cpu/mcf530x/start.S +++ b/arch/m68k/cpu/mcf530x/start.S @@ -6,7 +6,6 @@
#include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -257,10 +256,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION -.ascii " (", U_BOOT_DATE, " - ", U_BOOT_TIME, ")" -.ascii CONFIG_IDENT_STRING, "\0" -.align 4 diff --git a/arch/m68k/cpu/mcf532x/start.S b/arch/m68k/cpu/mcf532x/start.S index e2d7c72ceec6..8114e20803ba 100644 --- a/arch/m68k/cpu/mcf532x/start.S +++ b/arch/m68k/cpu/mcf532x/start.S @@ -9,7 +9,6 @@
#include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -267,8 +266,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION_STRING, "\0" -.align 4 diff --git a/arch/m68k/cpu/mcf5445x/start.S b/arch/m68k/cpu/mcf5445x/start.S index 40c497436570..54e98a471be7 100644 --- a/arch/m68k/cpu/mcf5445x/start.S +++ b/arch/m68k/cpu/mcf5445x/start.S @@ -10,7 +10,6 @@ #include <common.h> #include <asm-offsets.h> #include <config.h> -#include "version.h" #include <asm/cache.h>
#define _START _start @@ -608,8 +607,3 @@ _int_handler: RESTORE_ALL
/******************************************************************************/ - -.globl version_string -version_string: -.ascii U_BOOT_VERSION_STRING, "\0" -.align 4
On Mon, Aug 02, 2021 at 03:18:32PM +0200, Pali Rohár wrote:
There is no need to overload version_string at the end of start.S files. Common implementation of version_string should be fine.
Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot/next, thanks!
This would allow to reference this variable easily from linker scripts.
Signed-off-by: Pali Rohár pali@kernel.org --- cmd/version.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/version.c b/cmd/version.c index 965ac2e2144d..ad632fe4fb7a 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -13,7 +13,7 @@ #include <asm/cb_sysinfo.h> #endif
-const char __weak version_string[] = U_BOOT_VERSION_STRING; +const char __weak version_string[] __section(".text_version_string") = U_BOOT_VERSION_STRING;
static int do_version(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
On Mon, Aug 02, 2021 at 03:18:33PM +0200, Pali Rohár wrote:
This would allow to reference this variable easily from linker scripts.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
Update the linker script macros to know that we need to include the .text_version_string section now as well.
Signed-off-by: Tom Rini trini@konsulko.com --- arch/xtensa/include/asm/ldscript.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/xtensa/include/asm/ldscript.h b/arch/xtensa/include/asm/ldscript.h index 08f5d0135ed0..84c496e09f1b 100644 --- a/arch/xtensa/include/asm/ldscript.h +++ b/arch/xtensa/include/asm/ldscript.h @@ -72,6 +72,7 @@ { \ _text_start = ABSOLUTE(.); \ *(.literal .text) \ + *(.literal .text_version_string) \ *(.literal.* .text.* .stub) \ *(.gnu.warning .gnu.linkonce.literal.*) \ *(.gnu.linkonce.t.*.literal .gnu.linkonce.t.*) \
On Thu, Sep 16, 2021 at 01:30:21PM -0400, Tom Rini wrote:
*(.literal .text) \
*(.literal .text_version_string) \
Isn't ".litteral" a duplication? Even if I'm pretty sure it will not cause any difference in the generated binary I would remove it.
Francesco
On Thu, Sep 16, 2021 at 09:38:19PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 01:30:21PM -0400, Tom Rini wrote:
*(.literal .text) \
*(.literal .text_version_string) \Isn't ".litteral" a duplication? Even if I'm pretty sure it will not cause any difference in the generated binary I would remove it.
Honestly, I don't know xtensa. We also don't have qemu support for it currently, and I'm a bit worried in general about the state of the platform (it's on my TODO list to poke some people now). So, I hesitate to make any change that's not basically copy/paste of the existing lines.
On Thu, Sep 16, 2021 at 03:42:20PM -0400, Tom Rini wrote:
On Thu, Sep 16, 2021 at 09:38:19PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 01:30:21PM -0400, Tom Rini wrote:
*(.literal .text) \
*(.literal .text_version_string) \Isn't ".litteral" a duplication? Even if I'm pretty sure it will not cause any difference in the generated binary I would remove it.
Honestly, I don't know xtensa. We also don't have qemu support for it currently, and I'm a bit worried in general about the state of the platform (it's on my TODO list to poke some people now). So, I hesitate to make any change that's not basically copy/paste of the existing lines.
I have no experience on xtensa either, but this is just the section name, and you are just telling to put ".literal" there 2 times.
Francesco
On Thu, Sep 16, 2021 at 09:50:35PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 03:42:20PM -0400, Tom Rini wrote:
On Thu, Sep 16, 2021 at 09:38:19PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 01:30:21PM -0400, Tom Rini wrote:
*(.literal .text) \
*(.literal .text_version_string) \Isn't ".litteral" a duplication? Even if I'm pretty sure it will not cause any difference in the generated binary I would remove it.
Honestly, I don't know xtensa. We also don't have qemu support for it currently, and I'm a bit worried in general about the state of the platform (it's on my TODO list to poke some people now). So, I hesitate to make any change that's not basically copy/paste of the existing lines.
I have no experience on xtensa either, but this is just the section name, and you are just telling to put ".literal" there 2 times.
I don't know. Looking at the resulting linker script (and with your suggestion): .text (((0x00002000 - 0x00002000) + ((128 << 20))) - 0x00040000) : AT(((LOADADDR(.DoubleExceptionVector.text) + SIZEOF(.DoubleExceptionVector.text) + 4 -1)) & ~(4 -1)) { _text_start = ABSOLUTE(.); *(.literal .text) *(.text_version_string) *(.literal.* .text.* .stub) *(.gnu.warning .gnu.linkonce.literal.*) *(.gnu.linkonce.t.*.literal .gnu.linkonce.t.*) *(.fini.literal) *(.fini) *(.gnu.version) _text_end = ABSOLUTE(.); }
So there's "literal" scattered everywhere. If it doesn't matter, it reads more consistently to me to toss literal in one more time. But I emailed around and we'll see if xtensa stays around, it's also our oldest toolchain and virtually untouched since submission :(
On Thu, Sep 16, 2021 at 04:13:31PM -0400, Tom Rini wrote:
On Thu, Sep 16, 2021 at 09:50:35PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 03:42:20PM -0400, Tom Rini wrote:
On Thu, Sep 16, 2021 at 09:38:19PM +0200, Francesco Dolcini wrote:
On Thu, Sep 16, 2021 at 01:30:21PM -0400, Tom Rini wrote:
*(.literal .text) \
*(.literal .text_version_string) \Isn't ".litteral" a duplication? Even if I'm pretty sure it will not cause any difference in the generated binary I would remove it.
Honestly, I don't know xtensa. We also don't have qemu support for it currently, and I'm a bit worried in general about the state of the platform (it's on my TODO list to poke some people now). So, I hesitate to make any change that's not basically copy/paste of the existing lines.
I have no experience on xtensa either, but this is just the section name, and you are just telling to put ".literal" there 2 times.
I don't know. Looking at the resulting linker script (and with your suggestion): .text (((0x00002000 - 0x00002000) + ((128 << 20))) - 0x00040000) : AT(((LOADADDR(.DoubleExceptionVector.text) + SIZEOF(.DoubleExceptionVector.text) + 4 -1)) & ~(4 -1)) { _text_start = ABSOLUTE(.); *(.literal .text) *(.text_version_string) *(.literal.* .text.* .stub) *(.gnu.warning .gnu.linkonce.literal.*) *(.gnu.linkonce.t.*.literal .gnu.linkonce.t.*) *(.fini.literal) *(.fini) *(.gnu.version) _text_end = ABSOLUTE(.); }
So there's "literal" scattered everywhere. If it doesn't matter, it reads more consistently to me to toss literal in one more time.
Heheh, ".literal" is present only once here, ".literal.*" is matching something else, likewise all the others that are scathered all around. This looks fine to me.
But I emailed around and we'll see if xtensa stays around, it's also our oldest toolchain and virtually untouched since submission :(
My small comment is just about the linker script format, I guess some other folk should be able to confirm what I wrote. Unfortunately it will not solve the general problem with xtensa ...
Francesco
On Thu, Sep 16, 2021 at 12:42 PM Tom Rini trini@konsulko.com wrote:
We also don't have qemu support for it
I'm curious what happened to it and what I should do to update it?
xtensa is still supported in the QEMU and AFAIK nothing has changed about it: neither building nor invocation.
On Thu, Sep 16, 2021 at 10:21:29PM -0700, Max Filippov wrote:
On Thu, Sep 16, 2021 at 12:42 PM Tom Rini trini@konsulko.com wrote:
We also don't have qemu support for it
I'm curious what happened to it and what I should do to update it?
xtensa is still supported in the QEMU and AFAIK nothing has changed about it: neither building nor invocation.
OK, yes, I was just wrong there. It's still there, it's part of Azure and GitLab CI, tested every time. Sorry for the confusion.
On Thu, Sep 16, 2021 at 10:30 AM Tom Rini trini@konsulko.com wrote:
Update the linker script macros to know that we need to include the .text_version_string section now as well.
Signed-off-by: Tom Rini trini@konsulko.com
arch/xtensa/include/asm/ldscript.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/xtensa/include/asm/ldscript.h b/arch/xtensa/include/asm/ldscript.h index 08f5d0135ed0..84c496e09f1b 100644 --- a/arch/xtensa/include/asm/ldscript.h +++ b/arch/xtensa/include/asm/ldscript.h @@ -72,6 +72,7 @@ { \ _text_start = ABSOLUTE(.); \ *(.literal .text) \
*(.literal .text_version_string) \
This does not belong to .text, as far as I understand it's rodata and so it should go with rodata, probably like this: ---8<--- diff --git a/arch/xtensa/include/asm/ldscript.h b/arch/xtensa/include/asm/ldscript.h index 08f5d0135ed0..e03fcffdd6f1 100644 --- a/arch/xtensa/include/asm/ldscript.h +++ b/arch/xtensa/include/asm/ldscript.h @@ -87,6 +87,7 @@ _rodata_start = ABSOLUTE(.); \ *(.rodata) \ *(.rodata.*) \ + *(.text_version_string) \ *(.dtb.init.rodata) \ *(.gnu.linkonce.r.*) \ *(.rodata1) \ ---8<---
On Thu, Sep 16, 2021 at 10:14:03PM -0700, Max Filippov wrote:
On Thu, Sep 16, 2021 at 10:30 AM Tom Rini trini@konsulko.com wrote:
Update the linker script macros to know that we need to include the .text_version_string section now as well.
Signed-off-by: Tom Rini trini@konsulko.com
arch/xtensa/include/asm/ldscript.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/xtensa/include/asm/ldscript.h b/arch/xtensa/include/asm/ldscript.h index 08f5d0135ed0..84c496e09f1b 100644 --- a/arch/xtensa/include/asm/ldscript.h +++ b/arch/xtensa/include/asm/ldscript.h @@ -72,6 +72,7 @@ { \ _text_start = ABSOLUTE(.); \ *(.literal .text) \
*(.literal .text_version_string) \This does not belong to .text, as far as I understand it's rodata and so it should go with rodata, probably like this: ---8<--- diff --git a/arch/xtensa/include/asm/ldscript.h b/arch/xtensa/include/asm/ldscript.h index 08f5d0135ed0..e03fcffdd6f1 100644 --- a/arch/xtensa/include/asm/ldscript.h +++ b/arch/xtensa/include/asm/ldscript.h @@ -87,6 +87,7 @@ _rodata_start = ABSOLUTE(.); \ *(.rodata) \ *(.rodata.*) \
*(.text_version_string) \ *(.dtb.init.rodata) \ *(.gnu.linkonce.r.*) \ *(.rodata1) \
So this is in context with: https://patchwork.ozlabs.org/project/uboot/list/?series=256258 and specifically: https://patchwork.ozlabs.org/project/uboot/patch/20210802131838.21097-7-pali...
Now that said, I think the whole point of that has been removed with: https://patchwork.ozlabs.org/project/uboot/patch/20210916195648.9424-1-trini... and no longer needing to reference it from the linker script for PowerPC. So maybe I can just drop this whole part.
On Fri, Sep 17, 2021 at 5:04 AM Tom Rini trini@konsulko.com wrote:
So this is in context with: https://patchwork.ozlabs.org/project/uboot/list/?series=256258 and specifically: https://patchwork.ozlabs.org/project/uboot/patch/20210802131838.21097-7-pali...
Yes, I've found that series and the purpose of this patch puzzled me, given the absence of actual references to the newly added section in the series.
Now that said, I think the whole point of that has been removed with: https://patchwork.ozlabs.org/project/uboot/patch/20210916195648.9424-1-trini... and no longer needing to reference it from the linker script for PowerPC.
Aha, that explains the purpose of that section, thanks.
So maybe I can just drop this whole part.
It is unknown by version string is placed at specific position on these powerpc mpc platforms. But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
Signed-off-by: Pali Rohár pali@kernel.org --- arch/powerpc/cpu/mpc83xx/start.S | 10 +++------- arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------ arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++----- board/cssi/MCR3000/u-boot.lds | 2 ++ 8 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 9da22ce486a9..cc1d80368ab4 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -13,7 +13,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc83xx.h> -#include <version.h>
#define CONFIG_83XX 1 /* needed for Linux kernel header files*/
@@ -76,7 +75,7 @@ * times so the processor can fetch it out of flash whether the flash * is 8, 16, 32, or 64 bits wide (hardware trickery). */ - .text + .text_pre #define _HRCW_TABLE_ENTRY(w) \ .fill 8,1,(((w)>>24)&0xff); \ .fill 8,1,(((w)>>16)&0xff); \ @@ -92,12 +91,9 @@ */ .long 0x27051956 /* U-Boot Magic Number */
- .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0" - - .align 2 +/* U-Boot version string is filled at this place by linker script */
+ .text .globl enable_addr_trans enable_addr_trans: /* enable address translation */ diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds index d10f528da4c4..309082bc3df5 100644 --- a/arch/powerpc/cpu/mpc83xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds @@ -10,6 +10,9 @@ SECTIONS /* Read-only sections, merged into text segment: */ .text : { + arch/powerpc/cpu/mpc83xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(2); arch/powerpc/cpu/mpc83xx/start.o (.text*) *(.text*) . = ALIGN(16); diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S index f41e82ad189f..f89072d38d0d 100644 --- a/arch/powerpc/cpu/mpc85xx/start.S +++ b/arch/powerpc/cpu/mpc85xx/start.S @@ -14,7 +14,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc85xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -1134,15 +1133,14 @@ switch_as: blr #endif
- .text + .text_pre .globl _start _start: .long 0x27051956 /* U-BOOT Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
- .align 4 +/* U-Boot version string is filled at this place by linker script */ + + .text .globl _start_cont _start_cont: /* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/ diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds index 75b0285e4e51..6e48223380cb 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds @@ -25,6 +25,10 @@ SECTIONS .interp : { *(.interp) } .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .; diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds index 27a5fe6306a3..2312cd47f11d 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds @@ -20,6 +20,10 @@ SECTIONS { . = IMAGE_TEXT_BASE; .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } _etext = .; diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds index 22bbac51aa33..40d181ef2caa 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds @@ -31,6 +31,10 @@ SECTIONS .interp : { *(.interp) } .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .; diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..d7003c76ca05 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -60,12 +59,12 @@ * r3 - 1st arg to board_init(): IMMP pointer * r4 - 2nd arg to board_init(): boot flag */ - .text + .text_pre .long 0x27051956 /* U-Boot Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
+/* U-Boot version string is filled at this place by linker script */ + + .text . = EXC_OFF_SYS_RESET .globl _start _start: diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds index 70aef3241c8e..9b2ead29b4a5 100644 --- a/board/cssi/MCR3000/u-boot.lds +++ b/board/cssi/MCR3000/u-boot.lds @@ -16,6 +16,8 @@ SECTIONS . = + SIZEOF_HEADERS; .text : { + arch/powerpc/cpu/mpc8xx/start.o (.text_pre) + *(.text_version_string) arch/powerpc/cpu/mpc8xx/start.o (.text) arch/powerpc/cpu/mpc8xx/traps.o (.text*) arch/powerpc/lib/built-in.o (.text*)
It is unknown why version string is placed at specific position on these powerpc mpc platforms. But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
Signed-off-by: Pali Rohár pali@kernel.org
--- Changes in v2: * Put explicit ".section" keyword before declaring ".text_pre" section as some gcc versions cannot recognize specifying custom section without it. * Tested compilation for: $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin and checked that u-boot.bin header is same with and without this patch --- arch/powerpc/cpu/mpc83xx/start.S | 10 +++------- arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------ arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++----- board/cssi/MCR3000/u-boot.lds | 2 ++ 8 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 9da22ce486a9..87747f2b2275 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -13,7 +13,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc83xx.h> -#include <version.h>
#define CONFIG_83XX 1 /* needed for Linux kernel header files*/
@@ -76,7 +75,7 @@ * times so the processor can fetch it out of flash whether the flash * is 8, 16, 32, or 64 bits wide (hardware trickery). */ - .text + .section .text_pre #define _HRCW_TABLE_ENTRY(w) \ .fill 8,1,(((w)>>24)&0xff); \ .fill 8,1,(((w)>>16)&0xff); \ @@ -92,12 +91,9 @@ */ .long 0x27051956 /* U-Boot Magic Number */
- .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0" - - .align 2 +/* U-Boot version string is filled at this place by linker script */
+ .text .globl enable_addr_trans enable_addr_trans: /* enable address translation */ diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds index d10f528da4c4..309082bc3df5 100644 --- a/arch/powerpc/cpu/mpc83xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds @@ -10,6 +10,9 @@ SECTIONS /* Read-only sections, merged into text segment: */ .text : { + arch/powerpc/cpu/mpc83xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(2); arch/powerpc/cpu/mpc83xx/start.o (.text*) *(.text*) . = ALIGN(16); diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S index f41e82ad189f..5ba26d3c449b 100644 --- a/arch/powerpc/cpu/mpc85xx/start.S +++ b/arch/powerpc/cpu/mpc85xx/start.S @@ -14,7 +14,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc85xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -1134,15 +1133,14 @@ switch_as: blr #endif
- .text + .section .text_pre .globl _start _start: .long 0x27051956 /* U-BOOT Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
- .align 4 +/* U-Boot version string is filled at this place by linker script */ + + .text .globl _start_cont _start_cont: /* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/ diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds index 75b0285e4e51..6e48223380cb 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds @@ -25,6 +25,10 @@ SECTIONS .interp : { *(.interp) } .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .; diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds index 27a5fe6306a3..2312cd47f11d 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds @@ -20,6 +20,10 @@ SECTIONS { . = IMAGE_TEXT_BASE; .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } _etext = .; diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds index 22bbac51aa33..40d181ef2caa 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds @@ -31,6 +31,10 @@ SECTIONS .interp : { *(.interp) } .text : { + arch/powerpc/cpu/mpc85xx/start.o (.text_pre) + *(.text_version_string) + . = ALIGN(4); + arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .; diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..09628a0d60f0 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -60,12 +59,12 @@ * r3 - 1st arg to board_init(): IMMP pointer * r4 - 2nd arg to board_init(): boot flag */ - .text + .section .text_pre .long 0x27051956 /* U-Boot Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
+/* U-Boot version string is filled at this place by linker script */ + + .text . = EXC_OFF_SYS_RESET .globl _start _start: diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds index 70aef3241c8e..9b2ead29b4a5 100644 --- a/board/cssi/MCR3000/u-boot.lds +++ b/board/cssi/MCR3000/u-boot.lds @@ -16,6 +16,8 @@ SECTIONS . = + SIZEOF_HEADERS; .text : { + arch/powerpc/cpu/mpc8xx/start.o (.text_pre) + *(.text_version_string) arch/powerpc/cpu/mpc8xx/start.o (.text) arch/powerpc/cpu/mpc8xx/traps.o (.text*) arch/powerpc/lib/built-in.o (.text*)
Christophe, could you please review this change? According to your commit https://source.denx.de/u-boot/u-boot/-/commit/907208c452999427cb2f4345030804... mpc8xx code will have to be maintained until at least 2027.
On Sunday 08 August 2021 13:20:38 Pali Rohár wrote:
It is unknown why version string is placed at specific position on these powerpc mpc platforms.
Also, could you look at this, why version string is at specific location of u-boot.bin header?
But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
Signed-off-by: Pali Rohár pali@kernel.org
Changes in v2:
- Put explicit ".section" keyword before declaring ".text_pre" section as some gcc versions cannot recognize specifying custom section without it.
- Tested compilation for: $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin and checked that u-boot.bin header is same with and without this patch
arch/powerpc/cpu/mpc83xx/start.S | 10 +++------- arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------ arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++----- board/cssi/MCR3000/u-boot.lds | 2 ++ 8 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 9da22ce486a9..87747f2b2275 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -13,7 +13,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc83xx.h> -#include <version.h>
#define CONFIG_83XX 1 /* needed for Linux kernel header files*/
@@ -76,7 +75,7 @@
- times so the processor can fetch it out of flash whether the flash
- is 8, 16, 32, or 64 bits wide (hardware trickery).
*/
- .text
- .section .text_pre
#define _HRCW_TABLE_ENTRY(w) \ .fill 8,1,(((w)>>24)&0xff); \ .fill 8,1,(((w)>>16)&0xff); \ @@ -92,12 +91,9 @@ */ .long 0x27051956 /* U-Boot Magic Number */
- .globl version_string
-version_string:
- .ascii U_BOOT_VERSION_STRING, "\0"
- .align 2
+/* U-Boot version string is filled at this place by linker script */
- .text .globl enable_addr_trans
enable_addr_trans: /* enable address translation */ diff --git a/arch/powerpc/cpu/mpc83xx/u-boot.lds b/arch/powerpc/cpu/mpc83xx/u-boot.lds index d10f528da4c4..309082bc3df5 100644 --- a/arch/powerpc/cpu/mpc83xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc83xx/u-boot.lds @@ -10,6 +10,9 @@ SECTIONS /* Read-only sections, merged into text segment: */ .text : {
- arch/powerpc/cpu/mpc83xx/start.o (.text_pre)
- *(.text_version_string)
- . = ALIGN(2); arch/powerpc/cpu/mpc83xx/start.o (.text*) *(.text*) . = ALIGN(16);
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S index f41e82ad189f..5ba26d3c449b 100644 --- a/arch/powerpc/cpu/mpc85xx/start.S +++ b/arch/powerpc/cpu/mpc85xx/start.S @@ -14,7 +14,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc85xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -1134,15 +1133,14 @@ switch_as: blr #endif
- .text
- .section .text_pre .globl _start
_start: .long 0x27051956 /* U-BOOT Magic Number */
- .globl version_string
-version_string:
.ascii U_BOOT_VERSION_STRING, "\0"
.align 4
+/* U-Boot version string is filled at this place by linker script */
- .text .globl _start_cont
_start_cont: /* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/ diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds index 75b0285e4e51..6e48223380cb 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-nand.lds @@ -25,6 +25,10 @@ SECTIONS .interp : { *(.interp) } .text : {
- arch/powerpc/cpu/mpc85xx/start.o (.text_pre)
- *(.text_version_string)
- . = ALIGN(4);
- arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .;
diff --git a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds index 27a5fe6306a3..2312cd47f11d 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot-spl.lds @@ -20,6 +20,10 @@ SECTIONS { . = IMAGE_TEXT_BASE; .text : {
arch/powerpc/cpu/mpc85xx/start.o (.text_pre)*(.text_version_string). = ALIGN(4);arch/powerpc/cpu/mpc85xx/start.o (.text*)diff --git a/arch/powerpc/cpu/mpc85xx/u-boot.lds b/arch/powerpc/cpu/mpc85xx/u-boot.lds index 22bbac51aa33..40d181ef2caa 100644 --- a/arch/powerpc/cpu/mpc85xx/u-boot.lds +++ b/arch/powerpc/cpu/mpc85xx/u-boot.lds @@ -31,6 +31,10 @@ SECTIONS .interp : { *(.interp) } .text : {
- arch/powerpc/cpu/mpc85xx/start.o (.text_pre)
- *(.text_version_string)
- . = ALIGN(4);
- arch/powerpc/cpu/mpc85xx/start.o (.text*) *(.text*) } :text _etext = .;
diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..09628a0d60f0 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -60,12 +59,12 @@
- r3 - 1st arg to board_init(): IMMP pointer
- r4 - 2nd arg to board_init(): boot flag
*/
- .text
- .section .text_pre .long 0x27051956 /* U-Boot Magic Number */
- .globl version_string
-version_string:
- .ascii U_BOOT_VERSION_STRING, "\0"
+/* U-Boot version string is filled at this place by linker script */
- .text . = EXC_OFF_SYS_RESET .globl _start
_start: diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds index 70aef3241c8e..9b2ead29b4a5 100644 --- a/board/cssi/MCR3000/u-boot.lds +++ b/board/cssi/MCR3000/u-boot.lds @@ -16,6 +16,8 @@ SECTIONS . = + SIZEOF_HEADERS; .text : {
arch/powerpc/cpu/mpc8xx/start.o (.text_pre)*(.text_version_string)-- 2.20.1
Le 08/08/2021 à 13:36, Pali Rohár a écrit :
Christophe, could you please review this change? According to your commit https://source.denx.de/u-boot/u-boot/-/commit/907208c452999427cb2f4345030804... mpc8xx code will have to be maintained until at least 2027.
On Sunday 08 August 2021 13:20:38 Pali Rohár wrote:
It is unknown why version string is placed at specific position on these powerpc mpc platforms.
Also, could you look at this, why version string is at specific location of u-boot.bin header?
I have no idea. AFAIKS it's been like that since the first version in git in 2002. Maybe Wolfgang has the answer ?
Christophe
Dear Christophe,
In message 75ae39aa-3762-6b90-c39a-7595fc417870@csgroup.eu you wrote:
Also, could you look at this, why version string is at specific location of u-boot.bin header?
I have no idea. AFAIKS it's been like that since the first version in git in 2002. Maybe Wolfgang has the answer ?
The Power Architecture was the very first where PPCBoot / U-Boot were running, more than 20 years ago. At that time, resources were tight. Systems would typically boot from a small NOR flash. Typical configurations came with 4 MB NOR / 16 MB RAM or such.
One problem was that you needed always at least one (or better two, with redundancy) full sectors of the flash for the environment, even though this was typically only a few hundred bytes long. Many NOR flash chips of that time did not have uniform sector sizes; instead, they would come with mixed sector sizes.
For example, there were NOR flash chips which came with sector sizes of 8, 4, 4, 16, 32, 32, 32, 32, ... kB.
Here you would naturally want to use the two small 4 kB sectors for the environment, but this results in the need to have the environment storage "embedded" within the U-Boot image. This resulted in the need to come up with special linker scripts. And of course you would try to use the first 8 kB sector as good as possible, which was achieved by used manually optimized linker scripts, collecting functions or other objects such that the wasted gap at the end of the first sector was minimal - usually less than 100 bytes were wasted.
As part of such optimizations, it might have made sense to put the version string there, too - it has a fixed size of a few ten bytes, so it is a good candidate to fill the gap.
Today, such boot sector NOR flashes are no longer found in new designs, nor does anybody care to keep U-Boot small and tidy. As a result, knowledge about such optimizations is disappearing.
Hope this helps.
Best regards,
Wolfgang Denk
Le 08/08/2021 à 13:20, Pali Rohár a écrit :
It is unknown why version string is placed at specific position on these powerpc mpc platforms. But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
What's the benefit of this change ? I see more insertions than deletions, it doesn't remove any redundancy either it seems. And it provides a strange look to u-boot.lds
I think we should probably investigate the reason for the version at that specific place if other architectures don't do the same.
Signed-off-by: Pali Rohár pali@kernel.org
Changes in v2:
- Put explicit ".section" keyword before declaring ".text_pre" section as some gcc versions cannot recognize specifying custom section without it.
- Tested compilation for: $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin and checked that u-boot.bin header is same with and without this patch
arch/powerpc/cpu/mpc83xx/start.S | 10 +++------- arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------ arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++----- board/cssi/MCR3000/u-boot.lds | 2 ++ 8 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..09628a0d60f0 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -60,12 +59,12 @@
- r3 - 1st arg to board_init(): IMMP pointer
- r4 - 2nd arg to board_init(): boot flag
*/
- .text
- .section .text_pre .long 0x27051956 /* U-Boot Magic Number */
Instead of creating a new artificial section, maybe you can add the magic number in front of the version string directly in u-boot.lds ?
- .globl version_string
-version_string:
- .ascii U_BOOT_VERSION_STRING, "\0"
+/* U-Boot version string is filled at this place by linker script */
- .text . = EXC_OFF_SYS_RESET .globl _start _start:
diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds index 70aef3241c8e..9b2ead29b4a5 100644 --- a/board/cssi/MCR3000/u-boot.lds +++ b/board/cssi/MCR3000/u-boot.lds @@ -16,6 +16,8 @@ SECTIONS . = + SIZEOF_HEADERS; .text : {
arch/powerpc/cpu/mpc8xx/start.o (.text_pre)*(.text_version_string)
On Tuesday 24 August 2021 07:28:07 Christophe Leroy wrote:
Le 08/08/2021 à 13:20, Pali Rohár a écrit :
It is unknown why version string is placed at specific position on these powerpc mpc platforms. But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
What's the benefit of this change ?
Removal of the _weak_ property of version_string symbol to cleanup U-Boot code, which allows U-Boot build system on second build run to recompile only files which were really changed.
I see more insertions than deletions, it doesn't remove any redundancy either it seems. And it provides a strange look to u-boot.lds
It is only to ensure that version_string is defined only at one place in U-Boot sources. And for this kind of things it is required to use linker hacks.
I think we should probably investigate the reason for the version at that specific place if other architectures don't do the same.
I agree. This is really the only platform which has version_string at specific place.
Signed-off-by: Pali Rohár pali@kernel.org
Changes in v2:
- Put explicit ".section" keyword before declaring ".text_pre" section as some gcc versions cannot recognize specifying custom section without it.
- Tested compilation for: $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin and checked that u-boot.bin header is same with and without this patch
arch/powerpc/cpu/mpc83xx/start.S | 10 +++------- arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++------ arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++----- board/cssi/MCR3000/u-boot.lds | 2 ++ 8 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..09628a0d60f0 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h> #include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -60,12 +59,12 @@
- r3 - 1st arg to board_init(): IMMP pointer
- r4 - 2nd arg to board_init(): boot flag
*/
- .text
- .section .text_pre .long 0x27051956 /* U-Boot Magic Number */
Instead of creating a new artificial section, maybe you can add the magic number in front of the version string directly in u-boot.lds ?
Well, this patch series is about version_string. I do not want to touch other parts, like magic numbers in this patch series as it is not related to version string in build system.
- .globl version_string
-version_string:
- .ascii U_BOOT_VERSION_STRING, "\0"
+/* U-Boot version string is filled at this place by linker script */
- .text . = EXC_OFF_SYS_RESET .globl _start _start:
diff --git a/board/cssi/MCR3000/u-boot.lds b/board/cssi/MCR3000/u-boot.lds index 70aef3241c8e..9b2ead29b4a5 100644 --- a/board/cssi/MCR3000/u-boot.lds +++ b/board/cssi/MCR3000/u-boot.lds @@ -16,6 +16,8 @@ SECTIONS . = + SIZEOF_HEADERS; .text : {
arch/powerpc/cpu/mpc8xx/start.o (.text_pre)*(.text_version_string)
On Sun, Aug 08, 2021 at 01:20:38PM +0200, Pali Rohár wrote:
It is unknown why version string is placed at specific position on these powerpc mpc platforms. But there is no need to overload version_string symbol. Just use common definition of version_string and modify linker script to put it at "correct place".
Signed-off-by: Pali Rohár pali@kernel.org
Changes in v2:
- Put explicit ".section" keyword before declaring ".text_pre" section as some gcc versions cannot recognize specifying custom section without it.
- Tested compilation for: $ make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin and checked that u-boot.bin header is same with and without this patch
Please note that qemu-ppce500 still fails to build, now with a different error: https://source.denx.de/u-boot/u-boot/-/jobs/323027 which I am investigating how to resolve.
As explained by Wolfgang, historically PowerPC would do a number of things to hand-optimize placement of the binary on NOR flash in order to maximize utilization of very scarce resources. These days, we simply aren't optimizing our binary layout for NOR flash placement and it's quite likely this wasn't working as intended. Furthermore, this level of optimization makes it difficult to have version_string be a global, instead of a weak and overridden value, and so make more progress on reproducible builds, which is a current concern.
Move to having PowerPC no longer store version_string in the early part of text so that it might be part of the first page of NOR and instead use the same declaration everyone else does.
Link: https://lore.kernel.org/r/96716.1629798400@gemini.denx.de/ Signed-off-by: Tom Rini trini@konsulko.com --- arch/powerpc/cpu/mpc83xx/start.S | 7 ------- arch/powerpc/cpu/mpc85xx/start.S | 5 ----- arch/powerpc/cpu/mpc8xx/start.S | 4 ---- 3 files changed, 16 deletions(-)
diff --git a/arch/powerpc/cpu/mpc83xx/start.S b/arch/powerpc/cpu/mpc83xx/start.S index 9da22ce486a9..c4953df4a271 100644 --- a/arch/powerpc/cpu/mpc83xx/start.S +++ b/arch/powerpc/cpu/mpc83xx/start.S @@ -13,7 +13,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc83xx.h> -#include <version.h>
#define CONFIG_83XX 1 /* needed for Linux kernel header files*/
@@ -92,12 +91,6 @@ */ .long 0x27051956 /* U-Boot Magic Number */
- .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0" - - .align 2 - .globl enable_addr_trans enable_addr_trans: /* enable address translation */ diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S index f41e82ad189f..aca31b24c079 100644 --- a/arch/powerpc/cpu/mpc85xx/start.S +++ b/arch/powerpc/cpu/mpc85xx/start.S @@ -14,7 +14,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc85xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -1138,11 +1137,7 @@ switch_as: .globl _start _start: .long 0x27051956 /* U-BOOT Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
- .align 4 .globl _start_cont _start_cont: /* Setup the stack in initial RAM,could be L2-as-SRAM or L1 dcache*/ diff --git a/arch/powerpc/cpu/mpc8xx/start.S b/arch/powerpc/cpu/mpc8xx/start.S index ed735cdee005..0ebb7b33a8bc 100644 --- a/arch/powerpc/cpu/mpc8xx/start.S +++ b/arch/powerpc/cpu/mpc8xx/start.S @@ -23,7 +23,6 @@ #include <asm-offsets.h> #include <config.h> #include <mpc8xx.h> -#include <version.h>
#include <ppc_asm.tmpl> #include <ppc_defs.h> @@ -62,9 +61,6 @@ */ .text .long 0x27051956 /* U-Boot Magic Number */ - .globl version_string -version_string: - .ascii U_BOOT_VERSION_STRING, "\0"
. = EXC_OFF_SYS_RESET .globl _start
On Thu, Sep 16, 2021 at 03:56:48PM -0400, Tom Rini wrote:
As explained by Wolfgang, historically PowerPC would do a number of things to hand-optimize placement of the binary on NOR flash in order to maximize utilization of very scarce resources. These days, we simply aren't optimizing our binary layout for NOR flash placement and it's quite likely this wasn't working as intended. Furthermore, this level of optimization makes it difficult to have version_string be a global, instead of a weak and overridden value, and so make more progress on reproducible builds, which is a current concern.
Move to having PowerPC no longer store version_string in the early part of text so that it might be part of the first page of NOR and instead use the same declaration everyone else does.
Link: https://lore.kernel.org/r/96716.1629798400@gemini.denx.de/ Signed-off-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
There is no platform which needs to overload version_string[] variable, so remove weak symbol mark.
Signed-off-by: Pali Rohár pali@kernel.org --- cmd/version.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/version.c b/cmd/version.c index ad632fe4fb7a..38a26552a148 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -13,7 +13,7 @@ #include <asm/cb_sysinfo.h> #endif
-const char __weak version_string[] __section(".text_version_string") = U_BOOT_VERSION_STRING; +const char version_string[] __section(".text_version_string") = U_BOOT_VERSION_STRING;
static int do_version(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
On Mon, Aug 02, 2021 at 03:18:35PM +0200, Pali Rohár wrote:
There is no platform which needs to overload version_string[] variable, so remove weak symbol mark.
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
On Mon, Aug 02, 2021 at 03:18:35PM +0200, Pali Rohár wrote:
There is no platform which needs to overload version_string[] variable, so remove weak symbol mark.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
U_BOOT_DATE and U_BOOT_TIME are updated on every run of make command. Therefore mrc.c file is recompiled every time when running make which means that whole U-Boot binary is recompiled on every run of make command.
Simplify it and do not recompile U-Boot binary on every run of make command by not depending on macros U_BOOT_DATE and U_BOOT_TIME.
Signed-off-by: Pali Rohár pali@kernel.org --- arch/x86/cpu/quark/mrc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/cpu/quark/mrc.c b/arch/x86/cpu/quark/mrc.c index 3e8c0bc28c5f..ce3c2b8ab426 100644 --- a/arch/x86/cpu/quark/mrc.c +++ b/arch/x86/cpu/quark/mrc.c @@ -33,7 +33,6 @@ */
#include <common.h> -#include <version.h> #include <asm/arch/mrc.h> #include <asm/arch/msg_port.h> #include "mrc_util.h" @@ -191,8 +190,7 @@ void mrc_init(struct mrc_params *mrc_params) { ENTERFN();
- DPF(D_INFO, "MRC Version %04x %s %s\n", MRC_VERSION, - U_BOOT_DATE, U_BOOT_TIME); + DPF(D_INFO, "MRC Version %04x\n", MRC_VERSION);
/* Set up the data structures used by mrc_mem_init() */ mrc_adjust_params(mrc_params);
On Mon, 2 Aug 2021 at 07:19, Pali Rohár pali@kernel.org wrote:
U_BOOT_DATE and U_BOOT_TIME are updated on every run of make command. Therefore mrc.c file is recompiled every time when running make which means that whole U-Boot binary is recompiled on every run of make command.
Simplify it and do not recompile U-Boot binary on every run of make command by not depending on macros U_BOOT_DATE and U_BOOT_TIME.
Signed-off-by: Pali Rohár pali@kernel.org
arch/x86/cpu/quark/mrc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Aug 2, 2021 at 9:19 PM Pali Rohár pali@kernel.org wrote:
U_BOOT_DATE and U_BOOT_TIME are updated on every run of make command. Therefore mrc.c file is recompiled every time when running make which means that whole U-Boot binary is recompiled on every run of make command.
Simplify it and do not recompile U-Boot binary on every run of make command by not depending on macros U_BOOT_DATE and U_BOOT_TIME.
Signed-off-by: Pali Rohár pali@kernel.org
arch/x86/cpu/quark/mrc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Mon, Aug 02, 2021 at 03:18:36PM +0200, Pali Rohár wrote:
U_BOOT_DATE and U_BOOT_TIME are updated on every run of make command. Therefore mrc.c file is recompiled every time when running make which means that whole U-Boot binary is recompiled on every run of make command.
Simplify it and do not recompile U-Boot binary on every run of make command by not depending on macros U_BOOT_DATE and U_BOOT_TIME.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Applied to u-boot/next, thanks!
Version string is available in global variable char version_string[]. Macro U_BOOT_VERSION_STRING is not used by any other file, so remove it completely from version.h. Other files were already converted to use variable version_string[].
Signed-off-by: Pali Rohár pali@kernel.org --- cmd/version.c | 3 +++ doc/develop/version.rst | 21 ++++++++++++--------- include/version.h | 3 --- 3 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/cmd/version.c b/cmd/version.c index 38a26552a148..42eb85b75bb7 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -13,6 +13,9 @@ #include <asm/cb_sysinfo.h> #endif
+#define U_BOOT_VERSION_STRING U_BOOT_VERSION " (" U_BOOT_DATE " - " \ + U_BOOT_TIME " " U_BOOT_TZ ")" CONFIG_IDENT_STRING + const char version_string[] __section(".text_version_string") = U_BOOT_VERSION_STRING;
static int do_version(struct cmd_tbl *cmdtp, int flag, int argc, diff --git a/doc/develop/version.rst b/doc/develop/version.rst index 3f2b07cd2261..5c9046aa17aa 100644 --- a/doc/develop/version.rst +++ b/doc/develop/version.rst @@ -60,15 +60,6 @@ The following are available:
This is used as part of the banner string when U-Boot starts.
- U_BOOT_VERSION_STRING (string #define) - U_BOOT_VERSION followed by build-time information - and CONFIG_IDENT_STRING. - - Examples:: - - U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700) - U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring - U_BOOT_VERSION_NUM (integer #define) Release year, e.g. 2021 for release 2021.01. Note this is an integer, not a string. @@ -77,6 +68,18 @@ The following are available: Patch number, e.g. 1 for release 2020.01. Note this is an integer, not a string.
+Human readable U-Boot version string is available in header file +include/version_string.h in following variable: + + version_string (const char[]) + U_BOOT_VERSION followed by build-time information + and CONFIG_IDENT_STRING. + + Examples:: + + U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700) + U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring + Build date/time is also included. See the generated file include/generated/timestamp_autogenerated.h for the available fields. For example:: diff --git a/include/version.h b/include/version.h index 0a3b29adb89a..8ee07134fd2f 100644 --- a/include/version.h +++ b/include/version.h @@ -13,7 +13,4 @@ #include "generated/version_autogenerated.h" #endif
-#define U_BOOT_VERSION_STRING U_BOOT_VERSION " (" U_BOOT_DATE " - " \ - U_BOOT_TIME " " U_BOOT_TZ ")" CONFIG_IDENT_STRING - #endif /* __VERSION_H__ */
On Mon, Aug 02, 2021 at 03:18:37PM +0200, Pali Rohár wrote:
Version string is available in global variable char version_string[]. Macro U_BOOT_VERSION_STRING is not used by any other file, so remove it completely from version.h. Other files were already converted to use variable version_string[].
Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Tom Rini trini@konsulko.com
On Mon, Aug 02, 2021 at 03:18:37PM +0200, Pali Rohár wrote:
Version string is available in global variable char version_string[]. Macro U_BOOT_VERSION_STRING is not used by any other file, so remove it completely from version.h. Other files were already converted to use variable version_string[].
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org --- arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c index cc908e1b0e81..56c65149ebe5 100644 --- a/arch/arm/mach-rockchip/tpl.c +++ b/arch/arm/mach-rockchip/tpl.c @@ -16,6 +16,10 @@ #include <asm/arch-rockchip/bootrom.h> #include <linux/bitops.h>
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL_SUPPORT) && defined(CONFIG_TPL_BANNER_PRINT) +#include <timestamp.h> +#endif + #define TIMER_LOAD_COUNT_L 0x00 #define TIMER_LOAD_COUNT_H 0x04 #define TIMER_CONTROL_REG 0x10 diff --git a/board/work-microwave/work_92105/work_92105_display.c b/board/work-microwave/work_92105/work_92105_display.c index fecbbbdb584a..6add68518247 100644 --- a/board/work-microwave/work_92105/work_92105_display.c +++ b/board/work-microwave/work_92105/work_92105_display.c @@ -20,6 +20,7 @@ #include <env.h> #include <spi.h> #include <i2c.h> +#include <timestamp.h> #include <version.h> #include <vsprintf.h> #include <linux/delay.h> diff --git a/cmd/version.c b/cmd/version.c index 42eb85b75bb7..11a5d5d9f150 100644 --- a/cmd/version.c +++ b/cmd/version.c @@ -6,6 +6,7 @@
#include <common.h> #include <command.h> +#include <timestamp.h> #include <version.h> #include <version_string.h> #include <linux/compiler.h> diff --git a/common/spl/spl.c b/common/spl/spl.c index d55d3c284851..3c04b75ab8e4 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -34,6 +34,10 @@ #include <bootcount.h> #include <wdt.h>
+#if defined(CONFIG_SPL_SERIAL_SUPPORT) && CONFIG_IS_ENABLED(BANNER_PRINT) +#include <timestamp.h> +#endif + DECLARE_GLOBAL_DATA_PTR;
#ifndef CONFIG_SYS_UBOOT_START diff --git a/drivers/rtc/emul_rtc.c b/drivers/rtc/emul_rtc.c index 8f0e1ab5ac63..6f47d82522ba 100644 --- a/drivers/rtc/emul_rtc.c +++ b/drivers/rtc/emul_rtc.c @@ -9,8 +9,8 @@ #include <div64.h> #include <dm.h> #include <env.h> -#include <generated/timestamp_autogenerated.h> #include <rtc.h> +#include <timestamp.h>
/** * struct emul_rtc - private data for emulated RTC driver diff --git a/include/version.h b/include/version.h index 8ee07134fd2f..5955b21e8904 100644 --- a/include/version.h +++ b/include/version.h @@ -7,8 +7,6 @@ #ifndef __VERSION_H__ #define __VERSION_H__
-#include <timestamp.h> - #ifndef DO_DEPS_ONLY #include "generated/version_autogenerated.h" #endif
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right? It could be confusing otherwise, as people cannot 'update' the banner without making a trivial change.
Regards, SImon
On Monday 02 August 2021 13:21:58 Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
This is current behavior and these my patches do not change it. Patches just smartly moves the source of this timestamp (from macros to global variable; so source files do not have to be recompiled when external global variable changes -- as opposite of macros).
It could be confusing otherwise, as people cannot 'update' the banner without making a trivial change.
IIRC linux kernel does not change this date 'banner' when nothing was changed. So maybe it is, maybe it is not confusing...
On Mon, Aug 02, 2021 at 09:42:23PM +0200, Pali Rohár wrote:
On Monday 02 August 2021 13:21:58 Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
This is current behavior and these my patches do not change it. Patches just smartly moves the source of this timestamp (from macros to global variable; so source files do not have to be recompiled when external global variable changes -- as opposite of macros).
It could be confusing otherwise, as people cannot 'update' the banner without making a trivial change.
IIRC linux kernel does not change this date 'banner' when nothing was changed. So maybe it is, maybe it is not confusing...
I think that if nothing changes the banner not changing is the right behavior.
Reviewed-by: Tom Rini trini@konsulko.com
Dear Tom,
In message 20210802213100.GG9379@bill-the-cat you wrote:
I think that if nothing changes the banner not changing is the right behavior.
Hm... is it? How about "external" changes like building with a different tool chain? Then at least the "version" command should produce a corresponding output, which means some parts _have_ to be recompiled, resulting in a new banner, too ?
Best regards,
Wolfgang Denk
On Wed, Aug 04, 2021 at 07:59:21AM +0200, Wolfgang Denk wrote:
Dear Tom,
In message 20210802213100.GG9379@bill-the-cat you wrote:
I think that if nothing changes the banner not changing is the right behavior.
Hm... is it? How about "external" changes like building with a different tool chain? Then at least the "version" command should produce a corresponding output, which means some parts _have_ to be recompiled, resulting in a new banner, too ?
There's some misunderstanding about what this series does, and does not do. First, with the series applied and you don't force SOURCE_DATE_EPOCH, and you run make, and you re-run make, cmd/version.c is rebuilt (but only that and not a bunch of other things anymore) and the banner timestamp changes. Second, if you do set SOURCE_DATE_EPOCH to a fixed value (so you're trying to reproduce a build with a specific timestamp) AND re-run make, nothing changes, as it should. Third, if you change gcc, everything gets rebuilt including cmd/version.c and a new banner.
Hi Tom,
On Wed, 4 Aug 2021 at 06:44, Tom Rini trini@konsulko.com wrote:
On Wed, Aug 04, 2021 at 07:59:21AM +0200, Wolfgang Denk wrote:
Dear Tom,
In message 20210802213100.GG9379@bill-the-cat you wrote:
I think that if nothing changes the banner not changing is the right behavior.
Hm... is it? How about "external" changes like building with a different tool chain? Then at least the "version" command should produce a corresponding output, which means some parts _have_ to be recompiled, resulting in a new banner, too ?
There's some misunderstanding about what this series does, and does not do. First, with the series applied and you don't force SOURCE_DATE_EPOCH, and you run make, and you re-run make, cmd/version.c is rebuilt (but only that and not a bunch of other things anymore) and the banner timestamp changes. Second, if you do set SOURCE_DATE_EPOCH to a fixed value (so you're trying to reproduce a build with a specific timestamp) AND re-run make, nothing changes, as it should. Third, if you change gcc, everything gets rebuilt including cmd/version.c and a new banner.
Yes that's my understanding and it seems fine.
Regards, Simon
On Wed, Aug 04, 2021 at 08:36:27AM -0600, Simon Glass wrote:
Hi Tom,
On Wed, 4 Aug 2021 at 06:44, Tom Rini trini@konsulko.com wrote:
On Wed, Aug 04, 2021 at 07:59:21AM +0200, Wolfgang Denk wrote:
Dear Tom,
In message 20210802213100.GG9379@bill-the-cat you wrote:
I think that if nothing changes the banner not changing is the right behavior.
Hm... is it? How about "external" changes like building with a different tool chain? Then at least the "version" command should produce a corresponding output, which means some parts _have_ to be recompiled, resulting in a new banner, too ?
There's some misunderstanding about what this series does, and does not do. First, with the series applied and you don't force SOURCE_DATE_EPOCH, and you run make, and you re-run make, cmd/version.c is rebuilt (but only that and not a bunch of other things anymore) and the banner timestamp changes. Second, if you do set SOURCE_DATE_EPOCH to a fixed value (so you're trying to reproduce a build with a specific timestamp) AND re-run make, nothing changes, as it should. Third, if you change gcc, everything gets rebuilt including cmd/version.c and a new banner.
Yes that's my understanding and it seems fine.
To be clear, I did a quick test with the series (git pw is very handy) for the above.
On Wednesday 04 August 2021 10:40:42 Tom Rini wrote:
On Wed, Aug 04, 2021 at 08:36:27AM -0600, Simon Glass wrote:
Hi Tom,
On Wed, 4 Aug 2021 at 06:44, Tom Rini trini@konsulko.com wrote:
On Wed, Aug 04, 2021 at 07:59:21AM +0200, Wolfgang Denk wrote:
Dear Tom,
In message 20210802213100.GG9379@bill-the-cat you wrote:
I think that if nothing changes the banner not changing is the right behavior.
Hm... is it? How about "external" changes like building with a different tool chain? Then at least the "version" command should produce a corresponding output, which means some parts _have_ to be recompiled, resulting in a new banner, too ?
There's some misunderstanding about what this series does, and does not do. First, with the series applied and you don't force SOURCE_DATE_EPOCH, and you run make, and you re-run make, cmd/version.c is rebuilt (but only that and not a bunch of other things anymore) and the banner timestamp changes. Second, if you do set SOURCE_DATE_EPOCH to a fixed value (so you're trying to reproduce a build with a specific timestamp) AND re-run make, nothing changes, as it should. Third, if you change gcc, everything gets rebuilt including cmd/version.c and a new banner.
Yes that's my understanding and it seems fine.
To be clear, I did a quick test with the series (git pw is very handy) for the above.
Thanks for clarification! You are of course right.
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
--Sean
On Wednesday 04 August 2021 17:43:41 Sean Anderson wrote:
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
You are missing 'FORCE' prerequisite for u-boot/u-boot.bin target (but I understand what you mean; I'm using this stuff too).
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
This behavior which you described is e.g. used by Linux kernel build system (also based on Makefile(s) and Kbuild as U-Boot).
Maybe we can discuss if such change can or cannot be useful for U-Boot too. But to not make another confusion, it is not part and it is not related to this my patch series. So I would suggest to discuss about it in separate / new thread (fell free to CC me).
On Wed, Aug 04, 2021 at 05:43:41PM -0400, Sean Anderson wrote:
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
Note that with this series applied, if you made use of SOURCE_DATE_EPOCH, nothing would be rebuilt. That may or may not make sense however, in your case. This series does get us closer to being able to do what the linux kernel does as there's now just one place rather than a bunch of places that are rebuilt.
On Wednesday 04 August 2021 18:09:14 Tom Rini wrote:
On Wed, Aug 04, 2021 at 05:43:41PM -0400, Sean Anderson wrote:
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
Note that with this series applied, if you made use of SOURCE_DATE_EPOCH, nothing would be rebuilt.
IIRC with SOURCE_DATE_EPOCH nothing would be rebuilt also without applying this my patch series.
That may or may not make sense however, in your case. This series does get us closer to being able to do what the linux kernel does as there's now just one place rather than a bunch of places that are rebuilt.
Just small correction. Even with this patch series there would not be just one place. There are still more places but this patch series reduce number of these places.
On Thu, Aug 05, 2021 at 12:14:29AM +0200, Pali Rohár wrote:
On Wednesday 04 August 2021 18:09:14 Tom Rini wrote:
On Wed, Aug 04, 2021 at 05:43:41PM -0400, Sean Anderson wrote:
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
Note that with this series applied, if you made use of SOURCE_DATE_EPOCH, nothing would be rebuilt.
IIRC with SOURCE_DATE_EPOCH nothing would be rebuilt also without applying this my patch series.
Correct, it does not.
That may or may not make sense however, in your case. This series does get us closer to being able to do what the linux kernel does as there's now just one place rather than a bunch of places that are rebuilt.
Just small correction. Even with this patch series there would not be just one place. There are still more places but this patch series reduce number of these places.
Ah, I only saw one place in my limited testing this morning.
On Wednesday 04 August 2021 18:15:55 Tom Rini wrote:
On Thu, Aug 05, 2021 at 12:14:29AM +0200, Pali Rohár wrote:
On Wednesday 04 August 2021 18:09:14 Tom Rini wrote:
On Wed, Aug 04, 2021 at 05:43:41PM -0400, Sean Anderson wrote:
On 8/2/21 3:21 PM, Simon Glass wrote:
Hi Pali,
On Mon, 2 Aug 2021 at 07:20, Pali Rohár pali@kernel.org wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org
arch/arm/mach-rockchip/tpl.c | 4 ++++ board/work-microwave/work_92105/work_92105_display.c | 1 + cmd/version.c | 1 + common/spl/spl.c | 4 ++++ drivers/rtc/emul_rtc.c | 2 +- include/version.h | 2 -- 6 files changed, 11 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
I assume we do actually want to regenerate the timestamp when U-Boot builds, even if nothing has changed. Is that right?
I know this is the current behavior, but it would be nice if this was not the case. If one is building U-Boot as part of a larger project, one might want to have a makefile rule like
u-boot/u-boot.bin: $(MAKE) -C u-boot $(@F)
but u-boot/u-boot.bin will always be remade even if no changes have been done. This will cause make to remake all dependents of U-Boot as well (which can be rather time-consuming).
At the moment, I just use U-Boot as an order-only dependency and remake it manually. But I would love it if U-Boot was only remade if dependencies had actually changed, since this would make it easier to integrate it with the rest of my build system.
Note that with this series applied, if you made use of SOURCE_DATE_EPOCH, nothing would be rebuilt.
IIRC with SOURCE_DATE_EPOCH nothing would be rebuilt also without applying this my patch series.
Correct, it does not.
That may or may not make sense however, in your case. This series does get us closer to being able to do what the linux kernel does as there's now just one place rather than a bunch of places that are rebuilt.
Just small correction. Even with this patch series there would not be just one place. There are still more places but this patch series reduce number of these places.
Ah, I only saw one place in my limited testing this morning.
For my tested boards it is also just one place. But for some other boards with extended config options it may be more places (like emul_rtc or work display, ...).
But if your testing discovered also only one place it could mean that it can be common case for more people...
On Mon, Aug 02, 2021 at 03:18:38PM +0200, Pali Rohár wrote:
Header file version.h does not use anything from timestamp.h. Including of timestamp.h has side effect which cause recompiling object file at every make run because timestamp.h changes at every run.
So remove timestamp.h from version.h and include timestamp.h in files which needs it.
This change reduce recompilation time of final U-Boot binary when U-Boot source files were not changed as less source files needs to be recompiled.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/next, thanks!
On Monday 02 August 2021 15:18:27 Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Hello! Is there anything else needed for this patch series?
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
arch/arm/mach-rockchip/px30-board-tpl.c | 1 - arch/arm/mach-rockchip/tpl.c | 4 ++++ arch/m68k/cpu/mcf5227x/start.S | 6 ------ arch/m68k/cpu/mcf523x/start.S | 6 ------ arch/m68k/cpu/mcf52x2/start.S | 6 ------ arch/m68k/cpu/mcf530x/start.S | 8 ------- arch/m68k/cpu/mcf532x/start.S | 6 ------ arch/m68k/cpu/mcf5445x/start.S | 7 ------- arch/nios2/cpu/start.S | 1 - arch/powerpc/cpu/mpc83xx/start.S | 10 +++------ arch/powerpc/cpu/mpc83xx/u-boot.lds | 3 +++ arch/powerpc/cpu/mpc85xx/start.S | 10 ++++----- arch/powerpc/cpu/mpc85xx/u-boot-nand.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot-spl.lds | 4 ++++ arch/powerpc/cpu/mpc85xx/u-boot.lds | 4 ++++ arch/powerpc/cpu/mpc8xx/start.S | 9 ++++---- arch/x86/cpu/quark/mrc.c | 4 +--- arch/x86/lib/acpi_table.c | 1 - board/atmel/sama5d2_ptc_ek/sama5d2_ptc_ek.c | 1 - board/cssi/MCR3000/u-boot.lds | 2 ++ board/ge/b1x5v2/b1x5v2.c | 2 +- board/ge/bx50v3/bx50v3.c | 2 +- board/ge/mx53ppd/mx53ppd.c | 2 +- board/l+g/vinco/vinco.c | 1 - board/renesas/grpeach/lowlevel_init.S | 1 - .../work_92105/work_92105_display.c | 1 + cmd/version.c | 7 ++++++- common/main.c | 2 +- common/spl/spl.c | 4 ++++ doc/develop/version.rst | 21 +++++++++++-------- drivers/rtc/emul_rtc.c | 2 +- drivers/usb/gadget/f_rockusb.c | 1 - drivers/video/cfb_console.c | 3 +-- include/configs/bcmstb.h | 1 - include/version.h | 8 ------- include/version_string.h | 8 +++++++ lib/display_options.c | 2 +- lib/efi_loader/efi_tcg2.c | 7 +++---- net/cdp.c | 3 --- test/print_ut.c | 2 +- 40 files changed, 75 insertions(+), 102 deletions(-) create mode 100644 include/version_string.h
-- 2.20.1
On Tue, Aug 17, 2021 at 01:02:41PM +0200, Pali Rohár wrote:
On Monday 02 August 2021 15:18:27 Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Hello! Is there anything else needed for this patch series?
This is likely going to wait for the next branch to open, thanks.
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
On Wednesday 01 September 2021 17:33:57 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
Including timestamp.h (either directly or transitionally) cause build system to recompile binaries at every 'make' run. This has disadvantage in U-Boot development as for every small change 'make' recompiles lot of other irrelevant files which were not touched / changed.
This patch series eliminate transitional / indirect usage of timestamp.h by removing unneeded inclusion of header files, moving timestamp values from macros to global variables, etc...
After these patches, U-Boot tools are not recompiled by every 'make' run, which decrease time for incremental U-Boot recompilation.
Please test these patches, specially m68k and powerpc parts as I do not have any of these boards.
Patch series depend on this patch (now marked as accepted): http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@...
Pali Rohár (11): Remove #include <timestamp.h> from files which do not need it Remove #include <version.h> from files which do not need it efi_loader: Use directly version_string variable version: Move version_string[] from version.h to version_string.h m68k: mcf: Remove overloading version_string version: Put version_string[] variable into section .text_version_string powerpc: mpc: Put U-Boot version string at correct place by linker script version: Do not make version_string[] variable as a weak x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log version: Remove global macro U_BOOT_VERSION_STRING from version.h Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
So this is the only other build which failed, right? I suspect that there is some other bug in xtfpga linker script, that it missed specifying wildcard sections and this change triggered it.
I will try to look at it.
It is pity that in above gitlab build log is missing full command which produced that error as in its arguments could be something interesting, like path to linker script or compile flags...
On Wednesday 01 September 2021 23:44:52 Pali Rohár wrote:
On Wednesday 01 September 2021 17:33:57 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
> Including timestamp.h (either directly or transitionally) cause build > system to recompile binaries at every 'make' run. This has disadvantage > in U-Boot development as for every small change 'make' recompiles lot of > other irrelevant files which were not touched / changed. > > This patch series eliminate transitional / indirect usage of > timestamp.h by removing unneeded inclusion of header files, moving > timestamp values from macros to global variables, etc... > > After these patches, U-Boot tools are not recompiled by every 'make' run, > which decrease time for incremental U-Boot recompilation. > > Please test these patches, specially m68k and powerpc parts as I do not > have any of these boards. > > Patch series depend on this patch (now marked as accepted): > http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@... > > Pali Rohár (11): > Remove #include <timestamp.h> from files which do not need it > Remove #include <version.h> from files which do not need it > efi_loader: Use directly version_string variable > version: Move version_string[] from version.h to version_string.h > m68k: mcf: Remove overloading version_string > version: Put version_string[] variable into section > .text_version_string > powerpc: mpc: Put U-Boot version string at correct place by linker > script > version: Do not make version_string[] variable as a weak > x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log > version: Remove global macro U_BOOT_VERSION_STRING from version.h > Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
So this is the only other build which failed, right? I suspect that there is some other bug in xtfpga linker script, that it missed specifying wildcard sections and this change triggered it.
I will try to look at it.
Just a quick look... https://source.denx.de/u-boot/u-boot/-/blob/master/arch/xtensa/cpu/u-boot.ld... Probably it is needed to specify .text_version_string section here, like is specified .text section there.
It is pity that in above gitlab build log is missing full command which produced that error as in its arguments could be something interesting, like path to linker script or compile flags...
On Wednesday 01 September 2021 23:49:57 Pali Rohár wrote:
On Wednesday 01 September 2021 23:44:52 Pali Rohár wrote:
On Wednesday 01 September 2021 17:33:57 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote: > On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote: > > > Including timestamp.h (either directly or transitionally) cause build > > system to recompile binaries at every 'make' run. This has disadvantage > > in U-Boot development as for every small change 'make' recompiles lot of > > other irrelevant files which were not touched / changed. > > > > This patch series eliminate transitional / indirect usage of > > timestamp.h by removing unneeded inclusion of header files, moving > > timestamp values from macros to global variables, etc... > > > > After these patches, U-Boot tools are not recompiled by every 'make' run, > > which decrease time for incremental U-Boot recompilation. > > > > Please test these patches, specially m68k and powerpc parts as I do not > > have any of these boards. > > > > Patch series depend on this patch (now marked as accepted): > > http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@... > > > > Pali Rohár (11): > > Remove #include <timestamp.h> from files which do not need it > > Remove #include <version.h> from files which do not need it > > efi_loader: Use directly version_string variable > > version: Move version_string[] from version.h to version_string.h > > m68k: mcf: Remove overloading version_string > > version: Put version_string[] variable into section > > .text_version_string > > powerpc: mpc: Put U-Boot version string at correct place by linker > > script > > version: Do not make version_string[] variable as a weak > > x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log > > version: Remove global macro U_BOOT_VERSION_STRING from version.h > > Remove including timestamp.h in version.h > > So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 > this fails to build for at least qemu-ppce500 and xtfpga. Over in > doc/develop/ci_testing.rst we document how to run a world build. Please > fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
So this is the only other build which failed, right? I suspect that there is some other bug in xtfpga linker script, that it missed specifying wildcard sections and this change triggered it.
I will try to look at it.
Just a quick look... https://source.denx.de/u-boot/u-boot/-/blob/master/arch/xtensa/cpu/u-boot.ld... Probably it is needed to specify .text_version_string section here, like is specified .text section there.
I really do not know what is the memory layout of u-boot image for this platform, but could not something like this help?
--- arch/xtensa/cpu/u-boot.lds 2021-09-10 22:50:51.501324477 +0200 +++ arch/xtensa/cpu/u-boot.lds 2021-09-10 22:53:55.271410047 +0200 @@ -47,6 +47,7 @@ SECTIONS RELOCATE2(DoubleExceptionVector,literal); RELOCATE2(DoubleExceptionVector,text); RELOCATE1(text); + RELOCATE1(text_version_string); RELOCATE1(rodata); RELOCATE1(data); RELOCATE1(u_boot_list); @@ -76,7 +77,8 @@ SECTIONS __monitor_start = XTENSA_SYS_TEXT_ADDR;
SECTION_text(XTENSA_SYS_TEXT_ADDR, FOLLOWING(.DoubleExceptionVector.text)) - SECTION_rodata(ALIGN(16), FOLLOWING(.text)) + SECTION_text_version_string(ALIGN(16), FOLLOWING(.text)) + SECTION_rodata(ALIGN(16), FOLLOWING(.text_version_string)) SECTION_u_boot_list(ALIGN(16), FOLLOWING(.rodata)) SECTION_data(ALIGN(16), FOLLOWING(.u_boot_list))
It is pity that in above gitlab build log is missing full command which produced that error as in its arguments could be something interesting, like path to linker script or compile flags...
On Fri, Sep 10, 2021 at 10:56:18PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 23:49:57 Pali Rohár wrote:
On Wednesday 01 September 2021 23:44:52 Pali Rohár wrote:
On Wednesday 01 September 2021 17:33:57 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote: > On Wednesday 01 September 2021 16:59:09 Tom Rini wrote: > > On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote: > > > > > Including timestamp.h (either directly or transitionally) cause build > > > system to recompile binaries at every 'make' run. This has disadvantage > > > in U-Boot development as for every small change 'make' recompiles lot of > > > other irrelevant files which were not touched / changed. > > > > > > This patch series eliminate transitional / indirect usage of > > > timestamp.h by removing unneeded inclusion of header files, moving > > > timestamp values from macros to global variables, etc... > > > > > > After these patches, U-Boot tools are not recompiled by every 'make' run, > > > which decrease time for incremental U-Boot recompilation. > > > > > > Please test these patches, specially m68k and powerpc parts as I do not > > > have any of these boards. > > > > > > Patch series depend on this patch (now marked as accepted): > > > http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@... > > > > > > Pali Rohár (11): > > > Remove #include <timestamp.h> from files which do not need it > > > Remove #include <version.h> from files which do not need it > > > efi_loader: Use directly version_string variable > > > version: Move version_string[] from version.h to version_string.h > > > m68k: mcf: Remove overloading version_string > > > version: Put version_string[] variable into section > > > .text_version_string > > > powerpc: mpc: Put U-Boot version string at correct place by linker > > > script > > > version: Do not make version_string[] variable as a weak > > > x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log > > > version: Remove global macro U_BOOT_VERSION_STRING from version.h > > > Remove including timestamp.h in version.h > > > > So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 > > this fails to build for at least qemu-ppce500 and xtfpga. Over in > > doc/develop/ci_testing.rst we document how to run a world build. Please > > fix these build errors and re-submit, thanks. > > Already happened about month ago > https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@... > > As stated, following build command now passes: > make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
So this is the only other build which failed, right? I suspect that there is some other bug in xtfpga linker script, that it missed specifying wildcard sections and this change triggered it.
I will try to look at it.
Just a quick look... https://source.denx.de/u-boot/u-boot/-/blob/master/arch/xtensa/cpu/u-boot.ld... Probably it is needed to specify .text_version_string section here, like is specified .text section there.
I really do not know what is the memory layout of u-boot image for this platform, but could not something like this help?
--- arch/xtensa/cpu/u-boot.lds 2021-09-10 22:50:51.501324477 +0200 +++ arch/xtensa/cpu/u-boot.lds 2021-09-10 22:53:55.271410047 +0200 @@ -47,6 +47,7 @@ SECTIONS RELOCATE2(DoubleExceptionVector,literal); RELOCATE2(DoubleExceptionVector,text); RELOCATE1(text);
- RELOCATE1(text_version_string); RELOCATE1(rodata); RELOCATE1(data); RELOCATE1(u_boot_list);
@@ -76,7 +77,8 @@ SECTIONS __monitor_start = XTENSA_SYS_TEXT_ADDR;
SECTION_text(XTENSA_SYS_TEXT_ADDR, FOLLOWING(.DoubleExceptionVector.text))
- SECTION_rodata(ALIGN(16), FOLLOWING(.text))
- SECTION_text_version_string(ALIGN(16), FOLLOWING(.text))
- SECTION_rodata(ALIGN(16), FOLLOWING(.text_version_string)) SECTION_u_boot_list(ALIGN(16), FOLLOWING(.rodata)) SECTION_data(ALIGN(16), FOLLOWING(.u_boot_list))
It is pity that in above gitlab build log is missing full command which produced that error as in its arguments could be something interesting, like path to linker script or compile flags...
Seems likely, thanks for digging something out!
On Wed, Sep 01, 2021 at 11:44:52PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:33:57 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:28:54PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 17:17:06 Tom Rini wrote:
On Wed, Sep 01, 2021 at 11:05:45PM +0200, Pali Rohár wrote:
On Wednesday 01 September 2021 16:59:09 Tom Rini wrote:
On Mon, Aug 02, 2021 at 03:18:27PM +0200, Pali Rohár wrote:
> Including timestamp.h (either directly or transitionally) cause build > system to recompile binaries at every 'make' run. This has disadvantage > in U-Boot development as for every small change 'make' recompiles lot of > other irrelevant files which were not touched / changed. > > This patch series eliminate transitional / indirect usage of > timestamp.h by removing unneeded inclusion of header files, moving > timestamp values from macros to global variables, etc... > > After these patches, U-Boot tools are not recompiled by every 'make' run, > which decrease time for incremental U-Boot recompilation. > > Please test these patches, specially m68k and powerpc parts as I do not > have any of these boards. > > Patch series depend on this patch (now marked as accepted): > http://patchwork.ozlabs.org/project/uboot/patch/20210710111001.32325-1-pali@... > > Pali Rohár (11): > Remove #include <timestamp.h> from files which do not need it > Remove #include <version.h> from files which do not need it > efi_loader: Use directly version_string variable > version: Move version_string[] from version.h to version_string.h > m68k: mcf: Remove overloading version_string > version: Put version_string[] variable into section > .text_version_string > powerpc: mpc: Put U-Boot version string at correct place by linker > script > version: Do not make version_string[] variable as a weak > x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log > version: Remove global macro U_BOOT_VERSION_STRING from version.h > Remove including timestamp.h in version.h
So, looking at https://source.denx.de/u-boot/u-boot/-/pipelines/8948 this fails to build for at least qemu-ppce500 and xtfpga. Over in doc/develop/ci_testing.rst we document how to run a world build. Please fix these build errors and re-submit, thanks.
Already happened about month ago https://patchwork.ozlabs.org/project/uboot/patch/20210808112038.7942-1-pali@...
As stated, following build command now passes: make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin
OK, I'll make sure to grab that. Note that xtfpga isn't PowerPC...
I saw only error https://source.denx.de/u-boot/u-boot/-/jobs/316601 and this should be fixed above patch. At least I got similar error for MCR3000_defconfig with new gcc before that.
But now after scrolling down I see that second xtfpga error https://source.denx.de/u-boot/u-boot/-/jobs/316614 But seems that in this UI is error log truncated. I see only
+xtensa-dc233c-elf-ld.bfd: section .text_version_string LMA [00000000fe021584,00000000fe0215c7] overlaps section .u_boot_list LMA [00000000fe021584,00000000fe021e6b]
Is there a way how to show full build log? And which defconfig and compiler is used? Because that error does not help me what is wrong here...
That's the full error log, from the linker, I believe. It's the xtfpga config for the xtensa architecture. It's one of the few that buildman won't fetch a good toolchain for so you'll want to look at tools/docker/Dockerfile and see we get it from https://github.com/foss-xtensa/toolchain/releases/download/2020.07/x86_64-20... if you don't have the CI builder container itself handy already.
So this is the only other build which failed, right? I suspect that
I don't know. The GitLab job is configured in three stages so if something early fails we don't wait forever to fail. The Azure job is over at https://dev.azure.com/u-boot/u-boot/_build/results?buildId=2792&view=res... and hasn't completed but doesn't look like it's showing anything you haven't already fixed.
there is some other bug in xtfpga linker script, that it missed specifying wildcard sections and this change triggered it.
I will try to look at it.
It is pity that in above gitlab build log is missing full command which produced that error as in its arguments could be something interesting, like path to linker script or compile flags...
I think what you're missing isn't about gitlab, but rather about buildman. If you've picked up any tips on how to debug these problems, updating doc/develop/ci_testing.rst for the next time would be good.
participants (11)
-
Bin Meng -
Christophe Leroy -
Francesco Dolcini -
Heinrich Schuchardt -
Ilias Apalodimas -
Max Filippov -
Pali Rohár -
Sean Anderson -
Simon Glass -
Tom Rini -
Wolfgang Denk