All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] dm: Experiments for reducing SPL memory usage
@ 2022-03-27 20:26 Simon Glass
  2022-03-27 20:26 ` [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build Simon Glass
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, AKASHI Takahiro,
	Alexandru Gagniuc, Bin Meng, Dario Binacchi, Ilias Apalodimas,
	Joel Peshkin, Marek Vasut, Masahiro Yamada, Pali Rohár,
	Patrick Delaunay, Pavel Herrmann, Rasmus Villemoes,
	Ricardo Salveti, Stefan Roese, Ye Li

This series explores using the (not yet applied) tag support in driver
model to reduce memory usage in SPL.

This is particularly important on 64-bit machines, which use a
ridiculously large 8 bytes for each pointer into what what is sometimes
only 64KB of available memory.

The primary focus of this series is struct udevice, which can be shrunk
in various ways:

- Including devres_head only when DEVRES is enabled (i.e. not in SPL)
- Using tags instead of pointers for attached data like plat_, priv_ and
  driver_data
- Using singly linked lists, currently not supported in U-Boot
- Using a table index for the driver pointer and uclass pointer

Together these bring the size of struct udevice down from 0xa0 (160) bytes
to 0x30 (48) bytes.

Another option is to drop the device name, although this is a pain for
debugging.

It would also be possible to implement doubly linked lists with a
16-bit index into malloc space, in SPL, thus reducing the overhead in each
node from 16 bytes to 2, or just using a fixed-size list for each data
structure, since the number of items is quite limited in SPL.

To implement the tag side of things, functions like dev_set_parent_priv()
need to be modified to call dev_tag_set_ptr(), and dev_get_parent_priv()
needs to call dev_tag_get_ptr().

Note there has been some previous work:

- tiny-dm analysis[1] which we decided was too disruptive
- build-time instantiation, to reduce SPL code size[2].

[1] https://patchwork.ozlabs.org/project/uboot/cover/20200702211004.1491489-1-sjg@chromium.org/
[2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html#build-time-instantiation


AKASHI Takahiro (1):
  RFC: dm: add tag support

Simon Glass (9):
  Makefile: v2 Allow LTO to be disabled for a build
  sandbox: Correct loss of early output in SPL
  Makefile: Drop a stale comment about linking
  Makefile: Avoid resetting link flags in config.mk
  sandbox: Allow link flags to be given
  sandbox: Align linker lists to a 32-byte boundary
  dm: core: Allow devres to be disabled in SPL
  dm: core: Deal with a wrinkle with linker lists
  WIP: dm: core: Add a command to calculate memory usage

 Makefile                           |  22 ++--
 arch/arm/config.mk                 |   4 +-
 arch/arm/include/asm/global_data.h |   2 +-
 arch/sandbox/config.mk             |   4 +-
 arch/sandbox/cpu/os.c              |   2 +-
 arch/sandbox/cpu/u-boot-spl.lds    |   2 +-
 arch/sandbox/cpu/u-boot.lds        |   2 +-
 cmd/dm.c                           |  15 ++-
 common/spl/spl.c                   |   9 ++
 config.mk                          |   1 -
 doc/build/gcc.rst                  |  17 +++
 drivers/core/Makefile              |   4 +-
 drivers/core/device.c              |  70 +++++++++++-
 drivers/core/dump.c                |  73 +++++++++++++
 drivers/core/root.c                |  63 ++++++++++-
 drivers/core/tag.c                 | 168 +++++++++++++++++++++++++++++
 include/asm-generic/global_data.h  |   4 +
 include/dm/device-internal.h       |   6 +-
 include/dm/device.h                |  16 ++-
 include/dm/devres.h                |   4 +-
 include/dm/root.h                  |  45 ++++++++
 include/dm/tag.h                   | 126 ++++++++++++++++++++++
 include/dm/util.h                  |   9 ++
 scripts/Makefile.spl               |   2 +-
 test/dm/Makefile                   |   2 +-
 25 files changed, 640 insertions(+), 32 deletions(-)
 create mode 100644 drivers/core/tag.c
 create mode 100644 include/dm/tag.h

-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-03-31 10:29   ` Andrew Scull
  2022-03-27 20:26 ` [PATCH 02/10] sandbox: Correct loss of early output in SPL Simon Glass
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Bin Meng,
	Ilias Apalodimas, Joel Peshkin, Masahiro Yamada,
	Patrick Delaunay, Ye Li

LTO (Link-Time Optimisation) is an very useful feature which can
significantly reduce the size of U-Boot binaries. So far it has been
made available for selected ARM boards and sandbox.

However, incremental builds are much slower when LTO is used. For example,
an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
seconds with LTO enabled.

Add a LTO_BUILD=n parameter to the build, so it can be disabled during
development if needed, for faster builds.

Add some documentation about LTO while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile                           | 18 +++++++++++++-----
 arch/arm/config.mk                 |  4 ++--
 arch/arm/include/asm/global_data.h |  2 +-
 doc/build/gcc.rst                  | 17 +++++++++++++++++
 scripts/Makefile.spl               |  2 +-
 5 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 06572ac07ee..c9585ddebfc 100644
--- a/Makefile
+++ b/Makefile
@@ -434,6 +434,9 @@ KBUILD_CFLAGS	+= -fshort-wchar -fno-strict-aliasing
 KBUILD_AFLAGS   := -D__ASSEMBLY__
 KBUILD_LDFLAGS  :=
 
+# Set this to "n" use of LTO for this build, e.g. LTO_BUILD=n
+LTO_BUILD	?= y
+
 ifeq ($(cc-name),clang)
 ifneq ($(CROSS_COMPILE),)
 CLANG_TARGET	:= --target=$(notdir $(CROSS_COMPILE:%-=%))
@@ -643,6 +646,11 @@ export CFLAGS_EFI	# Compiler flags to add when building EFI app
 export CFLAGS_NON_EFI	# Compiler flags to remove when building EFI app
 export EFI_TARGET	# binutils target if EFI is natively supported
 
+export LTO_ENABLE
+
+# This is y if LTO is enabled for this build
+LTO_ENABLE=$(if $(CONFIG_LTO),$(LTO_BUILD),)
+
 # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use
 # that (or fail if absent).  Otherwise, search for a linker script in a
 # standard location.
@@ -690,16 +698,16 @@ endif
 LTO_CFLAGS :=
 LTO_FINAL_LDFLAGS :=
 export LTO_CFLAGS LTO_FINAL_LDFLAGS
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 	ifeq ($(cc-name),clang)
-		LTO_CFLAGS		+= -flto
+		LTO_CFLAGS		+= -DLTO_ENABLE -flto
 		LTO_FINAL_LDFLAGS	+= -flto
 
 		AR			= $(shell $(CC) -print-prog-name=llvm-ar)
 		NM			= $(shell $(CC) -print-prog-name=llvm-nm)
 	else
 		NPROC			:= $(shell nproc 2>/dev/null || echo 1)
-		LTO_CFLAGS		+= -flto=$(NPROC)
+		LTO_CFLAGS		+= -DLTO_ENABLE -flto=$(NPROC)
 		LTO_FINAL_LDFLAGS	+= -fuse-linker-plugin -flto=$(NPROC)
 
 		# use plugin aware tools
@@ -1740,7 +1748,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(ARCH)/Makefile.postlink)
 
 # Generate linker list symbols references to force compiler to not optimize
 # them away when compiling with LTO
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 u-boot-keep-syms-lto := keep-syms-lto.o
 u-boot-keep-syms-lto_c := $(patsubst %.o,%.c,$(u-boot-keep-syms-lto))
 
@@ -1762,7 +1770,7 @@ endif
 
 # Rule to link u-boot
 # May be overridden by arch/$(ARCH)/config.mk
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 quiet_cmd_u-boot__ ?= LTO     $@
       cmd_u-boot__ ?=								\
 		$(CC) -nostdlib -nostartfiles					\
diff --git a/arch/arm/config.mk b/arch/arm/config.mk
index b107b1af27a..065dbec4064 100644
--- a/arch/arm/config.mk
+++ b/arch/arm/config.mk
@@ -15,11 +15,11 @@ CFLAGS_NON_EFI := -fno-pic -ffixed-r9 -ffunction-sections -fdata-sections \
 		  -fstack-protector-strong
 CFLAGS_EFI := -fpic -fshort-wchar
 
-ifneq ($(CONFIG_LTO)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
+ifneq ($(LTO_ENABLE)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
 LDFLAGS_FINAL += --gc-sections
 endif
 
-ifndef CONFIG_LTO
+ifneq ($(LTO_ENABLE),y)
 PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
 endif
 
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index 085e12b5d4d..b255b195aa0 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -98,7 +98,7 @@ struct arch_global_data {
 
 #include <asm-generic/global_data.h>
 
-#if defined(__clang__) || defined(CONFIG_LTO)
+#if defined(__clang__) || defined(LTO_ENABLE)
 
 #define DECLARE_GLOBAL_DATA_PTR
 #define gd	get_gd()
diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
index 470a7aa3498..792be176aad 100644
--- a/doc/build/gcc.rst
+++ b/doc/build/gcc.rst
@@ -152,6 +152,23 @@ of dtc is new enough. It also makes sure that pylibfdt is present, if needed
 Note that the :doc:`tools` are always built with the included version of libfdt
 so it is not possible to build U-Boot tools with a system libfdt, at present.
 
+Link-time optimisation (LTO)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+U-Boot supports link-time optimisation which can reduce the size of the final
+U-Boot binaries, particularly with SPL.
+
+At present this can be enabled by ARM boards by adding `CONFIG_LTO=y` into the
+defconfig file. Other architectures are not supported. LTO is enabled by default
+for sandbox.
+
+This does incur a link-time penalty of several seconds. For faster incremental
+builds during development, you can disable it by setting `LTO_BUILD` to `n`.
+
+.. code-block:: bash
+
+    LTO_BUILD=n make
+
 Other build targets
 ~~~~~~~~~~~~~~~~~~~
 
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 83a95ee4aa2..e3ca69c4449 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -472,7 +472,7 @@ endif
 
 # Rule to link u-boot-spl
 # May be overridden by arch/$(ARCH)/config.mk
-ifdef CONFIG_LTO
+ifeq ($(LTO_ENABLE),y)
 quiet_cmd_u-boot-spl ?= LTO     $@
       cmd_u-boot-spl ?= \
 	(									\
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/10] sandbox: Correct loss of early output in SPL
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
  2022-03-27 20:26 ` [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 03/10] Makefile: Drop a stale comment about linking Simon Glass
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass

