Re: [U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
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
> >> ---
> >> 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!
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
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
>> ---
>> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
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
> ---
> 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?
--
Tom
signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
> On 13.02.2019, at 22:38, 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 Reviewed-by: Philipp Tomsich ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 1/1] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
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
That was fast :)
Reviewed-by: Simon Goldschmidt
---
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=0x
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=0xff69
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)
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[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
---
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=0x
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=0xff69
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)
--
2.20.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
