[PATCH 00/10] dm: Experiments for reducing SPL memory usage
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-sj... [2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html#bu...
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
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 ?= \ ( \
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
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/2d4ebfb0e8e86a1b...
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
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/2d4ebfb0e8e86a1b...
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.
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/2d4ebfb0e8e86a1b...
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.
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)
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!
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) \
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!
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 :=
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!
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)) \
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!
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*))); }
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!
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
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
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!
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 *
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!
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 */
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
participants (4)
-
Andrew Scull -
Angus Ainslie -
Simon Glass -
Tom Rini