At present fputc() is used before the console is available, then write()
is used. These are not compatible. Since fputc() buffers internally it is
better to use the write(), so that a partial line is immediately
displayed.

This has a slight effect on performance, but we are already using write()
for the vast majority of the output with no obvious impacts.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/os.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index d83c862182d..5ea54179176 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -644,7 +644,7 @@ int os_get_filesize(const char *fname, long long *size)
 
 void os_putc(int ch)
 {
-	fputc(ch, stdout);
+	os_write(1, &ch, 1);
 }
 
 void os_puts(const char *str)
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/10] Makefile: Drop a stale comment about linking
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
  2022-03-27 20:26 ` [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build Simon Glass
  2022-03-27 20:26 ` [PATCH 02/10] sandbox: Correct loss of early output in SPL Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk Simon Glass
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Masahiro Yamada

The bug mentioned here is fixed, so drop the comment.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/Makefile b/Makefile
index c9585ddebfc..e23c3204d21 100644
--- a/Makefile
+++ b/Makefile
@@ -1785,10 +1785,6 @@ quiet_cmd_u-boot__ ?= LTO     $@
 		-Wl,-Map,u-boot.map;						\
 		$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 else
-# Note: Linking efi-x86_app64 causes a segfault in the linker at present
-# when using x86_64-linux-gnu-ld.bfd
-# For now, disable --whole-archive which makes things link, although not
-# correctly
 quiet_cmd_u-boot__ ?= LD      $@
       cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@		\
 		-T u-boot.lds $(u-boot-init)					\
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (2 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 03/10] Makefile: Drop a stale comment about linking Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 05/10] sandbox: Allow link flags to be given Simon Glass
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Masahiro Yamada

This makes it impossible to change them elsewhere. The default value is
'empty' anyway, so just drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 config.mk | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mk b/config.mk
index 2595aed218b..b915c29b3f3 100644
--- a/config.mk
+++ b/config.mk
@@ -12,7 +12,6 @@
 #  If we did not have Tegra SoCs, build system would be much simpler...)
 PLATFORM_RELFLAGS :=
 PLATFORM_CPPFLAGS :=
-KBUILD_LDFLAGS :=
 LDFLAGS_FINAL :=
 LDFLAGS_STANDALONE :=
 OBJCOPYFLAGS :=
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/10] sandbox: Allow link flags to be given
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (3 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary Simon Glass
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass

At present the link flags are not used for sandbox. Update the command
line to use them.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/config.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
index 2b1b657831c..02a3ba0c0e9 100644
--- a/arch/sandbox/config.mk
+++ b/arch/sandbox/config.mk
@@ -16,7 +16,7 @@ PLATFORM_CPPFLAGS += $(shell $(SDL_CONFIG) --cflags)
 endif
 
 cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) \
-	$(LTO_FINAL_LDFLAGS) \
+	$(KBUILD_LDFLAGS:%=-Wl,%)$(LTO_FINAL_LDFLAGS) \
 	-Wl,--whole-archive \
 		$(u-boot-main) \
 		$(u-boot-keep-syms-lto) \
@@ -24,7 +24,7 @@ cmd_u-boot__ = $(CC) -o $@ -Wl,-T u-boot.lds $(u-boot-init) \
 	$(PLATFORM_LIBS) -Wl,-Map -Wl,u-boot.map
 
 cmd_u-boot-spl = (cd $(obj) && $(CC) -o $(SPL_BIN) -Wl,-T u-boot-spl.lds \
-	$(LTO_FINAL_LDFLAGS) \
+	$(KBUILD_LDFLAGS:%=-Wl,%) $(LTO_FINAL_LDFLAGS) \
 	$(patsubst $(obj)/%,%,$(u-boot-spl-init)) \
 	-Wl,--whole-archive \
 		$(patsubst $(obj)/%,%,$(u-boot-spl-main)) \
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (4 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 05/10] sandbox: Allow link flags to be given Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 07/10] dm: core: Allow devres to be disabled in SPL Simon Glass
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass

Use this larger boundary to ensure that linker lists at least start on the
maximum possible alignment boundary. See also the CONFIG_LINKER_LIST_ALIGN
setting, but that is host-arch-specific, so it seems better to use the
largest value for every host architecture.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/sandbox/cpu/u-boot-spl.lds | 2 +-
 arch/sandbox/cpu/u-boot.lds     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds
