[U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 Instead of using CONFIG_SPL_MAX_SIZE with an estimate of the FDT size introduce a new test in scripts/Makefile.spl. --- Kconfig | 8 ++++++++ configs/tinker-rk3288_defconfig | 1 + scripts/Makefile.spl | 16 ++++++++++++++++ 3 files changed, 25 insertions(+)
diff --git a/Kconfig b/Kconfig index 512c7beb89..7cce53da74 100644 --- a/Kconfig +++ b/Kconfig @@ -172,6 +172,14 @@ config TPL_SYS_MALLOC_F_LEN particular needs this to operate, so that it can allocate the initial serial device and any others that are needed.
+config SPL_WITH_DTB_SIZE_LIMIT + int "Maximum size of SPL image including device tree" + depends on SPL + default 0 + help + Specifies the maximum length of the U-Boot SPL image including the + device tree. If this value is zero, it is ignored. + menuconfig EXPERT bool "Configure standard U-Boot features (expert users)" default y diff --git a/configs/tinker-rk3288_defconfig b/configs/tinker-rk3288_defconfig index fdcab28185..cc5e59bcf1 100644 --- a/configs/tinker-rk3288_defconfig +++ b/configs/tinker-rk3288_defconfig @@ -3,6 +3,7 @@ CONFIG_ARCH_ROCKCHIP=y CONFIG_SYS_TEXT_BASE=0x00000000 CONFIG_SYS_MALLOC_F_LEN=0x2000 CONFIG_ROCKCHIP_RK3288=y +CONFIG_SPL_WITH_DTB_SIZE_LIMIT=32700 CONFIG_SPL_ROCKCHIP_BACK_TO_BROM=y CONFIG_TARGET_TINKER_RK3288=y CONFIG_DEBUG_UART_BASE=0xff690000 diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 9d5921606e..afc329a410 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -254,11 +254,27 @@ FINAL_DTB_CONTAINER = $(obj)/$(SPL_BIN).multidtb.fit endif
+ifneq ($(CONFIG_SPL_WITH_DTB_SIZE_LIMIT),0) +SPL_WITH_DTB_SIZE_CHECK = \ + @actual=`wc -c $@ | awk '{print $$1}'`; \ + limit=`printf "%d" $(CONFIG_SPL_WITH_DTB_SIZE_LIMIT)`; \ + if test $$actual -gt $$limit; then \ + echo "$@ exceeds file size limit:" >&2 ; \ + echo " limit: $$limit bytes" >&2 ; \ + echo " actual: $$actual bytes" >&2 ; \ + echo " excess: $$((actual - limit)) bytes" >&2; \ + exit 1; \ + fi +else +SPL_WITH_DTB_SIZE_CHECK = +endif + ifeq ($(CONFIG_$(SPL_TPL_)OF_CONTROL)$(CONFIG_OF_SEPARATE)$(CONFIG_$(SPL_TPL_)OF_PLATDATA),yy) $(obj)/$(SPL_BIN)-dtb.bin: $(obj)/$(SPL_BIN)-nodtb.bin \ $(if $(CONFIG_SPL_SEPARATE_BSS),,$(obj)/$(SPL_BIN)-pad.bin) \ $(FINAL_DTB_CONTAINER) FORCE $(call if_changed,cat) + $(SPL_WITH_DTB_SIZE_CHECK)
$(obj)/$(SPL_BIN).bin: $(obj)/$(SPL_BIN)-dtb.bin FORCE $(call if_changed,copy)
Am 13.02.2019 um 22:38 schrieb Heinrich Schuchardt:
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
That was fast :)
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
v2 Instead of using CONFIG_SPL_MAX_SIZE with an estimate of the FDT size introduce a new test in scripts/Makefile.spl.
Kconfig | 8 ++++++++ configs/tinker-rk3288_defconfig | 1 + scripts/Makefile.spl | 16 ++++++++++++++++ 3 files changed, 25 insertions(+)
diff --git a/Kconfig b/Kconfig index 512c7beb89..7cce53da74 100644 --- a/Kconfig +++ b/Kconfig @@ -172,6 +172,14 @@ config TPL_SYS_MALLOC_F_LEN particular needs this to operate, so that it can allocate the initial serial device and any others that are needed.
+config SPL_WITH_DTB_SIZE_LIMIT
- int "Maximum size of SPL image including device tree"
- depends on SPL
- default 0
- help
Specifies the maximum length of the U-Boot SPL image including thedevice tree. If this value is zero, it is ignored.- menuconfig EXPERT bool "Configure standard U-Boot features (expert users)" default y
diff --git a/configs/tinker-rk3288_defconfig b/configs/tinker-rk3288_defconfig index fdcab28185..cc5e59bcf1 100644 --- a/configs/tinker-rk3288_defconfig +++ b/configs/tinker-rk3288_defconfig @@ -3,6 +3,7 @@ CONFIG_ARCH_ROCKCHIP=y CONFIG_SYS_TEXT_BASE=0x00000000 CONFIG_SYS_MALLOC_F_LEN=0x2000 CONFIG_ROCKCHIP_RK3288=y +CONFIG_SPL_WITH_DTB_SIZE_LIMIT=32700 CONFIG_SPL_ROCKCHIP_BACK_TO_BROM=y CONFIG_TARGET_TINKER_RK3288=y CONFIG_DEBUG_UART_BASE=0xff690000 diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 9d5921606e..afc329a410 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -254,11 +254,27 @@ FINAL_DTB_CONTAINER = $(obj)/$(SPL_BIN).multidtb.fit endif
+ifneq ($(CONFIG_SPL_WITH_DTB_SIZE_LIMIT),0) +SPL_WITH_DTB_SIZE_CHECK = \
- @actual=`wc -c $@ | awk '{print $$1}'`; \
- limit=`printf "%d" $(CONFIG_SPL_WITH_DTB_SIZE_LIMIT)`; \
- if test $$actual -gt $$limit; then \
echo "$@ exceeds file size limit:" >&2 ; \echo " limit: $$limit bytes" >&2 ; \echo " actual: $$actual bytes" >&2 ; \echo " excess: $$((actual - limit)) bytes" >&2; \exit 1; \- fi
+else +SPL_WITH_DTB_SIZE_CHECK = +endif
ifeq ($(CONFIG_$(SPL_TPL_)OF_CONTROL)$(CONFIG_OF_SEPARATE)$(CONFIG_$(SPL_TPL_)OF_PLATDATA),yy) $(obj)/$(SPL_BIN)-dtb.bin: $(obj)/$(SPL_BIN)-nodtb.bin \ $(if $(CONFIG_SPL_SEPARATE_BSS),,$(obj)/$(SPL_BIN)-pad.bin) \ $(FINAL_DTB_CONTAINER) FORCE $(call if_changed,cat)
$(SPL_WITH_DTB_SIZE_CHECK)
$(obj)/$(SPL_BIN).bin: $(obj)/$(SPL_BIN)-dtb.bin FORCE $(call if_changed,copy)
On 13.02.2019, at 22:38, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Reviewed-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com
On Wed, Feb 13, 2019 at 10:38:09PM +0100, Heinrich Schuchardt wrote:
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2 Instead of using CONFIG_SPL_MAX_SIZE with an estimate of the FDT size introduce a new test in scripts/Makefile.spl.
[snip]
+ifneq ($(CONFIG_SPL_WITH_DTB_SIZE_LIMIT),0) +SPL_WITH_DTB_SIZE_CHECK = \
- @actual=`wc -c $@ | awk '{print $$1}'`; \
- limit=`printf "%d" $(CONFIG_SPL_WITH_DTB_SIZE_LIMIT)`; \
- if test $$actual -gt $$limit; then \
echo "$@ exceeds file size limit:" >&2 ; \echo " limit: $$limit bytes" >&2 ; \echo " actual: $$actual bytes" >&2 ; \echo " excess: $$((actual - limit)) bytes" >&2; \exit 1; \- fi
+else +SPL_WITH_DTB_SIZE_CHECK = +endif
OK, but now we have 3 copies of this logic. Can we not define a function and pass the limit in to it? Then we'd have a few things like: ifneq ($(CONFIG_xxx_MAX_SIZE),0) xxx_SIZE_CHECK = ... call func with $@ and size else xxx_SIZE_CHECK = endif
Or do we need something else to avoid duplicating this in so many places?
On 2/16/19 11:11 PM, Tom Rini wrote:
On Wed, Feb 13, 2019 at 10:38:09PM +0100, Heinrich Schuchardt wrote:
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2 Instead of using CONFIG_SPL_MAX_SIZE with an estimate of the FDT size introduce a new test in scripts/Makefile.spl.
[snip]
+ifneq ($(CONFIG_SPL_WITH_DTB_SIZE_LIMIT),0) +SPL_WITH_DTB_SIZE_CHECK = \
- @actual=`wc -c $@ | awk '{print $$1}'`; \
- limit=`printf "%d" $(CONFIG_SPL_WITH_DTB_SIZE_LIMIT)`; \
- if test $$actual -gt $$limit; then \
echo "$@ exceeds file size limit:" >&2 ; \echo " limit: $$limit bytes" >&2 ; \echo " actual: $$actual bytes" >&2 ; \echo " excess: $$((actual - limit)) bytes" >&2; \exit 1; \- fi
+else +SPL_WITH_DTB_SIZE_CHECK = +endif
OK, but now we have 3 copies of this logic. Can we not define a function and pass the limit in to it? Then we'd have a few things like: ifneq ($(CONFIG_xxx_MAX_SIZE),0) xxx_SIZE_CHECK = ... call func with $@ and size else xxx_SIZE_CHECK = endif
Or do we need something else to avoid duplicating this in so many places?
I found BOARD_SIZE_CHECK in Makefile and in arch/arm/mach-imx/Makefile. It is unclear to me why the copy in arch/arm/mach-imx/Makefile exists at all. Isn't that Makefile included by the main Makefile?
You once mentioned that there are other boards with the same problem as the Tinker Board. Which are these?
Best regards
Heinrich
On Sat, Feb 16, 2019 at 11:51:54PM +0100, Heinrich Schuchardt wrote:
On 2/16/19 11:11 PM, Tom Rini wrote:
On Wed, Feb 13, 2019 at 10:38:09PM +0100, Heinrich Schuchardt wrote:
The SPL image for the Tinker Board has to fit into 32 KiB. This includes 4 KiB for the device tree and up to 2 KiB for the file header.
A new configuration variable CONFIG_SPL_WITH_DTB_SIZE_LIMIT is introduced to define the board specific limit.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v2 Instead of using CONFIG_SPL_MAX_SIZE with an estimate of the FDT size introduce a new test in scripts/Makefile.spl.
[snip]
+ifneq ($(CONFIG_SPL_WITH_DTB_SIZE_LIMIT),0) +SPL_WITH_DTB_SIZE_CHECK = \
- @actual=`wc -c $@ | awk '{print $$1}'`; \
- limit=`printf "%d" $(CONFIG_SPL_WITH_DTB_SIZE_LIMIT)`; \
- if test $$actual -gt $$limit; then \
echo "$@ exceeds file size limit:" >&2 ; \echo " limit: $$limit bytes" >&2 ; \echo " actual: $$actual bytes" >&2 ; \echo " excess: $$((actual - limit)) bytes" >&2; \exit 1; \- fi
+else +SPL_WITH_DTB_SIZE_CHECK = +endif
OK, but now we have 3 copies of this logic. Can we not define a function and pass the limit in to it? Then we'd have a few things like: ifneq ($(CONFIG_xxx_MAX_SIZE),0) xxx_SIZE_CHECK = ... call func with $@ and size else xxx_SIZE_CHECK = endif
Or do we need something else to avoid duplicating this in so many places?
I found BOARD_SIZE_CHECK in Makefile and in arch/arm/mach-imx/Makefile. It is unclear to me why the copy in arch/arm/mach-imx/Makefile exists at all. Isn't that Makefile included by the main Makefile?
Yes, the logic got copied to two places, and then updated somewhat recently. I want to prevent it from becoming three places.
You once mentioned that there are other boards with the same problem as the Tinker Board. Which are these?
Well, Simon has been talking about SoCFPA. And as we get into migrating a number of platforms over to SPL+SPL_OF_CONTROL a lot more platforms are going to hit this too. Heck, I wonder if the failure I saw on am335x_evm a few hours ago was this one. And a quick ls -lk says it's quite close and I'd have to actually check the real values. So it's real important to me that we be able to plug "everyone" in here as the existing CONFIG_SPL_MAX_SIZE isn't catching nearly so much as we don't have a link error here but rather a final value overflow and that's what's been the point of these checks all along. Thanks!
participants (4)
-
Heinrich Schuchardt -
Philipp Tomsich -
Simon Goldschmidt -
Tom Rini