[PATCH 00/11] Reduce usage of timestamp macros

[PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/

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

[PATCH 01/11] Remove #include <timestamp.h> from files which do not need it

[Pali Rohár]

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"
 
-- 
2.20.1

[PATCH 02/11] Remove #include <version.h> from files which do not need it

[Pali Rohár]

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__
-- 
2.20.1

[PATCH 03/11] efi_loader: Use directly version_string variable

[Pali Rohár]

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;
-- 
2.20.1

[PATCH 04/11] version: Move version_string[] from version.h to version_string.h

[Pali Rohár]

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>
-- 
2.20.1

[PATCH 05/11] m68k: mcf: Remove overloading version_string

[Pali Rohár]

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
-- 
2.20.1

[PATCH 06/11] version: Put version_string[] variable into section .text_version_string

[Pali Rohár]

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[])
-- 
2.20.1

[PATCH 07/11] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Pali Rohár]

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*)
-- 
2.20.1

[PATCH 08/11] version: Do not make version_string[] variable as a weak

[Pali Rohár]

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[])
-- 
2.20.1

[PATCH 09/11] x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log

[Pali Rohár]

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);
-- 
2.20.1

[PATCH 10/11] version: Remove global macro U_BOOT_VERSION_STRING from version.h

[Pali Rohár]

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__ */
-- 
2.20.1

[PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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
-- 
2.20.1

Re: [PATCH 01/11] Remove #include <timestamp.h> from files which do not need it

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 168 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 02/11] Remove #include <version.h> from files which do not need it

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 168 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 04/11] version: Move version_string[] from version.h to version_string.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 06/11] version: Put version_string[] variable into section .text_version_string

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 247 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 08/11] version: Do not make version_string[] variable as a weak

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 10/11] version: Remove global macro U_BOOT_VERSION_STRING from version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/11] x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log

[Simon Glass]

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>

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Simon Glass]

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

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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...

Re: [PATCH 03/11] efi_loader: Use directly version_string variable

[Heinrich Schuchardt]

+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;
>

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

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>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/11] x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log

[Bin Meng]

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>

Re: [PATCH 03/11] efi_loader: Use directly version_string variable

[Ilias Apalodimas]

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;
> >

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Any sufficiently advanced  technology  is  indistinguishable  from  a
rigged demo.                              - Andy Finkel, computer guy

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Simon Glass]

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

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1469 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Sean Anderson]

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

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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.

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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).

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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.

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3198 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Pali Rohár]

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...

[PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Pali Rohár]

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*)
-- 
2.20.1

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Pali Rohár]

Christophe, could you please review this change? According to your commit
https://source.denx.de/u-boot/u-boot/-/commit/907208c452999427cb2f43450308045bf8b42958
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*)
>  		*(.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*)
> -- 
> 2.20.1
> 

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/

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
> 

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

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@kernel.org/
> 
> Hello! Is there anything else needed for this patch series?

This is likely going to wait for the next branch to open, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Christophe Leroy]

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/907208c452999427cb2f43450308045bf8b42958
> 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

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Christophe Leroy]

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)
>   		arch/powerpc/cpu/mpc8xx/start.o	(.text)
>   		arch/powerpc/cpu/mpc8xx/traps.o	(.text*)
>   		arch/powerpc/lib/built-in.o		(.text*)
> 

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"If you want to eat hippopotamus, you've got to pay the freight."   -
attributed to an IBM guy, about why IBM software uses so much memory

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Pali Rohár]

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)
> >   		arch/powerpc/cpu/mpc8xx/start.o	(.text)
> >   		arch/powerpc/cpu/mpc8xx/traps.o	(.text*)
> >   		arch/powerpc/lib/built-in.o		(.text*)
> > 

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]

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@kernel.org/
> 
> 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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/
> > 
> > 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@kernel.org/

As stated, following build command now passes:
make CROSS_COMPILE=powerpc-linux-gnu- MCR3000_defconfig u-boot.bin

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]

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@kernel.org/
> > > 
> > > 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@kernel.org/
> 
> 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...

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/
> > > > 
> > > > 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@kernel.org/
> > 
> > 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...

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4038 bytes --]

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@kernel.org/
> > > > > 
> > > > > 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@kernel.org/
> > > 
> > > 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-2020.07-xtensa-dc233c-elf.tar.gz
if you don't have the CI builder container itself handy already.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/
> > > > > > 
> > > > > > 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@kernel.org/
> > > > 
> > > > 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-2020.07-xtensa-dc233c-elf.tar.gz
> 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...

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/
> > > > > > > 
> > > > > > > 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@kernel.org/
> > > > > 
> > > > > 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-2020.07-xtensa-dc233c-elf.tar.gz
> > 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.lds#L78
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...

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5422 bytes --]

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@kernel.org/
> > > > > > > 
> > > > > > > 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@kernel.org/
> > > > > 
> > > > > 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-2020.07-xtensa-dc233c-elf.tar.gz
> > 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=results
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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Pali Rohár]

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@kernel.org/
> > > > > > > > 
> > > > > > > > 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@kernel.org/
> > > > > > 
> > > > > > 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-2020.07-xtensa-dc233c-elf.tar.gz
> > > 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.lds#L78
> 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...

Re: [PATCH 00/11] Reduce usage of timestamp macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6635 bytes --]

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@kernel.org/
> > > > > > > > > 
> > > > > > > > > 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@kernel.org/
> > > > > > > 
> > > > > > > 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-2020.07-xtensa-dc233c-elf.tar.gz
> > > > 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.lds#L78
> > 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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Tom Rini]

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.*)		\
-- 
2.17.1

Re: [PATCH v2] powerpc: mpc: Put U-Boot version string at correct place by linker script

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Francesco Dolcini]

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

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Francesco Dolcini]

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

[PATCH 1/1] powerpc: Drop version_string placement optimization

[Tom Rini]

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
-- 
2.17.1

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

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 :(

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Francesco Dolcini]

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

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Max Filippov]

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<---

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Max Filippov]

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.

-- 
Thanks.
-- Max

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 524 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]

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@kernel.org/

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@konsulko.com/
and no longer needing to reference it from the linker script for
PowerPC.  So maybe I can just drop this whole part.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] xtensa: Put U-Boot version string at correct place by linker script

[Max Filippov]

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@kernel.org/

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@konsulko.com/
> 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.

-- 
Thanks.
-- Max

Re: [PATCH 01/11] Remove #include <timestamp.h> from files which do not need it

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 203 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 02/11] Remove #include <version.h> from files which do not need it

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 203 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 03/11] efi_loader: Use directly version_string variable

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 04/11] version: Move version_string[] from version.h to version_string.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 05/11] m68k: mcf: Remove overloading version_string

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 08/11] version: Do not make version_string[] variable as a weak

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 09/11] x86: quark: MRC: Remove U_BOOT_DATE and U_BOOT_TIME from debug log

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 10/11] version: Remove global macro U_BOOT_VERSION_STRING from version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 11/11] Remove including timestamp.h in version.h

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/1] powerpc: Drop version_string placement optimization

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]