index 6754f4ef6cc..206e265e74b 100644
--- a/arch/sandbox/cpu/u-boot-spl.lds
+++ b/arch/sandbox/cpu/u-boot-spl.lds
@@ -8,7 +8,7 @@
 SECTIONS
 {
 
-	. = ALIGN(4);
+	. = ALIGN(32);
 	.u_boot_list : {
 		KEEP(*(SORT(.u_boot_list*)));
 	}
diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
index 6d710618f59..92e834a8d2b 100644
--- a/arch/sandbox/cpu/u-boot.lds
+++ b/arch/sandbox/cpu/u-boot.lds
@@ -8,7 +8,7 @@
 SECTIONS
 {
 
-	. = ALIGN(4);
+	. = ALIGN(32);
 	.u_boot_list : {
 		KEEP(*(SORT(.u_boot_list*)));
 	}
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/10] dm: core: Allow devres to be disabled in SPL
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (5 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-12 19:07   ` Angus Ainslie
  2022-04-19 12:49   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists Simon Glass
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Marek Vasut,
	Pavel Herrmann

At present if devres is enabled in U-Boot proper it is enabled in SPL.
We don't normally want it there, so disable it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/Makefile        | 2 +-
 drivers/core/device.c        | 2 +-
 include/dm/device-internal.h | 6 +++---
 include/dm/device.h          | 2 +-
 include/dm/devres.h          | 4 ++--
 test/dm/Makefile             | 2 +-
 6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 5edd4e41357..0cbc3ab217e 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -4,7 +4,7 @@
 
 obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o
 obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
-obj-$(CONFIG_DEVRES) += devres.o
+obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
 obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)	+= device-remove.o
 obj-$(CONFIG_$(SPL_)SIMPLE_BUS)	+= simple-bus.o
 obj-$(CONFIG_SIMPLE_PM_BUS)	+= simple-pm-bus.o
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 1b356f12dd8..b7ce8544140 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -68,7 +68,7 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv,
 	INIT_LIST_HEAD(&dev->sibling_node);
 	INIT_LIST_HEAD(&dev->child_head);
 	INIT_LIST_HEAD(&dev->uclass_node);
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
 	INIT_LIST_HEAD(&dev->devres_head);
 #endif
 	dev_set_plat(dev, plat);
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index c420726287e..2fc41f31f5a 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -396,7 +396,7 @@ fdt_addr_t simple_bus_translate(struct udevice *dev, fdt_addr_t addr);
 #define DM_UCLASS_ROOT_S_NON_CONST	(((gd_t *)gd)->uclass_root_s)
 
 /* device resource management */
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
 
 /**
  * devres_release_probe - Release managed resources allocated after probing
@@ -416,7 +416,7 @@ void devres_release_probe(struct udevice *dev);
  */
 void devres_release_all(struct udevice *dev);
 
-#else /* ! CONFIG_DEVRES */
+#else /* ! DEVRES */
 
 static inline void devres_release_probe(struct udevice *dev)
 {
@@ -426,7 +426,7 @@ static inline void devres_release_all(struct udevice *dev)
 {
 }
 
-#endif /* ! CONFIG_DEVRES */
+#endif /* DEVRES */
 
 static inline int device_notify(const struct udevice *dev, enum event_t type)
 {
diff --git a/include/dm/device.h b/include/dm/device.h
index cb52a0997c8..3d8961f9ac6 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -184,7 +184,7 @@ struct udevice {
 #if CONFIG_IS_ENABLED(OF_REAL)
 	ofnode node_;
 #endif
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
 	struct list_head devres_head;
 #endif
 #if CONFIG_IS_ENABLED(DM_DMA)
diff --git a/include/dm/devres.h b/include/dm/devres.h
index 0ab277ec38e..697534aa5be 100644
--- a/include/dm/devres.h
+++ b/include/dm/devres.h
@@ -30,7 +30,7 @@ struct devres_stats {
 	int total_size;
 };
 
-#ifdef CONFIG_DEVRES
+#if CONFIG_IS_ENABLED(DEVRES)
 
 #ifdef CONFIG_DEBUG_DEVRES
 void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
@@ -207,7 +207,7 @@ void devm_kfree(struct udevice *dev, void *ptr);
 /* Get basic stats on allocations */
 void devres_get_stats(const struct udevice *dev, struct devres_stats *stats);
 
-#else /* ! CONFIG_DEVRES */
+#else /* ! DEVRES */
 
 static inline void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp)
 {
diff --git a/test/dm/Makefile b/test/dm/Makefile
index d46552fbf32..9a1a904d906 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -32,7 +32,7 @@ obj-$(CONFIG_CLK) += clk.o clk_ccf.o
 obj-$(CONFIG_CPU) += cpu.o
 obj-$(CONFIG_CROS_EC) += cros_ec.o
 obj-$(CONFIG_PWM_CROS_EC) += cros_ec_pwm.o
-obj-$(CONFIG_DEVRES) += devres.o
+obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
 obj-$(CONFIG_DMA) += dma.o
 obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o
 obj-$(CONFIG_DM_DSA) += dsa.o
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (6 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 07/10] dm: core: Allow devres to be disabled in SPL Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-04-19 12:50   ` Tom Rini
  2022-03-27 20:26 ` [PATCH 09/10] RFC: dm: add tag support Simon Glass
  2022-03-27 20:26 ` [PATCH 10/10] WIP: dm: core: Add a command to calculate memory usage Simon Glass
  9 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Marek Vasut,
	Pavel Herrmann

When every member of a linker list is aligned by the compiler, we can no
longer rely on the sizeof of the struct to determine the number of
entries.

For example, if the struct size is 0x90 but every entry is aligned to 0xa0
by the compiler, the linker list entries takes more space in memory and
the calculation of the number of entries is incorrect. For example, we may
see 0x12 entries when there are only 0x11.

This is a real problem. There may be a general solution, although I cannot
currently think of one. So far it only bites with OF_PLATDATA_RT which
creates a pointer to each entry of the 'struct udevice' linker_list. This
does not happen without that option, so it only affects SPL.

Work around it by manually calculating the aligned size of struct udevice,
then using that for the n_ent calculation.

Note: the alignment fix to linker list was here:

   0b2fa98aa5e linker_lists: Fix alignment issue

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/device.c | 3 ++-
 drivers/core/root.c   | 8 +++++++-
 include/dm/device.h   | 8 ++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index b7ce8544140..3ab2583df38 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -1186,7 +1186,8 @@ int dev_enable_by_path(const char *path)
 static struct udevice_rt *dev_get_rt(const struct udevice *dev)
 {
 	struct udevice *base = ll_entry_start(struct udevice, udevice);
-	int idx = dev - base;
+	uint each_size = dm_udevice_size();
+	int idx = ((void *)dev - (void *)base) / each_size;
 
 	struct udevice_rt *urt = gd_dm_udevice_rt() + idx;
 
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 8efb4256b27..6d1a4097e14 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -136,12 +136,18 @@ static int dm_setup_inst(void)
 
 	if (CONFIG_IS_ENABLED(OF_PLATDATA_RT)) {
 		struct udevice_rt *urt;
+		void *start, *end;
+		int each_size;
 		void *base;
 		int n_ents;
 		uint size;
 
 		/* Allocate the udevice_rt table */
-		n_ents = ll_entry_count(struct udevice, udevice);
+		each_size = dm_udevice_size();
+		start = ll_entry_start(struct udevice, udevice);
+		end = ll_entry_end(struct udevice, udevice);
+		size = end - start;
+		n_ents = size / each_size;
 		urt = calloc(n_ents, sizeof(struct udevice_rt));
 		if (!urt)
 			return log_msg_ret("urt", -ENOMEM);
diff --git a/include/dm/device.h b/include/dm/device.h
index 3d8961f9ac6..e0f86f5df9f 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -192,6 +192,14 @@ struct udevice {
 #endif
 };
 
+static inline int dm_udevice_size(void)
+{
+	if (CONFIG_IS_ENABLED(OF_PLATDATA_RT))
+		return ALIGN(sizeof(struct udevice), CONFIG_LINKER_LIST_ALIGN);
+
+	return sizeof(struct udevice);
+}
+
 /**
  * struct udevice_rt - runtime information set up by U-Boot
  *
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/10] RFC: dm: add tag support
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (7 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  2022-03-27 20:26 ` [PATCH 10/10] WIP: dm: core: Add a command to calculate memory usage Simon Glass
  9 siblings, 0 replies; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, AKASHI Takahiro, Simon Glass,
	Bin Meng, Dario Binacchi, Marek Vasut, Pavel Herrmann,
	Rasmus Villemoes

From: AKASHI Takahiro <takahiro.akashi@linaro.org>

Note: This patch is *still* pending, so it is included in this series
just to make it work.

With dm-tag feature, any U-Boot subsystem is allowed to associate
arbitrary number of data with a particular udevice. This can been
see as expanding "struct udevice" without modifying the definition.

As a first user, UEFI subsystem makes use of tags to associate
an efi_disk object with a block device.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/core/Makefile             |   2 +-
 drivers/core/root.c               |   2 +
 drivers/core/tag.c                | 139 ++++++++++++++++++++++++++++++
 include/asm-generic/global_data.h |   4 +
 include/dm/tag.h                  | 110 +++++++++++++++++++++++
 5 files changed, 256 insertions(+), 1 deletion(-)
 create mode 100644 drivers/core/tag.c
 create mode 100644 include/dm/tag.h

diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 0cbc3ab217e..7099073a533 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -2,7 +2,7 @@
 #
 # Copyright (c) 2013 Google, Inc
 
-obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o
+obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o
 obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
 obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
 obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)	+= device-remove.o
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 6d1a4097e14..e09c12f4d6e 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -205,6 +205,8 @@ int dm_init(bool of_live)
 			return ret;
 	}
 
+	INIT_LIST_HEAD((struct list_head *)&gd->dmtag_list);
+
 	return 0;
 }
 
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
new file mode 100644
index 00000000000..6829bcd8806
--- /dev/null
+++ b/drivers/core/tag.c
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#include <malloc.h>
+#include <asm/global_data.h>
+#include <dm/tag.h>
+#include <linux/err.h>
+#include <linux/list.h>
+#include <linux/types.h>
+
+struct udevice;
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag >= DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag)
+			return -EEXIST;
+	}
+
+	node = calloc(sizeof(*node), 1);
+	if (!node)
+		return -ENOSPC;
+
+	node->dev = dev;
+	node->tag = tag;
+	node->ptr = ptr;
+	list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list);
+
+	return 0;
+}
+
+int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag >= DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag)
+			return -EEXIST;
+	}
+
+	node = calloc(sizeof(*node), 1);
+	if (!node)
+		return -ENOSPC;
+
+	node->dev = dev;
+	node->tag = tag;
+	node->val = val;
+	list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list);
+
+	return 0;
+}
+
+int dev_tag_get_ptr(struct udevice *dev, enum dm_tag_t tag, void **ptrp)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag >= DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			*ptrp = node->ptr;
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+int dev_tag_get_val(struct udevice *dev, enum dm_tag_t tag, ulong *valp)
+{
+	struct dmtag_node *node;
+
+	if (!dev || tag >= DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			*valp = node->val;
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+int dev_tag_del(struct udevice *dev, enum dm_tag_t tag)
+{
+	struct dmtag_node *node, *tmp;
+
+	if (!dev || tag >= DM_TAG_COUNT)
+		return -EINVAL;
+
+	list_for_each_entry_safe(node, tmp, &gd->dmtag_list, sibling) {
+		if (node->dev == dev && node->tag == tag) {
+			list_del(&node->sibling);
+			free(node);
+
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+int dev_tag_del_all(struct udevice *dev)
+{
+	struct dmtag_node *node, *tmp;
+	bool found = false;
+
+	if (!dev)
+		return -EINVAL;
+
+	list_for_each_entry_safe(node, tmp, &gd->dmtag_list, sibling) {
+		if (node->dev == dev) {
+			list_del(&node->sibling);
+			free(node);
+			found = true;
+		}
+	}
+
+	if (found)
+		return 0;
+
+	return -ENOENT;
+}
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index e49f5bf2f7d..59bd6804368 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -474,6 +474,10 @@ struct global_data {
 	 */
 	struct event_state event_state;
 #endif
+	/**
+	 * @dmtag_list: List of DM tags
+	 */
+	struct list_head dmtag_list;
 };
 #ifndef DO_DEPS_ONLY
 static_assert(sizeof(struct global_data) == GD_SIZE);
diff --git a/include/dm/tag.h b/include/dm/tag.h
new file mode 100644
index 00000000000..54fc31eb153
--- /dev/null
+++ b/include/dm/tag.h
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#ifndef _DM_TAG_H
+#define _DM_TAG_H
+
+#include <linux/list.h>
+#include <linux/types.h>
+
+struct udevice;
+
+enum dm_tag_t {
+	/* EFI_LOADER */
+	DM_TAG_EFI = 0,
+
+	DM_TAG_COUNT,
+};
+
+/**
+ * dmtag_node
+ *
+ * @sibling: List of dm-tag nodes
+ * @dev:     Associated udevice
+ * @tag:     Tag type
+ * @ptr:     Pointer as a value
+ * @val:     Value
+ */
+struct dmtag_node {
+	struct list_head sibling;
+	struct  udevice *dev;
+	enum dm_tag_t tag;
+	union {
+		void *ptr;
+		ulong val;
+	};
+};
+
+/**
+ * dev_tag_set_ptr() - set a tag's value as a pointer
+ * @dev: Device to operate
+ * @tag: Tag type
+ * @ptr: Pointer to set
+ *
+ * Set the value, @ptr, as of @tag associated with the device, @dev
+ *
+ * Return: 0 on success, -ve on error
+ */
+int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr);
+
+/**
+ * dev_tag_set_val() set a tag's value as an integer
+ * @dev: Device to operate
+ * @tag: Tag type
+ * @val: Value to set
+ *
+ * Set the value, @val, as of @tag associated with the device, @dev
+ *
+ * Return: on success, -ve on error
+ */
+int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val);
+
+/**
+ * dev_tag_get_ptr() - get a tag's value as a pointer
+ * @dev: Device to operate
+ * @tag: Tag type
+ * @ptrp: Pointer to tag's value (pointer)
+ *
+ * Get a tag's value as a pointer
+ *
+ * Return: on success, -ve on error
+ */
+int dev_tag_get_ptr(struct udevice *dev, enum dm_tag_t tag, void **ptrp);
+
+/**
+ * dev_tag_get_val() - get a tag's value as an integer
+ * @dev: Device to operate
+ * @tag: Tag type
+ * @valp: Pointer to tag's value (ulong)
+ *
+ * Get a tag's value as an integer
+ *
+ * Return: 0 on success, -ve on error
+ */
+int dev_tag_get_val(struct udevice *dev, enum dm_tag_t tag, ulong *valp);
+
+/**
+ * dev_tag_del() - delete a tag
+ * @dev: Device to operate
+ * @tag: Tag type
+ *
+ * Delete a tag of @tag associated with the device, @dev
+ *
+ * Return: 0 on success, -ve on error
+ */
+int dev_tag_del(struct udevice *dev, enum dm_tag_t tag);
+
+/**
+ * dev_tag_del_all() - delete all tags
+ * @dev: Device to operate
+ *
+ * Delete all the tags associated with the device, @dev
+ *
+ * Return: 0 on success, -ve on error
+ */
+int dev_tag_del_all(struct udevice *dev);
+
+#endif /* _DM_TAG_H */
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/10] WIP: dm: core: Add a command to calculate memory usage
  2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
                   ` (8 preceding siblings ...)
  2022-03-27 20:26 ` [PATCH 09/10] RFC: dm: add tag support Simon Glass
@ 2022-03-27 20:26 ` Simon Glass
  9 siblings, 0 replies; 23+ messages in thread
From: Simon Glass @ 2022-03-27 20:26 UTC (permalink / raw)
  To: U-Boot Mailing List
  Cc: Heinrich Schuchardt, Sean Anderson, Marek Behún,
	U-Boot Custodians, Tom Rini, Simon Glass, Alexandru Gagniuc,
	Marek Vasut, Pali Rohár, Pavel Herrmann, Ricardo Salveti,
	Stefan Roese

Driver model can use a lot of memory, as it is the core of all drivers
and devices in U-Boot. Add a command to show how much is in use, along
with the sizes of various data structures.

This patch can be used to analyse the impact of various potential changes
to driver model for SPL, none of which has been implemented. Most involve
shrinking the size of struct udevice, which is a particular problem on
64-bit machines since their pointers are so unnecessarily large in SPL.

To try this out, build and run on your board. You should see output for
SPL and U-Boot proper, like this:

   Struct sizes: udevice 90, driver 78, uclass 30, uc_driver 78
   Memory: device 11:990, device names 111, uclass a:1e0

   Attached type    Count   Size    Cur   Tags   Save
   ---------------  -----  -----  -----  -----  -----
   plat                 3     e0    990    914     7c (124)
   parent_plat          2     40    990    910     80 (128)
   uclass_plat          1     10    990    90c     84 (132)
   priv                 6    13d    990    920     70 (112)
   parent_priv          0      0    990    908     88 (136)
   uclass_priv          3     38    990    914     7c (124)
   driver_data          0      0    990    908     88 (136)
   uclass               0      0
   Attached total       f    2a5                   37c (892)
   tags                 0      0

   Total size: e15 (3605)

   With tags:       a99 (2713)
   - singly-linked: 901 (2305)
   - driver index:  88a (2186)
   - uclass index:  813 (2067)
   Drop device name (not SRAM): 111 (273)

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/dm.c              | 15 ++++++++-
 common/spl/spl.c      |  9 ++++++
 drivers/core/device.c | 65 ++++++++++++++++++++++++++++++++++++++
 drivers/core/dump.c   | 73 +++++++++++++++++++++++++++++++++++++++++++
 drivers/core/root.c   | 53 +++++++++++++++++++++++++++++++
 drivers/core/tag.c    | 29 +++++++++++++++++
 include/dm/device.h   |  6 ++++
 include/dm/root.h     | 45 ++++++++++++++++++++++++++
 include/dm/tag.h      | 18 ++++++++++-
 include/dm/util.h     |  9 ++++++
 10 files changed, 320 insertions(+), 2 deletions(-)

diff --git a/cmd/dm.c b/cmd/dm.c
index 1dd19fe45b5..ec32a13b177 100644
--- a/cmd/dm.c
+++ b/cmd/dm.c
@@ -64,6 +64,17 @@ static int do_dm_dump_static_driver_info(struct cmd_tbl *cmdtp, int flag, int ar
 	return 0;
 }
 
+static int do_dm_dump_mem(struct cmd_tbl *cmdtp, int flag, int argc,
+			  char *const argv[])
+{
+	struct dm_stats mem;
+
+	dm_get_mem(&mem);
+	dm_dump_mem(&mem);
+
+	return 0;
+}
+
 static struct cmd_tbl test_commands[] = {
 	U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_all, "", ""),
 	U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""),
@@ -71,6 +82,7 @@ static struct cmd_tbl test_commands[] = {
 	U_BOOT_CMD_MKENT(drivers, 1, 1, do_dm_dump_drivers, "", ""),
 	U_BOOT_CMD_MKENT(compat, 1, 1, do_dm_dump_driver_compat, "", ""),
 	U_BOOT_CMD_MKENT(static, 1, 1, do_dm_dump_static_driver_info, "", ""),
+	U_BOOT_CMD_MKENT(mem, 1, 1, do_dm_dump_mem, "", ""),
 };
 
 static __maybe_unused void dm_reloc(void)
@@ -114,5 +126,6 @@ U_BOOT_CMD(
 	"dm devres        Dump list of device resources for each device\n"
 	"dm drivers       Dump list of drivers with uclass and instances\n"
 	"dm compat        Dump list of drivers with compatibility strings\n"
-	"dm static        Dump list of drivers with static platform data"
+	"dm static        Dump list of drivers with static platform data\n"
+	"dm mem           Provide a summary of memory usage"
 );
diff --git a/common/spl/spl.c b/common/spl/spl.c
index b452d4feeb2..3182f774f13 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -32,6 +32,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <dm/root.h>
+#include <dm/util.h>
 #include <linux/compiler.h>
 #include <fdt_support.h>
 #include <bootcount.h>
@@ -743,6 +744,14 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 		}
 	}
 
+	/* Dump drive model states to aid analysis */
+	if (1) {
+		struct dm_stats mem;
+
+		dm_get_mem(&mem);
+		dm_dump_mem(&mem);
+	}
+
 #if CONFIG_IS_ENABLED(BOARD_INIT)
 	spl_board_init();
 #endif
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 3ab2583df38..5d9dca8a81f 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -680,6 +680,71 @@ void *dev_get_parent_priv(const struct udevice *dev)
 	return dm_priv_to_rw(dev->parent_priv_);
 }
 
+void *dev_get_attach(const struct udevice *dev, enum dm_tag_t tag)
+{
+	switch (tag) {
+	case DM_TAG_PLAT:
+		return dev_get_plat(dev);
+	case DM_TAG_PARENT_PLAT:
+		return dev_get_parent_plat(dev);
+	case DM_TAG_UC_PLAT:
+		return dev_get_uclass_plat(dev);
+	case DM_TAG_PRIV:
+		return dev_get_priv(dev);
+	case DM_TAG_PARENT_PRIV:
+		return dev_get_parent_priv(dev);
+	case DM_TAG_UC_PRIV:
+		return dev_get_uclass_priv(dev);
+	default:
+		return NULL;
+	}
+}
+
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag)
+{
+	const struct udevice *parent = dev_get_parent(dev);
+	const struct uclass *uc = dev->uclass;
+	const struct uclass_driver *uc_drv = uc->uc_drv;
+	const struct driver *parent_drv = NULL;
+	int size = 0;
+
+	if (parent)
+		parent_drv = parent->driver;
+
+	switch (tag) {
+	case DM_TAG_PLAT:
+		size = dev->driver->plat_auto;
+		break;
+	case DM_TAG_PARENT_PLAT:
+		if (parent) {
+			size = parent_drv->per_child_plat_auto;
+			if (!size)
+				size = parent->uclass->uc_drv->per_child_plat_auto;
+		}
+		break;
+	case DM_TAG_UC_PLAT:
+		size = uc_drv->per_device_plat_auto;
+		break;
+	case DM_TAG_PRIV:
+		size = dev->driver->priv_auto;
+		break;
+	case DM_TAG_PARENT_PRIV:
+		if (parent) {
+			size = parent_drv->per_child_auto;
+			if (!size)
+				size = parent->uclass->uc_drv->per_child_auto;
+		}
+		break;
+	case DM_TAG_UC_PRIV:
+		size = uc_drv->per_device_auto;
+		break;
+	default:
+		break;
+	}
+
+	return size;
+}
+
 static int device_get_device_tail(struct udevice *dev, int ret,
 				  struct udevice **devp)
 {
diff --git a/drivers/core/dump.c b/drivers/core/dump.c
index f2f9cacc56c..5e51d06787a 100644
--- a/drivers/core/dump.c
+++ b/drivers/core/dump.c
@@ -176,3 +176,76 @@ void dm_dump_static_driver_info(void)
 		       (ulong)map_to_sysmem(entry->plat));
 	}
 }
+
+void dm_dump_mem(struct dm_stats *stats)
+{
+	int total, total_delta;
+	int i;
+
+	/* Support SPL printf() */
+	printf("Struct sizes: udevice %x, driver %x, uclass %x, uc_driver %x\n",
+	       (int)sizeof(struct udevice), (int)sizeof(struct driver),
+	       (int)sizeof(struct uclass), (int)sizeof(struct uclass_driver));
+	printf("Memory: device %x:%x, device names %x, uclass %x:%x\n",
+	       stats->dev_count, stats->dev_size, stats->dev_name_size,
+	       stats->uc_count, stats->uc_size);
+	printf("\n");
+	printf("%-15s  %5s  %5s  %5s  %5s  %5s\n", "Attached type", "Count",
+	       "Size", "Cur", "Tags", "Save");
+	printf("%-15s  %5s  %5s  %5s  %5s  %5s\n", "---------------", "-----",
+	       "-----", "-----", "-----", "-----");
+	total_delta = 0;
+	for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) {
+		int cur_size, new_size, delta;
+
+		cur_size = stats->dev_count * sizeof(struct udevice);
+		new_size = stats->dev_count * (sizeof(struct udevice) -
+			sizeof(void *));
+		/*
+		 * Let's assume we can fit each dmtag_node into 32 bits. We can
+		 * limit the 'tiny tags' feature to SPL with
+		 * CONFIG_SPL_SYS_MALLOC_F_LEN <= 64KB, so needing 14 bits to
+		 * point to anything in that region (with 4-byte alignment).
+		 * So:
+		 *    4 bits for tag
+		 *    14 bits for offset of dev
+		 *    14 bits for offset of data
+		 */
+		new_size += stats->attach_count[i] * sizeof(u32);
+		delta = cur_size - new_size;
+		total_delta += delta;
+		printf("%-16s %5x %6x %6x %6x %6x (%d)\n", tag_get_name(i),
+		       stats->attach_count[i], stats->attach_size[i],
+		       cur_size, new_size, delta > 0 ? delta : 0, delta);
+	}
+	printf("%-16s %5x %6x\n", "uclass", stats->uc_attach_count,
+	       stats->uc_attach_size);
+	printf("%-16s %5x %6x  %5s  %5s  %6x (%d)\n", "Attached total",
+	       stats->attach_count_total + stats->uc_attach_count,
+	       stats->attach_size_total + stats->uc_attach_size, "", "",
+	       total_delta > 0 ? total_delta : 0, total_delta);
+	printf("%-16s %5x %6x\n", "tags", stats->tag_count, stats->tag_size);
+	printf("\n");
+	printf("Total size: %x (%d)\n", stats->total_size, stats->total_size);
+	printf("\n");
+
+	total = stats->total_size;
+	total -= total_delta;
+	printf("With tags:       %x (%d)\n", total, total);
+
+	/* Use singly linked lists in struct udevice (3 nodes in each) */
+	total -= sizeof(void *) * 3 * stats->dev_count;
+	printf("- singly-linked: %x (%d)\n", total, total);
+
+	/* Use an index into the struct_driver list instead of a pointer */
+	total = total + stats->dev_count * (1 - sizeof(void *));
+	printf("- driver index:  %x (%d)\n", total, total);
+
+	/* Same with the uclass */
+	total = total + stats->dev_count * (1 - sizeof(void *));
+	printf("- uclass index:  %x (%d)\n", total, total);
+
+	/* Drop the device name */
+	printf("Drop device name (not SRAM): %x (%d)\n", stats->dev_name_size,
+	       stats->dev_name_size);
+}
diff --git a/drivers/core/root.c b/drivers/core/root.c
index e09c12f4d6e..4f83cb2b557 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -427,6 +427,59 @@ void dm_get_stats(int *device_countp, int *uclass_countp)
 	*uclass_countp = uclass_get_count();
 }
 
+void dev_collect_stats(struct dm_stats *stats, const struct udevice *parent)
+{
+	const struct udevice *dev;
+	int i;
+
+	stats->dev_count++;
+	stats->dev_size += sizeof(struct udevice);
+	stats->dev_name_size += strlen(parent->name) + 1;
+	for (i = 0; i < DM_TAG_ATTACH_COUNT; i++) {
+		int size = dev_get_attach_size(parent, i);
+
+		if (size ||
+		    (i == DM_TAG_DRIVER_DATA && parent->driver_data)) {
+			stats->attach_count[i]++;
+			stats->attach_size[i] += size;
+			stats->attach_count_total++;
+			stats->attach_size_total += size;
+		}
+	}
+
+	list_for_each_entry(dev, &parent->child_head, sibling_node)
+		dev_collect_stats(stats, dev);
+}
+
+void uclass_collect_stats(struct dm_stats *stats)
+{
+	struct uclass *uc;
+
+	list_for_each_entry(uc, gd->uclass_root, sibling_node) {
+		int size;
+
+		stats->uc_count++;
+		stats->uc_size += sizeof(struct uclass);
+		size = uc->uc_drv->priv_auto;
+		if (size) {
+			stats->uc_attach_count++;
+			stats->uc_attach_size += size;
+		}
+	}
+}
+
+void dm_get_mem(struct dm_stats *stats)
+{
+	memset(stats, '\0', sizeof(*stats));
+	dev_collect_stats(stats, gd->dm_root);
+	uclass_collect_stats(stats);
+	dev_tag_collect_stats(stats);
+
+	stats->total_size = stats->dev_size + stats->uc_size +
+		stats->attach_size_total + stats->uc_attach_size +
+		stats->tag_size;
+}
+
 #ifdef CONFIG_ACPIGEN
 static int root_acpi_get_name(const struct udevice *dev, char *out_name)
 {
diff --git a/drivers/core/tag.c b/drivers/core/tag.c
index 6829bcd8806..d8042fa2c5b 100644
--- a/drivers/core/tag.c
+++ b/drivers/core/tag.c
@@ -6,6 +6,7 @@
 
 #include <malloc.h>
 #include <asm/global_data.h>
+#include <dm/root.h>
 #include <dm/tag.h>
 #include <linux/err.h>
 #include <linux/list.h>
@@ -15,6 +16,24 @@ struct udevice;
 
 DECLARE_GLOBAL_DATA_PTR;
 
+static const char *const tag_name[] = {
+	[DM_TAG_PLAT]		= "plat",
+	[DM_TAG_PARENT_PLAT]	= "parent_plat",
+	[DM_TAG_UC_PLAT]	= "uclass_plat",
+
+	[DM_TAG_PRIV]		= "priv",
+	[DM_TAG_PARENT_PRIV]	= "parent_priv",
+	[DM_TAG_UC_PRIV]	= "uclass_priv",
+	[DM_TAG_DRIVER_DATA]	= "driver_data",
+
+	[DM_TAG_EFI]		= "efi",
+};
+
+const char *tag_get_name(enum dm_tag_t tag)
+{
+	return tag_name[tag];
+}
+
 int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
 {
 	struct dmtag_node *node;
@@ -137,3 +156,13 @@ int dev_tag_del_all(struct udevice *dev)
 
 	return -ENOENT;
 }
+
+void dev_tag_collect_stats(struct dm_stats *stats)
+{
+	struct dmtag_node *node;
+
+	list_for_each_entry(node, &gd->dmtag_list, sibling) {
+		stats->tag_count++;
+		stats->tag_size += sizeof(struct dmtag_node);
+	}
+}
diff --git a/include/dm/device.h b/include/dm/device.h
index e0f86f5df9f..d8193900b12 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -11,6 +11,7 @@
 #define _DM_DEVICE_H
 
 #include <dm/ofnode.h>
+#include <dm/tag.h>
 #include <dm/uclass-id.h>
 #include <fdtdec.h>
 #include <linker_lists.h>
@@ -18,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/printk.h>
 
+struct dm_stats;
 struct driver_info;
 
 /* Driver is active (probed). Cleared when it is removed */
@@ -543,6 +545,10 @@ void *dev_get_parent_priv(const struct udevice *dev);
  */
 void *dev_get_uclass_priv(const struct udevice *dev);
 
+void *dev_get_attach(const struct udevice *dev, enum dm_tag_t tag);
+
+int dev_get_attach_size(const struct udevice *dev, enum dm_tag_t tag);
+
 /**
  * dev_get_parent() - Get the parent of a device
  *
diff --git a/include/dm/root.h b/include/dm/root.h
index e888fb993c0..382f83c7f5b 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -9,11 +9,49 @@
 #ifndef _DM_ROOT_H_
 #define _DM_ROOT_H_
 
+#include <dm/tag.h>
+
 struct udevice;
 
 /* Head of the uclass list if CONFIG_OF_PLATDATA_INST is enabled */
 extern struct list_head uclass_head;
 
+/**
+ * struct dm_stats - Information about driver model memory usage
+ *
+ * @total_size: All data
+ * @dev_count: Number of devices
+ * @dev_size: Size of all devices (just the struct udevice)
+ * @dev_name_size: Bytes used by device names
+ * @uc_count: Number of uclasses
+ * @uc_size: Size of all uclasses (just the struct uclass)
+ * @tag_count: Number of tags
+ * @tag_size: Bytes used by all tags
+ * @uc_attach_count: Number of uclasses with attached data (priv)
+ * @uc_attach_size: Total size of that attached data
+ * @attach_count_total: Total number of attached data items for all udevices and
+ *	uclasses
+ * @attach_size_total: Total number of bytes of attached data
+ * @attach_count: Number of devices with attached, for each type
+ * @attach_size: Total number of bytes of attached data, for each type
+ */
+struct dm_stats {
+	int total_size;
+	int dev_count;
+	int dev_size;
+	int dev_name_size;
+	int uc_count;
+	int uc_size;
+	int tag_count;
+	int tag_size;
+	int uc_attach_count;
+	int uc_attach_size;
+	int attach_count_total;
+	int attach_size_total;
+	int attach_count[DM_TAG_ATTACH_COUNT];
+	int attach_size[DM_TAG_ATTACH_COUNT];
+};
+
 /**
  * dm_root() - Return pointer to the top of the driver tree
  *
@@ -141,4 +179,11 @@ static inline int dm_remove_devices_flags(uint flags) { return 0; }
  */
 void dm_get_stats(int *device_countp, int *uclass_countp);
 
+/**
+ * dm_get_mem() - Get stats on memory usage in driver model
+ *
+ * @mem: Place to put the information
+ */
+void dm_get_mem(struct dm_stats *stats);
+
 #endif
diff --git a/include/dm/tag.h b/include/dm/tag.h
index 54fc31eb153..3b1ea7576ee 100644
--- a/include/dm/tag.h
+++ b/include/dm/tag.h
@@ -10,11 +10,23 @@
 #include <linux/list.h>
 #include <linux/types.h>
 
+struct dm_stats;
 struct udevice;
 
 enum dm_tag_t {
+	/* Types of info that can be attached to devices */
+	DM_TAG_PLAT,
+	DM_TAG_PARENT_PLAT,
+	DM_TAG_UC_PLAT,
+
+	DM_TAG_PRIV,
+	DM_TAG_PARENT_PRIV,
+	DM_TAG_UC_PRIV,
+	DM_TAG_DRIVER_DATA,
+	DM_TAG_ATTACH_COUNT,
+
 	/* EFI_LOADER */
-	DM_TAG_EFI = 0,
+	DM_TAG_EFI = DM_TAG_ATTACH_COUNT,
 
 	DM_TAG_COUNT,
 };
@@ -107,4 +119,8 @@ int dev_tag_del(struct udevice *dev, enum dm_tag_t tag);
  */
 int dev_tag_del_all(struct udevice *dev);
 
+const char *tag_get_name(enum dm_tag_t tag);
+
+void dev_tag_collect_stats(struct dm_stats *stats);
+
 #endif /* _DM_TAG_H */
diff --git a/include/dm/util.h b/include/dm/util.h
index 4428f045b72..b346ecfe042 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -6,6 +6,8 @@
 #ifndef __DM_UTIL_H
 #define __DM_UTIL_H
 
+struct dm_stats;
+
 #if CONFIG_IS_ENABLED(DM_WARN)
 #define dm_warn(fmt...) log(LOGC_DM, LOGL_WARNING, ##fmt)
 #else
@@ -48,6 +50,13 @@ void dm_dump_driver_compat(void);
 /* Dump out a list of drivers with static platform data */
 void dm_dump_static_driver_info(void);
 
+/**
+ * dm_dump_mem() - Dump stats on memory usage in driver model
+ *
+ * @mem: Stats to dump
+ */
+void dm_dump_mem(struct dm_stats *stats);
+
 #if CONFIG_IS_ENABLED(OF_PLATDATA_INST) && CONFIG_IS_ENABLED(READ_ONLY)
 void *dm_priv_to_rw(void *priv);
 #else
-- 
2.35.1.1021.g381101b075-goog


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
  2022-03-27 20:26 ` [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build Simon Glass
@ 2022-03-31 10:29   ` Andrew Scull
  2022-04-11 21:46     ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Andrew Scull @ 2022-03-31 10:29 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Tom Rini, Bin Meng,
	Ilias Apalodimas, Joel Peshkin, Masahiro Yamada,
	Patrick Delaunay, Ye Li

On Sun, 27 Mar 2022 at 21:27, Simon Glass <sjg@chromium.org> wrote:
>
> LTO (Link-Time Optimisation) is an very useful feature which can
> significantly reduce the size of U-Boot binaries. So far it has been
> made available for selected ARM boards and sandbox.
>
> However, incremental builds are much slower when LTO is used. For example,
> an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> seconds with LTO enabled.

This is something that has been bothering me too. I'd resorted to adding
`# CONFIG_LTO is not set` to sandbox_defconfig because I couldn't work
out how to stop .config being regenerated each build. But then the
diff gets in the way and is difficult to manage.

> Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> development if needed, for faster builds.

If you go the build parameter route rather than config route to
address this, could the flag be consistent e.g. with NO_SDL=1, which
would mean NO_LTO=1?

> Add some documentation about LTO while we are here.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  Makefile                           | 18 +++++++++++++-----
>  arch/arm/config.mk                 |  4 ++--
>  arch/arm/include/asm/global_data.h |  2 +-
>  doc/build/gcc.rst                  | 17 +++++++++++++++++
>  scripts/Makefile.spl               |  2 +-
>  5 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 06572ac07ee..c9585ddebfc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -434,6 +434,9 @@ KBUILD_CFLAGS       += -fshort-wchar -fno-strict-aliasing
>  KBUILD_AFLAGS   := -D__ASSEMBLY__
>  KBUILD_LDFLAGS  :=
>
> +# Set this to "n" use of LTO for this build, e.g. LTO_BUILD=n
> +LTO_BUILD      ?= y
> +
>  ifeq ($(cc-name),clang)
>  ifneq ($(CROSS_COMPILE),)
>  CLANG_TARGET   := --target=$(notdir $(CROSS_COMPILE:%-=%))
> @@ -643,6 +646,11 @@ export CFLAGS_EFI  # Compiler flags to add when building EFI app
>  export CFLAGS_NON_EFI  # Compiler flags to remove when building EFI app
>  export EFI_TARGET      # binutils target if EFI is natively supported
>
> +export LTO_ENABLE
> +
> +# This is y if LTO is enabled for this build
> +LTO_ENABLE=$(if $(CONFIG_LTO),$(LTO_BUILD),)
> +
>  # If board code explicitly specified LDSCRIPT or CONFIG_SYS_LDSCRIPT, use
>  # that (or fail if absent).  Otherwise, search for a linker script in a
>  # standard location.
> @@ -690,16 +698,16 @@ endif
>  LTO_CFLAGS :=
>  LTO_FINAL_LDFLAGS :=
>  export LTO_CFLAGS LTO_FINAL_LDFLAGS
> -ifdef CONFIG_LTO
> +ifeq ($(LTO_ENABLE),y)
>         ifeq ($(cc-name),clang)
> -               LTO_CFLAGS              += -flto
> +               LTO_CFLAGS              += -DLTO_ENABLE -flto
>                 LTO_FINAL_LDFLAGS       += -flto
>
>                 AR                      = $(shell $(CC) -print-prog-name=llvm-ar)
>                 NM                      = $(shell $(CC) -print-prog-name=llvm-nm)
>         else
>                 NPROC                   := $(shell nproc 2>/dev/null || echo 1)
> -               LTO_CFLAGS              += -flto=$(NPROC)
> +               LTO_CFLAGS              += -DLTO_ENABLE -flto=$(NPROC)
>                 LTO_FINAL_LDFLAGS       += -fuse-linker-plugin -flto=$(NPROC)
>
>                 # use plugin aware tools
> @@ -1740,7 +1748,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(ARCH)/Makefile.postlink)
>
>  # Generate linker list symbols references to force compiler to not optimize
>  # them away when compiling with LTO
> -ifdef CONFIG_LTO
> +ifeq ($(LTO_ENABLE),y)
>  u-boot-keep-syms-lto := keep-syms-lto.o
>  u-boot-keep-syms-lto_c := $(patsubst %.o,%.c,$(u-boot-keep-syms-lto))
>
> @@ -1762,7 +1770,7 @@ endif
>
>  # Rule to link u-boot
>  # May be overridden by arch/$(ARCH)/config.mk
> -ifdef CONFIG_LTO
> +ifeq ($(LTO_ENABLE),y)
>  quiet_cmd_u-boot__ ?= LTO     $@
>        cmd_u-boot__ ?=                                                          \
>                 $(CC) -nostdlib -nostartfiles                                   \
> diff --git a/arch/arm/config.mk b/arch/arm/config.mk
> index b107b1af27a..065dbec4064 100644
> --- a/arch/arm/config.mk
> +++ b/arch/arm/config.mk
> @@ -15,11 +15,11 @@ CFLAGS_NON_EFI := -fno-pic -ffixed-r9 -ffunction-sections -fdata-sections \
>                   -fstack-protector-strong
>  CFLAGS_EFI := -fpic -fshort-wchar
>
> -ifneq ($(CONFIG_LTO)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
> +ifneq ($(LTO_ENABLE)$(CONFIG_USE_PRIVATE_LIBGCC),yy)
>  LDFLAGS_FINAL += --gc-sections
>  endif
>
> -ifndef CONFIG_LTO
> +ifneq ($(LTO_ENABLE),y)
>  PLATFORM_RELFLAGS += -ffunction-sections -fdata-sections
>  endif
>
> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
> index 085e12b5d4d..b255b195aa0 100644
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
> @@ -98,7 +98,7 @@ struct arch_global_data {
>
>  #include <asm-generic/global_data.h>
>
> -#if defined(__clang__) || defined(CONFIG_LTO)
> +#if defined(__clang__) || defined(LTO_ENABLE)
>
>  #define DECLARE_GLOBAL_DATA_PTR
>  #define gd     get_gd()
> diff --git a/doc/build/gcc.rst b/doc/build/gcc.rst
> index 470a7aa3498..792be176aad 100644
> --- a/doc/build/gcc.rst
> +++ b/doc/build/gcc.rst
> @@ -152,6 +152,23 @@ of dtc is new enough. It also makes sure that pylibfdt is present, if needed
>  Note that the :doc:`tools` are always built with the included version of libfdt
>  so it is not possible to build U-Boot tools with a system libfdt, at present.
>
> +Link-time optimisation (LTO)
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +U-Boot supports link-time optimisation which can reduce the size of the final
> +U-Boot binaries, particularly with SPL.
> +
> +At present this can be enabled by ARM boards by adding `CONFIG_LTO=y` into the
> +defconfig file. Other architectures are not supported. LTO is enabled by default
> +for sandbox.
> +
> +This does incur a link-time penalty of several seconds. For faster incremental
> +builds during development, you can disable it by setting `LTO_BUILD` to `n`.
> +
> +.. code-block:: bash
> +
> +    LTO_BUILD=n make
> +
>  Other build targets
>  ~~~~~~~~~~~~~~~~~~~
>
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 83a95ee4aa2..e3ca69c4449 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -472,7 +472,7 @@ endif
>
>  # Rule to link u-boot-spl
>  # May be overridden by arch/$(ARCH)/config.mk
> -ifdef CONFIG_LTO
> +ifeq ($(LTO_ENABLE),y)
>  quiet_cmd_u-boot-spl ?= LTO     $@
>        cmd_u-boot-spl ?= \
>         (                                                                       \
> --
> 2.35.1.1021.g381101b075-goog
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
  2022-03-31 10:29   ` Andrew Scull
@ 2022-04-11 21:46     ` Simon Glass
  2022-04-11 21:53       ` Tom Rini
  2022-05-15 18:52       ` Andrew Scull
  0 siblings, 2 replies; 23+ messages in thread
From: Simon Glass @ 2022-04-11 21:46 UTC (permalink / raw)
  To: Andrew Scull
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Tom Rini, Bin Meng,
	Ilias Apalodimas, Joel Peshkin, Masahiro Yamada,
	Patrick Delaunay, Ye Li

Hi Andrew,

On Thu, 31 Mar 2022 at 04:29, Andrew Scull <ascull@google.com> wrote:
>
> On Sun, 27 Mar 2022 at 21:27, Simon Glass <sjg@chromium.org> wrote:
> >
> > LTO (Link-Time Optimisation) is an very useful feature which can
> > significantly reduce the size of U-Boot binaries. So far it has been
> > made available for selected ARM boards and sandbox.
> >
> > However, incremental builds are much slower when LTO is used. For example,
> > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > seconds with LTO enabled.
>
> This is something that has been bothering me too. I'd resorted to adding
> `# CONFIG_LTO is not set` to sandbox_defconfig because I couldn't work
> out how to stop .config being regenerated each build. But then the
> diff gets in the way and is difficult to manage.
>
> > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > development if needed, for faster builds.
>
> If you go the build parameter route rather than config route to
> address this, could the flag be consistent e.g. with NO_SDL=1, which
> would mean NO_LTO=1?

That seems OK to me. Would you like to do that and send a new version?

Here is a better version of the patch to use as a starting point:

https://source.denx.de/u-boot/custodians/u-boot-dm/-/commit/2d4ebfb0e8e86a1b347e6cbd4dbec9a4eb745bd5

Tom, please consider dropping your objection to this. I have already
shown that it makes a dramatic difference to incremental build time
for any board...

>
> > Add some documentation about LTO while we are here.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  Makefile                           | 18 +++++++++++++-----
> >  arch/arm/config.mk                 |  4 ++--
> >  arch/arm/include/asm/global_data.h |  2 +-
> >  doc/build/gcc.rst                  | 17 +++++++++++++++++
> >  scripts/Makefile.spl               |  2 +-
> >  5 files changed, 34 insertions(+), 9 deletions(-)
> >

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
  2022-04-11 21:46     ` Simon Glass
@ 2022-04-11 21:53       ` Tom Rini
  2022-05-15 18:52       ` Andrew Scull
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-11 21:53 UTC (permalink / raw)
  To: Simon Glass
  Cc: Andrew Scull, U-Boot Mailing List, Heinrich Schuchardt,
	Sean Anderson, Marek Behún, U-Boot Custodians, Bin Meng,
	Ilias Apalodimas, Joel Peshkin, Masahiro Yamada,
	Patrick Delaunay, Ye Li

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

On Mon, Apr 11, 2022 at 03:46:04PM -0600, Simon Glass wrote:
> Hi Andrew,
> 
> On Thu, 31 Mar 2022 at 04:29, Andrew Scull <ascull@google.com> wrote:
> >
> > On Sun, 27 Mar 2022 at 21:27, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > significantly reduce the size of U-Boot binaries. So far it has been
> > > made available for selected ARM boards and sandbox.
> > >
> > > However, incremental builds are much slower when LTO is used. For example,
> > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > seconds with LTO enabled.
> >
> > This is something that has been bothering me too. I'd resorted to adding
> > `# CONFIG_LTO is not set` to sandbox_defconfig because I couldn't work
> > out how to stop .config being regenerated each build. But then the
> > diff gets in the way and is difficult to manage.
> >
> > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > development if needed, for faster builds.
> >
> > If you go the build parameter route rather than config route to
> > address this, could the flag be consistent e.g. with NO_SDL=1, which
> > would mean NO_LTO=1?
> 
> That seems OK to me. Would you like to do that and send a new version?
> 
> Here is a better version of the patch to use as a starting point:
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/commit/2d4ebfb0e8e86a1b347e6cbd4dbec9a4eb745bd5
> 
> Tom, please consider dropping your objection to this. I have already
> shown that it makes a dramatic difference to incremental build time
> for any board...

Yeah, OK, reworked to be consistent with now we do SDL is workable.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/10] dm: core: Allow devres to be disabled in SPL
  2022-03-27 20:26 ` [PATCH 07/10] dm: core: Allow devres to be disabled in SPL Simon Glass
@ 2022-04-12 19:07   ` Angus Ainslie
  2022-04-19 12:49   ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Angus Ainslie @ 2022-04-12 19:07 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Tom Rini, Marek Vasut,
	Pavel Herrmann

On 2022-03-27 13:26, Simon Glass wrote:
> At present if devres is enabled in U-Boot proper it is enabled in SPL.
> We don't normally want it there, so disable it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Tested-by: Angus Ainslie <angus@akkea.ca>

> ---
> 
>  drivers/core/Makefile        | 2 +-
>  drivers/core/device.c        | 2 +-
>  include/dm/device-internal.h | 6 +++---
>  include/dm/device.h          | 2 +-
>  include/dm/devres.h          | 4 ++--
>  test/dm/Makefile             | 2 +-
>  6 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index 5edd4e41357..0cbc3ab217e 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -4,7 +4,7 @@
> 
>  obj-y	+= device.o fdtaddr.o lists.o root.o uclass.o util.o
>  obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o
> -obj-$(CONFIG_DEVRES) += devres.o
> +obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
>  obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE)	+= device-remove.o
>  obj-$(CONFIG_$(SPL_)SIMPLE_BUS)	+= simple-bus.o
>  obj-$(CONFIG_SIMPLE_PM_BUS)	+= simple-pm-bus.o
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 1b356f12dd8..b7ce8544140 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -68,7 +68,7 @@ static int device_bind_common(struct udevice
> *parent, const struct driver *drv,
>  	INIT_LIST_HEAD(&dev->sibling_node);
>  	INIT_LIST_HEAD(&dev->child_head);
>  	INIT_LIST_HEAD(&dev->uclass_node);
> -#ifdef CONFIG_DEVRES
> +#if CONFIG_IS_ENABLED(DEVRES)
>  	INIT_LIST_HEAD(&dev->devres_head);
>  #endif
>  	dev_set_plat(dev, plat);
> diff --git a/include/dm/device-internal.h 
> b/include/dm/device-internal.h
> index c420726287e..2fc41f31f5a 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -396,7 +396,7 @@ fdt_addr_t simple_bus_translate(struct udevice
> *dev, fdt_addr_t addr);
>  #define DM_UCLASS_ROOT_S_NON_CONST	(((gd_t *)gd)->uclass_root_s)
> 
>  /* device resource management */
> -#ifdef CONFIG_DEVRES
> +#if CONFIG_IS_ENABLED(DEVRES)
> 
>  /**
>   * devres_release_probe - Release managed resources allocated after 
> probing
> @@ -416,7 +416,7 @@ void devres_release_probe(struct udevice *dev);
>   */
>  void devres_release_all(struct udevice *dev);
> 
> -#else /* ! CONFIG_DEVRES */
> +#else /* ! DEVRES */
> 
>  static inline void devres_release_probe(struct udevice *dev)
>  {
> @@ -426,7 +426,7 @@ static inline void devres_release_all(struct 
> udevice *dev)
>  {
>  }
> 
> -#endif /* ! CONFIG_DEVRES */
> +#endif /* DEVRES */
> 
>  static inline int device_notify(const struct udevice *dev, enum 
> event_t type)
>  {
> diff --git a/include/dm/device.h b/include/dm/device.h
> index cb52a0997c8..3d8961f9ac6 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -184,7 +184,7 @@ struct udevice {
>  #if CONFIG_IS_ENABLED(OF_REAL)
>  	ofnode node_;
>  #endif
> -#ifdef CONFIG_DEVRES
> +#if CONFIG_IS_ENABLED(DEVRES)
>  	struct list_head devres_head;
>  #endif
>  #if CONFIG_IS_ENABLED(DM_DMA)
> diff --git a/include/dm/devres.h b/include/dm/devres.h
> index 0ab277ec38e..697534aa5be 100644
> --- a/include/dm/devres.h
> +++ b/include/dm/devres.h
> @@ -30,7 +30,7 @@ struct devres_stats {
>  	int total_size;
>  };
> 
> -#ifdef CONFIG_DEVRES
> +#if CONFIG_IS_ENABLED(DEVRES)
> 
>  #ifdef CONFIG_DEBUG_DEVRES
>  void *__devres_alloc(dr_release_t release, size_t size, gfp_t gfp,
> @@ -207,7 +207,7 @@ void devm_kfree(struct udevice *dev, void *ptr);
>  /* Get basic stats on allocations */
>  void devres_get_stats(const struct udevice *dev, struct devres_stats 
> *stats);
> 
> -#else /* ! CONFIG_DEVRES */
> +#else /* ! DEVRES */
> 
>  static inline void *devres_alloc(dr_release_t release, size_t size, 
> gfp_t gfp)
>  {
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index d46552fbf32..9a1a904d906 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -32,7 +32,7 @@ obj-$(CONFIG_CLK) += clk.o clk_ccf.o
>  obj-$(CONFIG_CPU) += cpu.o
>  obj-$(CONFIG_CROS_EC) += cros_ec.o
>  obj-$(CONFIG_PWM_CROS_EC) += cros_ec_pwm.o
> -obj-$(CONFIG_DEVRES) += devres.o
> +obj-$(CONFIG_$(SPL_TPL_)DEVRES) += devres.o
>  obj-$(CONFIG_DMA) += dma.o
>  obj-$(CONFIG_VIDEO_MIPI_DSI) += dsi_host.o
>  obj-$(CONFIG_DM_DSA) += dsa.o

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/10] sandbox: Correct loss of early output in SPL
  2022-03-27 20:26 ` [PATCH 02/10] sandbox: Correct loss of early output in SPL Simon Glass
@ 2022-04-19 12:49   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians

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

On Sun, Mar 27, 2022 at 02:26:14PM -0600, Simon Glass wrote:

> At present fputc() is used before the console is available, then write()
> is used. These are not compatible. Since fputc() buffers internally it is
> better to use the write(), so that a partial line is immediately
> displayed.
> 
> This has a slight effect on performance, but we are already using write()
> for the vast majority of the output with no obvious impacts.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/10] Makefile: Drop a stale comment about linking
  2022-03-27 20:26 ` [PATCH 03/10] Makefile: Drop a stale comment about linking Simon Glass
@ 2022-04-19 12:49   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Masahiro Yamada

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

On Sun, Mar 27, 2022 at 02:26:15PM -0600, Simon Glass wrote:

> The bug mentioned here is fixed, so drop the comment.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk
  2022-03-27 20:26 ` [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk Simon Glass
@ 2022-04-19 12:49   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Masahiro Yamada

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

On Sun, Mar 27, 2022 at 02:26:16PM -0600, Simon Glass wrote:

> This makes it impossible to change them elsewhere. The default value is
> 'empty' anyway, so just drop it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/10] sandbox: Allow link flags to be given
  2022-03-27 20:26 ` [PATCH 05/10] sandbox: Allow link flags to be given Simon Glass
@ 2022-04-19 12:49   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians

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

On Sun, Mar 27, 2022 at 02:26:17PM -0600, Simon Glass wrote:

> At present the link flags are not used for sandbox. Update the command
> line to use them.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary
  2022-03-27 20:26 ` [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary Simon Glass
@ 2022-04-19 12:49   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians

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

On Sun, Mar 27, 2022 at 02:26:18PM -0600, Simon Glass wrote:

> Use this larger boundary to ensure that linker lists at least start on the
> maximum possible alignment boundary. See also the CONFIG_LINKER_LIST_ALIGN
> setting, but that is host-arch-specific, so it seems better to use the
> largest value for every host architecture.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/10] dm: core: Allow devres to be disabled in SPL
  2022-03-27 20:26 ` [PATCH 07/10] dm: core: Allow devres to be disabled in SPL Simon Glass
  2022-04-12 19:07   ` Angus Ainslie
@ 2022-04-19 12:49   ` Tom Rini
  1 sibling, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:49 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Marek Vasut, Pavel Herrmann

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

On Sun, Mar 27, 2022 at 02:26:19PM -0600, Simon Glass wrote:

> At present if devres is enabled in U-Boot proper it is enabled in SPL.
> We don't normally want it there, so disable it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Tested-by: Angus Ainslie <angus@akkea.ca>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists
  2022-03-27 20:26 ` [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists Simon Glass
@ 2022-04-19 12:50   ` Tom Rini
  0 siblings, 0 replies; 23+ messages in thread
From: Tom Rini @ 2022-04-19 12:50 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Marek Vasut, Pavel Herrmann

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

On Sun, Mar 27, 2022 at 02:26:20PM -0600, Simon Glass wrote:

> When every member of a linker list is aligned by the compiler, we can no
> longer rely on the sizeof of the struct to determine the number of
> entries.
> 
> For example, if the struct size is 0x90 but every entry is aligned to 0xa0
> by the compiler, the linker list entries takes more space in memory and
> the calculation of the number of entries is incorrect. For example, we may
> see 0x12 entries when there are only 0x11.
> 
> This is a real problem. There may be a general solution, although I cannot
> currently think of one. So far it only bites with OF_PLATDATA_RT which
> creates a pointer to each entry of the 'struct udevice' linker_list. This
> does not happen without that option, so it only affects SPL.
> 
> Work around it by manually calculating the aligned size of struct udevice,
> then using that for the n_ent calculation.
> 
> Note: the alignment fix to linker list was here:
> 
>    0b2fa98aa5e linker_lists: Fix alignment issue
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build
  2022-04-11 21:46     ` Simon Glass
  2022-04-11 21:53       ` Tom Rini
@ 2022-05-15 18:52       ` Andrew Scull
  1 sibling, 0 replies; 23+ messages in thread
From: Andrew Scull @ 2022-05-15 18:52 UTC (permalink / raw)
  To: Simon Glass
  Cc: U-Boot Mailing List, Heinrich Schuchardt, Sean Anderson,
	Marek Behún, U-Boot Custodians, Tom Rini, Bin Meng,
	Ilias Apalodimas, Joel Peshkin, Masahiro Yamada,
	Patrick Delaunay, Ye Li

On Mon, 11 Apr 2022 at 22:46, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Andrew,
>
> On Thu, 31 Mar 2022 at 04:29, Andrew Scull <ascull@google.com> wrote:
> >
> > On Sun, 27 Mar 2022 at 21:27, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > LTO (Link-Time Optimisation) is an very useful feature which can
> > > significantly reduce the size of U-Boot binaries. So far it has been
> > > made available for selected ARM boards and sandbox.
> > >
> > > However, incremental builds are much slower when LTO is used. For example,
> > > an incremental build of sandbox takes 2.1 seconds on my machine, but 6.7
> > > seconds with LTO enabled.
> >
> > This is something that has been bothering me too. I'd resorted to adding
> > `# CONFIG_LTO is not set` to sandbox_defconfig because I couldn't work
> > out how to stop .config being regenerated each build. But then the
> > diff gets in the way and is difficult to manage.
> >
> > > Add a LTO_BUILD=n parameter to the build, so it can be disabled during
> > > development if needed, for faster builds.
> >
> > If you go the build parameter route rather than config route to
> > address this, could the flag be consistent e.g. with NO_SDL=1, which
> > would mean NO_LTO=1?
>
> That seems OK to me. Would you like to do that and send a new version?
>
> Here is a better version of the patch to use as a starting point:
>
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/commit/2d4ebfb0e8e86a1b347e6cbd4dbec9a4eb745bd5

I realised my make command was rebuilding the defconfig each time
which, unsurprisingly, was regenerating the .config file. Now I can
unset CONFIG_LTO in .config and have it persist for incremental
builds, which is a pattern I've also used when working on Linux, and
doesn't need an alternate toggle in the makefiles to achieve the same
result. My preference is to use .config for the time being.

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2022-05-15 18:53 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-27 20:26 [PATCH 00/10] dm: Experiments for reducing SPL memory usage Simon Glass
2022-03-27 20:26 ` [PATCH 01/10] Makefile: v2 Allow LTO to be disabled for a build Simon Glass
2022-03-31 10:29   ` Andrew Scull
2022-04-11 21:46     ` Simon Glass
2022-04-11 21:53       ` Tom Rini
2022-05-15 18:52       ` Andrew Scull
2022-03-27 20:26 ` [PATCH 02/10] sandbox: Correct loss of early output in SPL Simon Glass
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 03/10] Makefile: Drop a stale comment about linking Simon Glass
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 04/10] Makefile: Avoid resetting link flags in config.mk Simon Glass
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 05/10] sandbox: Allow link flags to be given Simon Glass
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 06/10] sandbox: Align linker lists to a 32-byte boundary Simon Glass
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 07/10] dm: core: Allow devres to be disabled in SPL Simon Glass
2022-04-12 19:07   ` Angus Ainslie
2022-04-19 12:49   ` Tom Rini
2022-03-27 20:26 ` [PATCH 08/10] dm: core: Deal with a wrinkle with linker lists Simon Glass
2022-04-19 12:50   ` Tom Rini
2022-03-27 20:26 ` [PATCH 09/10] RFC: dm: add tag support Simon Glass
2022-03-27 20:26 ` [PATCH 10/10] WIP: dm: core: Add a command to calculate memory usage Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.