All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
@ 2019-04-02 17:19 Heinrich Schuchardt
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK Heinrich Schuchardt
                   ` (6 more replies)
  0 siblings, 7 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:19 UTC (permalink / raw)
  To: u-boot

The SPL image for the Tinker Board has to fit into 32 KiB. This includes
up to 2 KiB for the file header.

A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
the board specific limit.

A common Makefile function is used for this test and the test against
CONFIG_BOARD_SIZE_LIMIT.

Move the board size check from arch/arm/mach-imx/Makefile to Makefile.

v4:
	use a common function for all size checks in the Makefiles

Heinrich Schuchardt (4):
  Makefile: reusable function for BOARD_SIZE_CHECK
  imx: move BOARD_SIZE_CHECK to main Makefile
  configs: define CONFIG_SPL_SIZE_LIMIT
  configs: rk3288: Tinker Board SPL file must fit into 32 KiB

 Kconfig                         |  8 ++++++++
 Makefile                        | 33 +++++++++++++++++++++++----------
 arch/arm/mach-imx/Makefile      | 16 ----------------
 configs/tinker-rk3288_defconfig |  1 +
 4 files changed, 32 insertions(+), 26 deletions(-)

--
2.20.1

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

* [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
@ 2019-04-02 17:19 ` Heinrich Schuchardt
  2019-06-07 22:04   ` Tom Rini
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:19 UTC (permalink / raw)
  To: u-boot

Carve out function size_check from macro BOARD_SIZE_CHECK. This will allow
us to reuse the function for other file size checks.

Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like the
following is thrown:

u-boot-dtb.img exceeds file size limit:
  limit:  409516 bytes
  actual: 444346 bytes
  excess: 34830 bytes
make: *** [Makefile:1212: u-boot-dtb.img] Error 1

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v4
	new patch
---
 Makefile | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index c1af9307b3..9878595a82 100644
--- a/Makefile
+++ b/Makefile
@@ -330,6 +330,19 @@ endif
 #  KBUILD_MODULES := 1
 #endif

+define size_check
+	actual=$$( wc -c $1 | awk '{print $$1}'); \
+	limit=$$( printf "%d" $2 ); \
+	if test $$actual -gt $$limit; then \
+		echo "$1 exceeds file size limit:" >&2; \
+		echo "  limit:  $$limit bytes" >&2; \
+		echo "  actual: $$actual bytes" >&2; \
+		echo "  excess: $$((actual - limit)) bytes" >&2; \
+		exit 1; \
+	fi
+endef
+export size_check
+
 export KBUILD_MODULES KBUILD_BUILTIN
 export KBUILD_CHECKSRC KBUILD_SRC KBUILD_EXTMOD

@@ -771,16 +784,7 @@ LDPPFLAGS += \
 #########################################################################

 ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
-BOARD_SIZE_CHECK = \
-	@actual=`wc -c $@ | awk '{print $$1}'`; \
-	limit=`printf "%d" $(CONFIG_BOARD_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
+BOARD_SIZE_CHECK= @ $(call size_check,$@,$(CONFIG_BOARD_SIZE_LIMIT))
 else
 BOARD_SIZE_CHECK =
 endif
--
2.20.1

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

* [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK Heinrich Schuchardt
@ 2019-04-02 17:19 ` Heinrich Schuchardt
  2019-04-04 20:53   ` Heinrich Schuchardt
                     ` (2 more replies)
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT Heinrich Schuchardt
                   ` (4 subsequent siblings)
  6 siblings, 3 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:19 UTC (permalink / raw)
  To: u-boot

We currently have duplicate definitions for BOARD_SIZE_CHECK in Makefile
and arch/arm/mach-imx/Makefile.

Move the board size check from arch/arm/mach-imx/Makefile to Makefile.

Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like an error
like the following is thrown:

u-boot-dtb.imx exceeds file size limit:
  limit:  503696 bytes
  actual: 509720 bytes
  excess: 6024 bytes
make: *** [Makefile:1051: u-boot-dtb.imx] Error 1

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v4
	new patch
---
 Makefile                   |  1 +
 arch/arm/mach-imx/Makefile | 16 ----------------
 2 files changed, 1 insertion(+), 16 deletions(-)

diff --git a/Makefile b/Makefile
index 9878595a82..6398117e64 100644
--- a/Makefile
+++ b/Makefile
@@ -1042,6 +1042,7 @@ endif

 %.imx: %.bin
 	$(Q)$(MAKE) $(build)=arch/arm/mach-imx $@
+	$(BOARD_SIZE_CHECK)

 %.vyb: %.imx
 	$(Q)$(MAKE) $(build)=arch/arm/cpu/armv7/vf610 $@
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index c3ed62aed6..7985afb154 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -61,21 +61,6 @@ obj-$(CONFIG_CMD_HDMIDETECT) += cmd_hdmidet.o
 obj-$(CONFIG_CMD_DEKBLOB) += cmd_dek.o
 endif

-ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
-BOARD_SIZE_CHECK = \
-        @actual=`wc -c $@ | awk '{print $$1}'`; \
-        limit=`printf "%d" $(CONFIG_BOARD_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
-BOARD_SIZE_CHECK =
-endif
-
 PLUGIN = board/$(BOARDDIR)/plugin

 ifeq ($(CONFIG_USE_IMXIMG_PLUGIN),y)
@@ -124,7 +109,6 @@ u-boot.imx: MKIMAGEOUTPUT = u-boot.imx.log

 u-boot.imx: u-boot.bin u-boot.cfgout $(PLUGIN).bin FORCE
 	$(call if_changed,mkimage)
-	$(BOARD_SIZE_CHECK)

 ifeq ($(CONFIG_OF_SEPARATE),y)
 MKIMAGEFLAGS_u-boot-dtb.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) \
--
2.20.1

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

* [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK Heinrich Schuchardt
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
@ 2019-04-02 17:19 ` Heinrich Schuchardt
  2019-06-07 22:05   ` Tom Rini
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:19 UTC (permalink / raw)
  To: u-boot

A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
the board specific maximum size for the SPL file.

Use Makefile function size_check() to implement the test.

Depending on the size of CONFIG_SPL_SIZE_LIMIT an error like the following
is thrown:

spl/u-boot-spl.bin exceeds file size limit:
  limit:  30720 bytes
  actual: 33426 bytes
  excess: 2706 bytes
make: *** [Makefile:1663: spl/u-boot-spl.bin] Error 1

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v4
	use function size_check()
	split off change in tinker-rk3288_defconfig
---
 Kconfig  | 8 ++++++++
 Makefile | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/Kconfig b/Kconfig
index 305b265ed7..69212bf58f 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_SIZE_LIMIT
+	int "Maximum size of SPL image"
+	depends on SPL
+	default 0
+	help
+	  Specifies the maximum length of the U-Boot SPL image.
+	  If this value is zero, it is ignored.
+
 menuconfig EXPERT
 	bool "Configure standard U-Boot features (expert users)"
 	default y
diff --git a/Makefile b/Makefile
index 6398117e64..073ef2b387 100644
--- a/Makefile
+++ b/Makefile
@@ -789,6 +789,12 @@ else
 BOARD_SIZE_CHECK =
 endif

+ifneq ($(CONFIG_SPL_SIZE_LIMIT),0)
+SPL_SIZE_CHECK = @$(call size_check,$@,$(CONFIG_SPL_SIZE_LIMIT))
+else
+SPL_SIZE_CHECK =
+endif
+
 # Statically apply RELA-style relocations (currently arm64 only)
 # This is useful for arm64 where static relocation needs to be performed on
 # the raw binary, but certain simulators only accept an ELF file (but don't
@@ -1654,6 +1660,8 @@ u-boot.lds: $(LDSCRIPT) prepare FORCE

 spl/u-boot-spl.bin: spl/u-boot-spl
 	@:
+	$(SPL_SIZE_CHECK)
+
 spl/u-boot-spl: tools prepare \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SPL_OF_PLATDATA),dts/dt.dtb) \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_TPL_OF_PLATDATA),dts/dt.dtb)
--
2.20.1

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

* [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
                   ` (2 preceding siblings ...)
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT Heinrich Schuchardt
@ 2019-04-02 17:19 ` Heinrich Schuchardt
  2019-06-07 22:05   ` Tom Rini
  2019-04-02 17:29 ` [U-Boot] [PATCH v4 0/4] " Tom Rini
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:19 UTC (permalink / raw)
  To: u-boot

The SPL image for the Tinker Board has to fit into 32 KiB. This includes
up to 2 KiB for the file header.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
The error for the TinkerBoard is resolved by:
configs: tinker-rk3288 disable CONFIG_SPL_I2C_SUPPORT
https://lists.denx.de/pipermail/u-boot/2019-February/360367.html

v4
	split of change in tinker-rk3288_defconfig
---
 configs/tinker-rk3288_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/tinker-rk3288_defconfig b/configs/tinker-rk3288_defconfig
index 85ef9dabbd..585fbf505c 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_SIZE_LIMIT=30720
 CONFIG_SPL_ROCKCHIP_BACK_TO_BROM=y
 CONFIG_TARGET_TINKER_RK3288=y
 CONFIG_DEBUG_UART_BASE=0xff690000
--
2.20.1

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
                   ` (3 preceding siblings ...)
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
@ 2019-04-02 17:29 ` Tom Rini
  2019-04-02 17:33   ` Heinrich Schuchardt
  2019-04-03  0:59 ` Kever Yang
  2019-04-18 20:12 ` Simon Goldschmidt
  6 siblings, 1 reply; 27+ messages in thread
From: Tom Rini @ 2019-04-02 17:29 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:19:03PM +0200, Heinrich Schuchardt wrote:

> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> up to 2 KiB for the file header.
> 
> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> the board specific limit.
> 
> A common Makefile function is used for this test and the test against
> CONFIG_BOARD_SIZE_LIMIT.
> 
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.

I'm sorry you weren't Cc'd on Simon's thread where we're trying to
improve the size check stuff to be generic enough to use everywhere.  We
can't generically use a shell script as we need to know some processed
values too.  I don't know if Simon got to the point of writing a C based
helper to use or not.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190402/a3669879/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:29 ` [U-Boot] [PATCH v4 0/4] " Tom Rini
@ 2019-04-02 17:33   ` Heinrich Schuchardt
  2019-04-02 17:47     ` Tom Rini
  2019-04-02 18:08     ` Simon Goldschmidt
  0 siblings, 2 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 17:33 UTC (permalink / raw)
  To: u-boot

On 4/2/19 7:29 PM, Tom Rini wrote:
> On Tue, Apr 02, 2019 at 07:19:03PM +0200, Heinrich Schuchardt wrote:
> 
>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>> up to 2 KiB for the file header.
>>
>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>> the board specific limit.
>>
>> A common Makefile function is used for this test and the test against
>> CONFIG_BOARD_SIZE_LIMIT.
>>
>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> 
> I'm sorry you weren't Cc'd on Simon's thread where we're trying to
> improve the size check stuff to be generic enough to use everywhere.  We
> can't generically use a shell script as we need to know some processed
> values too.  I don't know if Simon got to the point of writing a C based
> helper to use or not.
> 

Hello Tom,

could you, please, provide a link to the thread.

Is the test prior to my patch incorrect? Or do you want to imply that
after my patch we get different results?

Best regards

Heinrich

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190402/38746788/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:33   ` Heinrich Schuchardt
@ 2019-04-02 17:47     ` Tom Rini
  2019-04-02 18:17       ` Heinrich Schuchardt
  2019-04-02 18:08     ` Simon Goldschmidt
  1 sibling, 1 reply; 27+ messages in thread
From: Tom Rini @ 2019-04-02 17:47 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:33:42PM +0200, Heinrich Schuchardt wrote:
> On 4/2/19 7:29 PM, Tom Rini wrote:
> > On Tue, Apr 02, 2019 at 07:19:03PM +0200, Heinrich Schuchardt wrote:
> > 
> >> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >> up to 2 KiB for the file header.
> >>
> >> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >> the board specific limit.
> >>
> >> A common Makefile function is used for this test and the test against
> >> CONFIG_BOARD_SIZE_LIMIT.
> >>
> >> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> > 
> > I'm sorry you weren't Cc'd on Simon's thread where we're trying to
> > improve the size check stuff to be generic enough to use everywhere.  We
> > can't generically use a shell script as we need to know some processed
> > values too.  I don't know if Simon got to the point of writing a C based
> > helper to use or not.
> > 
> 
> Hello Tom,
> 
> could you, please, provide a link to the thread.

https://patchwork.ozlabs.org/patch/1050465/ (and no, I didn't apply it,
I forgot to delete my email before I sent out that batch of applieds).

> Is the test prior to my patch incorrect? Or do you want to imply that
> after my patch we get different results?

The problem is that I don't want to make wider use of the shell checking
notion.  It's not as complete as we'd like and can't be used on as many
platforms as need it either, due to needing cpp to actually determine
values in some cases.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190402/1a067864/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:33   ` Heinrich Schuchardt
  2019-04-02 17:47     ` Tom Rini
@ 2019-04-02 18:08     ` Simon Goldschmidt
  1 sibling, 0 replies; 27+ messages in thread
From: Simon Goldschmidt @ 2019-04-02 18:08 UTC (permalink / raw)
  To: u-boot



On 02.04.19 19:33, Heinrich Schuchardt wrote:
> On 4/2/19 7:29 PM, Tom Rini wrote:
>> On Tue, Apr 02, 2019 at 07:19:03PM +0200, Heinrich Schuchardt wrote:
>>
>>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>>> up to 2 KiB for the file header.
>>>
>>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>>> the board specific limit.
>>>
>>> A common Makefile function is used for this test and the test against
>>> CONFIG_BOARD_SIZE_LIMIT.
>>>
>>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>
>> I'm sorry you weren't Cc'd on Simon's thread where we're trying to
>> improve the size check stuff to be generic enough to use everywhere.  We
>> can't generically use a shell script as we need to know some processed
>> values too.  I don't know if Simon got to the point of writing a C based
>> helper to use or not.

Right, I should have included you, Heinrich, sorry.

No, unfortunately, I haven't found the time to work on the C based 
approach, yet.

To sum it up, Heinrich, the issue was that for socfpga gen5, we're 
approaching the 64KiB total limit for SPL. That includes all regions we 
can check via CONFIG_SPL_MAX_FOOTPRINT (e.g. text, data, bss) plus 
devicetree, early malloc, global data and initial stack.

My approach would have been much like your check (which by the way is 
better than mine in that the check stays in main Makefile), but the size 
limit needs to include those items I've mentioned above. So while I 
could add a Kconfig based total size, I would need to subtract gd, 
malloc area and reserved stack size to get the SPL_SIZE_LIMIT value.

The problem I see here is that this is a different size limit to the one 
you want to check. I need "SRAM_SIZE - x - y - z" while you need 
"LOAD_LIMIT", which is constant.

Maybe we could start with your limit and then add bool config options 
like "SPL_SIZE_LIMIT_SUBTRACT_GD", "SPL_SIZE_LIMIT_SUBTRACT_MALLOC_F" 
and "SPL_SIZE_LIMIT_SUBTRACT_STACK_SIZE" that could then be used by a C 
program to help the size check.

Regards,
Simon

>>
> 
> Hello Tom,
> 
> could you, please, provide a link to the thread.
> 
> Is the test prior to my patch incorrect? Or do you want to imply that
> after my patch we get different results?
> 
> Best regards
> 
> Heinrich
> 

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:47     ` Tom Rini
@ 2019-04-02 18:17       ` Heinrich Schuchardt
  0 siblings, 0 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-02 18:17 UTC (permalink / raw)
  To: u-boot

On 4/2/19 7:47 PM, Tom Rini wrote:
> On Tue, Apr 02, 2019 at 07:33:42PM +0200, Heinrich Schuchardt wrote:
>> On 4/2/19 7:29 PM, Tom Rini wrote:
>>> On Tue, Apr 02, 2019 at 07:19:03PM +0200, Heinrich Schuchardt wrote:
>>>
>>>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>>>> up to 2 KiB for the file header.
>>>>
>>>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>>>> the board specific limit.
>>>>
>>>> A common Makefile function is used for this test and the test against
>>>> CONFIG_BOARD_SIZE_LIMIT.
>>>>
>>>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>>
>>> I'm sorry you weren't Cc'd on Simon's thread where we're trying to
>>> improve the size check stuff to be generic enough to use everywhere.  We
>>> can't generically use a shell script as we need to know some processed
>>> values too.  I don't know if Simon got to the point of writing a C based
>>> helper to use or not.
>>>
>>
>> Hello Tom,
>>
>> could you, please, provide a link to the thread.
> 
> https://patchwork.ozlabs.org/patch/1050465/ (and no, I didn't apply it,
> I forgot to delete my email before I sent out that batch of applieds).
> 
>> Is the test prior to my patch incorrect? Or do you want to imply that
>> after my patch we get different results?
> 
> The problem is that I don't want to make wider use of the shell checking
> notion.  It's not as complete as we'd like and can't be used on as many
> platforms as need it either, due to needing cpp to actually determine
> values in some cases.
> 

Yes, the patch by Simon G. is similiar to my patch series. What his
patch does not address is the duplicate coding for checking *.imx files.
My idea has been to put all tests into the main Makefile to avoid
problems with variable substitutions.

It is unclear to me what you mean by: "It's not as complete as we'd
like" and where you see a need for "cpp to actually determine values".

I would not be able to provide a better patch without understanding your
view on the requirements.

Best regards

Heinrich

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190402/b3921ad9/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
                   ` (4 preceding siblings ...)
  2019-04-02 17:29 ` [U-Boot] [PATCH v4 0/4] " Tom Rini
@ 2019-04-03  0:59 ` Kever Yang
  2019-04-03  6:19   ` Heinrich Schuchardt
  2019-04-18 20:12 ` Simon Goldschmidt
  6 siblings, 1 reply; 27+ messages in thread
From: Kever Yang @ 2019-04-03  0:59 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,


On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> up to 2 KiB for the file header.

32KB is the limit of SPL size, not SPL image size, does not include 2KB
header.
>
> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> the board specific limit.

There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
I don't understand  why we need new variable.

Thanks,
- Kever
> A common Makefile function is used for this test and the test against
> CONFIG_BOARD_SIZE_LIMIT.
>
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>
> v4:
> 	use a common function for all size checks in the Makefiles
>
> Heinrich Schuchardt (4):
>   Makefile: reusable function for BOARD_SIZE_CHECK
>   imx: move BOARD_SIZE_CHECK to main Makefile
>   configs: define CONFIG_SPL_SIZE_LIMIT
>   configs: rk3288: Tinker Board SPL file must fit into 32 KiB
>
>  Kconfig                         |  8 ++++++++
>  Makefile                        | 33 +++++++++++++++++++++++----------
>  arch/arm/mach-imx/Makefile      | 16 ----------------
>  configs/tinker-rk3288_defconfig |  1 +
>  4 files changed, 32 insertions(+), 26 deletions(-)
>
> --
> 2.20.1
>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-03  0:59 ` Kever Yang
@ 2019-04-03  6:19   ` Heinrich Schuchardt
  2019-04-03 20:50     ` Benoît Thébaudeau
  0 siblings, 1 reply; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-03  6:19 UTC (permalink / raw)
  To: u-boot

On 4/3/19 2:59 AM, Kever Yang wrote:
> Hi Heinrich,
>
>
> On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>> up to 2 KiB for the file header.
>
> 32KB is the limit of SPL size, not SPL image size, does not include 2KB
> header.
>>
>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>> the board specific limit.
>
> There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
> I don't understand  why we need new variable.

SPL_MAX_SIZE is used to set CONFIG_SPL_PAD_TO if CONFIG_SPL_PAD_TO is
not defined. If CONFIG_SPL_PAD_TO is defined it specifies the maximum
value of CONFIG_SPL_PAD_TO. So in case CONFIG_SPL_PAD_TO is not defined
SPL_MAX_SIZE does not specify a maximum size but a minimum one.

It is unclear to me why Benoît in 6113d3f27ca ("Makefile: Change
CONFIG_SPL_PAD_TO to image offset") chose this design. Entangling the
variables in this way does not provide a clean design.

The check

#if defined(IMAGE_MAX_SIZE)
ASSERT(__image_copy_end - __image_copy_start < (IMAGE_MAX_SIZE), \
        "SPL image too big");
#endif

in arch/arm/cpu/u-boot-spl.lds neither considers the size of the device
tree nor of the BSS section. So@least for Rockchip SOCs it is
checking a measure that is not directly related to the image size
created by `mkimage -T rksd`.

Best regards

Heinrich

>
> Thanks,
> - Kever
>> A common Makefile function is used for this test and the test against
>> CONFIG_BOARD_SIZE_LIMIT.
>>
>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>
>> v4:
>> 	use a common function for all size checks in the Makefiles
>>
>> Heinrich Schuchardt (4):
>>   Makefile: reusable function for BOARD_SIZE_CHECK
>>   imx: move BOARD_SIZE_CHECK to main Makefile
>>   configs: define CONFIG_SPL_SIZE_LIMIT
>>   configs: rk3288: Tinker Board SPL file must fit into 32 KiB
>>
>>  Kconfig                         |  8 ++++++++
>>  Makefile                        | 33 +++++++++++++++++++++++----------
>>  arch/arm/mach-imx/Makefile      | 16 ----------------
>>  configs/tinker-rk3288_defconfig |  1 +
>>  4 files changed, 32 insertions(+), 26 deletions(-)
>>
>> --
>> 2.20.1

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-03  6:19   ` Heinrich Schuchardt
@ 2019-04-03 20:50     ` Benoît Thébaudeau
  0 siblings, 0 replies; 27+ messages in thread
From: Benoît Thébaudeau @ 2019-04-03 20:50 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Wed, Apr 3, 2019 at 8:20 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 4/3/19 2:59 AM, Kever Yang wrote:
> > Hi Heinrich,
> >
> >
> > On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
> >> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >> up to 2 KiB for the file header.
> >
> > 32KB is the limit of SPL size, not SPL image size, does not include 2KB
> > header.
> >>
> >> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >> the board specific limit.
> >
> > There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
> > I don't understand  why we need new variable.
>
> SPL_MAX_SIZE is used to set CONFIG_SPL_PAD_TO if CONFIG_SPL_PAD_TO is
> not defined. If CONFIG_SPL_PAD_TO is defined it specifies the maximum
> value of CONFIG_SPL_PAD_TO.

If CONFIG_SPL_PAD_TO is defined and non-zero, CONFIG_SPL_MAX_SIZE
specifies the _minimum_ value of CONFIG_SPL_PAD_TO.

> So in case CONFIG_SPL_PAD_TO is not defined
> SPL_MAX_SIZE does not specify a maximum size but a minimum one.

CONFIG_SPL_MAX_SIZE specifies a minimum value for CONFIG_SPL_PAD_TO,
but a maximum size for the SPL.

> It is unclear to me why Benoît in 6113d3f27ca ("Makefile: Change
> CONFIG_SPL_PAD_TO to image offset") chose this design. Entangling the
> variables in this way does not provide a clean design.

Everything is explained in this thread:
https://lists.denx.de/pipermail/u-boot/2013-February/146851.html

These two variables coexisted before this change. They have different
purposes, but they are related, so they cannot take any values
independently of each other.

The purpose of CONFIG_SPL_MAX_SIZE is not to define default or minimum
values for CONFIG_SPL_PAD_TO, but only what its name says, i.e. to
define the maximum SPL size.

After the SPL, there may be a gap, then a payload appended to the
image. The purpose of CONFIG_SPL_PAD_TO is to define the image offset
of the payload, if any. CONFIG_SPL_PAD_TO can be 0 if the payload can
always be appended to the SPL without any gap.

Therefore, the SPL must fit inside CONFIG_SPL_MAX_SIZE, and
CONFIG_SPL_MAX_SIZE must fit inside CONFIG_SPL_PAD_TO if the latter is
non-zero.

[...]

Best regards,
Benoît

On Wed, Apr 3, 2019 at 8:20 AM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 4/3/19 2:59 AM, Kever Yang wrote:
> > Hi Heinrich,
> >
> >
> > On 04/03/2019 01:19 AM, Heinrich Schuchardt wrote:
> >> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >> up to 2 KiB for the file header.
> >
> > 32KB is the limit of SPL size, not SPL image size, does not include 2KB
> > header.
> >>
> >> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >> the board specific limit.
> >
> > There is already CONFIG_SPL_MAX_SIZE for the SPL size limit, isn't it?
> > I don't understand  why we need new variable.
>
> SPL_MAX_SIZE is used to set CONFIG_SPL_PAD_TO if CONFIG_SPL_PAD_TO is
> not defined. If CONFIG_SPL_PAD_TO is defined it specifies the maximum
> value of CONFIG_SPL_PAD_TO. So in case CONFIG_SPL_PAD_TO is not defined
> SPL_MAX_SIZE does not specify a maximum size but a minimum one.
>
> It is unclear to me why Benoît in 6113d3f27ca ("Makefile: Change
> CONFIG_SPL_PAD_TO to image offset") chose this design. Entangling the
> variables in this way does not provide a clean design.
>
> The check
>
> #if defined(IMAGE_MAX_SIZE)
> ASSERT(__image_copy_end - __image_copy_start < (IMAGE_MAX_SIZE), \
>         "SPL image too big");
> #endif
>
> in arch/arm/cpu/u-boot-spl.lds neither considers the size of the device
> tree nor of the BSS section. So at least for Rockchip SOCs it is
> checking a measure that is not directly related to the image size
> created by `mkimage -T rksd`.
>
> Best regards
>
> Heinrich
>
> >
> > Thanks,
> > - Kever
> >> A common Makefile function is used for this test and the test against
> >> CONFIG_BOARD_SIZE_LIMIT.
> >>
> >> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> >>
> >> v4:
> >>      use a common function for all size checks in the Makefiles
> >>
> >> Heinrich Schuchardt (4):
> >>   Makefile: reusable function for BOARD_SIZE_CHECK
> >>   imx: move BOARD_SIZE_CHECK to main Makefile
> >>   configs: define CONFIG_SPL_SIZE_LIMIT
> >>   configs: rk3288: Tinker Board SPL file must fit into 32 KiB
> >>
> >>  Kconfig                         |  8 ++++++++
> >>  Makefile                        | 33 +++++++++++++++++++++++----------
> >>  arch/arm/mach-imx/Makefile      | 16 ----------------
> >>  configs/tinker-rk3288_defconfig |  1 +
> >>  4 files changed, 32 insertions(+), 26 deletions(-)
> >>
> >> --
> >> 2.20.1
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
@ 2019-04-04 20:53   ` Heinrich Schuchardt
  2019-04-05 13:05   ` Fabio Estevam
  2019-06-07 22:04   ` Tom Rini
  2 siblings, 0 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-04 20:53 UTC (permalink / raw)
  To: u-boot

On 4/2/19 7:19 PM, Heinrich Schuchardt wrote:
> We currently have duplicate definitions for BOARD_SIZE_CHECK in Makefile
> and arch/arm/mach-imx/Makefile.
>
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>
> Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like an error
> like the following is thrown:
>
> u-boot-dtb.imx exceeds file size limit:
>   limit:  503696 bytes
>   actual: 509720 bytes
>   excess: 6024 bytes
> make: *** [Makefile:1051: u-boot-dtb.imx] Error 1
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Hello Stefano, hello Fabio,

there have been some comments to 0/4 indicating that the first patch of
the series should be reworked.

But I think this one is worth merging on it own. Could you, please,
review it and if ok add it to your IMX repository.

Best regards

Heinrich


> ---
> v4
> 	new patch
> ---
>  Makefile                   |  1 +
>  arch/arm/mach-imx/Makefile | 16 ----------------
>  2 files changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9878595a82..6398117e64 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1042,6 +1042,7 @@ endif
>
>  %.imx: %.bin
>  	$(Q)$(MAKE) $(build)=arch/arm/mach-imx $@
> +	$(BOARD_SIZE_CHECK)
>
>  %.vyb: %.imx
>  	$(Q)$(MAKE) $(build)=arch/arm/cpu/armv7/vf610 $@
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index c3ed62aed6..7985afb154 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -61,21 +61,6 @@ obj-$(CONFIG_CMD_HDMIDETECT) += cmd_hdmidet.o
>  obj-$(CONFIG_CMD_DEKBLOB) += cmd_dek.o
>  endif
>
> -ifneq ($(CONFIG_BOARD_SIZE_LIMIT),)
> -BOARD_SIZE_CHECK = \
> -        @actual=`wc -c $@ | awk '{print $$1}'`; \
> -        limit=`printf "%d" $(CONFIG_BOARD_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
> -BOARD_SIZE_CHECK =
> -endif
> -
>  PLUGIN = board/$(BOARDDIR)/plugin
>
>  ifeq ($(CONFIG_USE_IMXIMG_PLUGIN),y)
> @@ -124,7 +109,6 @@ u-boot.imx: MKIMAGEOUTPUT = u-boot.imx.log
>
>  u-boot.imx: u-boot.bin u-boot.cfgout $(PLUGIN).bin FORCE
>  	$(call if_changed,mkimage)
> -	$(BOARD_SIZE_CHECK)
>
>  ifeq ($(CONFIG_OF_SEPARATE),y)
>  MKIMAGEFLAGS_u-boot-dtb.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) \
> --
> 2.20.1
>
>

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

* [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
  2019-04-04 20:53   ` Heinrich Schuchardt
@ 2019-04-05 13:05   ` Fabio Estevam
  2019-04-07 21:10     ` Heinrich Schuchardt
  2019-06-07 22:04   ` Tom Rini
  2 siblings, 1 reply; 27+ messages in thread
From: Fabio Estevam @ 2019-04-05 13:05 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Tue, Apr 2, 2019 at 2:19 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> We currently have duplicate definitions for BOARD_SIZE_CHECK in Makefile
> and arch/arm/mach-imx/Makefile.
>
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>
> Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like an error
> like the following is thrown:
>
> u-boot-dtb.imx exceeds file size limit:
>   limit:  503696 bytes
>   actual: 509720 bytes
>   excess: 6024 bytes
> make: *** [Makefile:1051: u-boot-dtb.imx] Error 1
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Yes, it makes sense. No need for a imx specific size check:

Reviewed-by: Fabio Estevam <festevam@gmail.com>

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

* [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile
  2019-04-05 13:05   ` Fabio Estevam
@ 2019-04-07 21:10     ` Heinrich Schuchardt
  0 siblings, 0 replies; 27+ messages in thread
From: Heinrich Schuchardt @ 2019-04-07 21:10 UTC (permalink / raw)
  To: u-boot

On 4/5/19 3:05 PM, Fabio Estevam wrote:
> Hi Heinrich,
>
> On Tue, Apr 2, 2019 at 2:19 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> We currently have duplicate definitions for BOARD_SIZE_CHECK in Makefile
>> and arch/arm/mach-imx/Makefile.
>>
>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>
>> Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like an error
>> like the following is thrown:
>>
>> u-boot-dtb.imx exceeds file size limit:
>>   limit:  503696 bytes
>>   actual: 509720 bytes
>>   excess: 6024 bytes
>> make: *** [Makefile:1051: u-boot-dtb.imx] Error 1
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Yes, it makes sense. No need for a imx specific size check:
>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>
Thanks for reviewing.

@Stefano: I have assigned this single patch to you in patchwork to
please pick-up for the imx tree.

Best regards

Heinrich

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
                   ` (5 preceding siblings ...)
  2019-04-03  0:59 ` Kever Yang
@ 2019-04-18 20:12 ` Simon Goldschmidt
  2019-04-22 14:36   ` Tom Rini
  6 siblings, 1 reply; 27+ messages in thread
From: Simon Goldschmidt @ 2019-04-18 20:12 UTC (permalink / raw)
  To: u-boot

Heinrich,

On 02.04.19 19:19, Heinrich Schuchardt wrote:
> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> up to 2 KiB for the file header.
> 
> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> the board specific limit.
> 
> A common Makefile function is used for this test and the test against
> CONFIG_BOARD_SIZE_LIMIT.
> 
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.

Has anything from this series been applied? I now have a working patch 
that applies on top of this and adds a full SPL SRAM size check 
(including HEAP, GD and stack; via a host tool) which works for socfpga 
(as an example of a platform where SPL binary is loaded to limited SRAM).

Actually, my patch would replace your patch 3/4 but build on 1/4 (2/4 
and 4/4 are arch-specific).

How should we proceed here? I could send a series including your 1/4, or 
I could send a series completely building on this series, at the 
downside of more or less reverting your 2/4.

Regards,
Simon

> 
> v4:
> 	use a common function for all size checks in the Makefiles
> 
> Heinrich Schuchardt (4):
>    Makefile: reusable function for BOARD_SIZE_CHECK
>    imx: move BOARD_SIZE_CHECK to main Makefile
>    configs: define CONFIG_SPL_SIZE_LIMIT
>    configs: rk3288: Tinker Board SPL file must fit into 32 KiB
> 
>   Kconfig                         |  8 ++++++++
>   Makefile                        | 33 +++++++++++++++++++++++----------
>   arch/arm/mach-imx/Makefile      | 16 ----------------
>   configs/tinker-rk3288_defconfig |  1 +
>   4 files changed, 32 insertions(+), 26 deletions(-)
> 
> --
> 2.20.1
> 

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-18 20:12 ` Simon Goldschmidt
@ 2019-04-22 14:36   ` Tom Rini
  2019-04-22 18:40     ` Simon Goldschmidt
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Rini @ 2019-04-22 14:36 UTC (permalink / raw)
  To: u-boot

On Thu, Apr 18, 2019 at 10:12:36PM +0200, Simon Goldschmidt wrote:
> Heinrich,
> 
> On 02.04.19 19:19, Heinrich Schuchardt wrote:
> >The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >up to 2 KiB for the file header.
> >
> >A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >the board specific limit.
> >
> >A common Makefile function is used for this test and the test against
> >CONFIG_BOARD_SIZE_LIMIT.
> >
> >Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> 
> Has anything from this series been applied? I now have a working patch that
> applies on top of this and adds a full SPL SRAM size check (including HEAP,
> GD and stack; via a host tool) which works for socfpga (as an example of a
> platform where SPL binary is loaded to limited SRAM).
> 
> Actually, my patch would replace your patch 3/4 but build on 1/4 (2/4 and
> 4/4 are arch-specific).
> 
> How should we proceed here? I could send a series including your 1/4, or I
> could send a series completely building on this series, at the downside of
> more or less reverting your 2/4.

What I would like to see is i.MX (and rockchip) converted to use your
new test as well (also, yay!  Thanks for following up on that!) and we
drop the existing check here.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190422/803f9a4c/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-22 14:36   ` Tom Rini
@ 2019-04-22 18:40     ` Simon Goldschmidt
  2019-04-22 19:29       ` Tom Rini
  0 siblings, 1 reply; 27+ messages in thread
From: Simon Goldschmidt @ 2019-04-22 18:40 UTC (permalink / raw)
  To: u-boot

Am 22.04.2019 um 16:36 schrieb Tom Rini:
> On Thu, Apr 18, 2019 at 10:12:36PM +0200, Simon Goldschmidt wrote:
>> Heinrich,
>>
>> On 02.04.19 19:19, Heinrich Schuchardt wrote:
>>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>>> up to 2 KiB for the file header.
>>>
>>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>>> the board specific limit.
>>>
>>> A common Makefile function is used for this test and the test against
>>> CONFIG_BOARD_SIZE_LIMIT.
>>>
>>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>
>> Has anything from this series been applied? I now have a working patch that
>> applies on top of this and adds a full SPL SRAM size check (including HEAP,
>> GD and stack; via a host tool) which works for socfpga (as an example of a
>> platform where SPL binary is loaded to limited SRAM).
>>
>> Actually, my patch would replace your patch 3/4 but build on 1/4 (2/4 and
>> 4/4 are arch-specific).
>>
>> How should we proceed here? I could send a series including your 1/4, or I
>> could send a series completely building on this series, at the downside of
>> more or less reverting your 2/4.
> 
> What I would like to see is i.MX (and rockchip) converted to use your
> new test as well (also, yay!  Thanks for following up on that!) and we
> drop the existing check here.
> 

Ok, so while I cannot really help on i.MX and rockchip, why don't you 
accept this series from Heinrich and I'll send my patch as followup? 
Then we can discuss this with i.MX and rockchip maintainers.

Regards,
Simon

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-22 18:40     ` Simon Goldschmidt
@ 2019-04-22 19:29       ` Tom Rini
  2019-04-22 19:50         ` Simon Goldschmidt
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Rini @ 2019-04-22 19:29 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 22, 2019 at 08:40:52PM +0200, Simon Goldschmidt wrote:
> Am 22.04.2019 um 16:36 schrieb Tom Rini:
> >On Thu, Apr 18, 2019 at 10:12:36PM +0200, Simon Goldschmidt wrote:
> >>Heinrich,
> >>
> >>On 02.04.19 19:19, Heinrich Schuchardt wrote:
> >>>The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> >>>up to 2 KiB for the file header.
> >>>
> >>>A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> >>>the board specific limit.
> >>>
> >>>A common Makefile function is used for this test and the test against
> >>>CONFIG_BOARD_SIZE_LIMIT.
> >>>
> >>>Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> >>
> >>Has anything from this series been applied? I now have a working patch that
> >>applies on top of this and adds a full SPL SRAM size check (including HEAP,
> >>GD and stack; via a host tool) which works for socfpga (as an example of a
> >>platform where SPL binary is loaded to limited SRAM).
> >>
> >>Actually, my patch would replace your patch 3/4 but build on 1/4 (2/4 and
> >>4/4 are arch-specific).
> >>
> >>How should we proceed here? I could send a series including your 1/4, or I
> >>could send a series completely building on this series, at the downside of
> >>more or less reverting your 2/4.
> >
> >What I would like to see is i.MX (and rockchip) converted to use your
> >new test as well (also, yay!  Thanks for following up on that!) and we
> >drop the existing check here.
> 
> Ok, so while I cannot really help on i.MX and rockchip, why don't you accept
> this series from Heinrich and I'll send my patch as followup? Then we can
> discuss this with i.MX and rockchip maintainers.

Can you post your series now, and we can move from there?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190422/41964e3e/attachment.sig>

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

* [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-22 19:29       ` Tom Rini
@ 2019-04-22 19:50         ` Simon Goldschmidt
  0 siblings, 0 replies; 27+ messages in thread
From: Simon Goldschmidt @ 2019-04-22 19:50 UTC (permalink / raw)
  To: u-boot



On 22.04.19 21:29, Tom Rini wrote:
> On Mon, Apr 22, 2019 at 08:40:52PM +0200, Simon Goldschmidt wrote:
>> Am 22.04.2019 um 16:36 schrieb Tom Rini:
>>> On Thu, Apr 18, 2019 at 10:12:36PM +0200, Simon Goldschmidt wrote:
>>>> Heinrich,
>>>>
>>>> On 02.04.19 19:19, Heinrich Schuchardt wrote:
>>>>> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
>>>>> up to 2 KiB for the file header.
>>>>>
>>>>> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
>>>>> the board specific limit.
>>>>>
>>>>> A common Makefile function is used for this test and the test against
>>>>> CONFIG_BOARD_SIZE_LIMIT.
>>>>>
>>>>> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
>>>>
>>>> Has anything from this series been applied? I now have a working patch that
>>>> applies on top of this and adds a full SPL SRAM size check (including HEAP,
>>>> GD and stack; via a host tool) which works for socfpga (as an example of a
>>>> platform where SPL binary is loaded to limited SRAM).
>>>>
>>>> Actually, my patch would replace your patch 3/4 but build on 1/4 (2/4 and
>>>> 4/4 are arch-specific).
>>>>
>>>> How should we proceed here? I could send a series including your 1/4, or I
>>>> could send a series completely building on this series, at the downside of
>>>> more or less reverting your 2/4.
>>>
>>> What I would like to see is i.MX (and rockchip) converted to use your
>>> new test as well (also, yay!  Thanks for following up on that!) and we
>>> drop the existing check here.
>>
>> Ok, so while I cannot really help on i.MX and rockchip, why don't you accept
>> this series from Heinrich and I'll send my patch as followup? Then we can
>> discuss this with i.MX and rockchip maintainers.
> 
> Can you post your series now, and we can move from there?  Thanks!

Did that:
https://patchwork.ozlabs.org/patch/1088865/
[Depends on this series from Heinrich]

Regards,
Simon

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

* [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK Heinrich Schuchardt
@ 2019-06-07 22:04   ` Tom Rini
  0 siblings, 0 replies; 27+ messages in thread
From: Tom Rini @ 2019-06-07 22:04 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:19:04PM +0200, Heinrich Schuchardt wrote:

> Carve out function size_check from macro BOARD_SIZE_CHECK. This will allow
> us to reuse the function for other file size checks.
> 
> Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like the
> following is thrown:
> 
> u-boot-dtb.img exceeds file size limit:
>   limit:  409516 bytes
>   actual: 444346 bytes
>   excess: 34830 bytes
> make: *** [Makefile:1212: u-boot-dtb.img] Error 1
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190607/750143df/attachment.sig>

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

* [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
  2019-04-04 20:53   ` Heinrich Schuchardt
  2019-04-05 13:05   ` Fabio Estevam
@ 2019-06-07 22:04   ` Tom Rini
  2 siblings, 0 replies; 27+ messages in thread
From: Tom Rini @ 2019-06-07 22:04 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:19:05PM +0200, Heinrich Schuchardt wrote:

> We currently have duplicate definitions for BOARD_SIZE_CHECK in Makefile
> and arch/arm/mach-imx/Makefile.
> 
> Move the board size check from arch/arm/mach-imx/Makefile to Makefile.
> 
> Depending on the value of CONFIG_BOARD_SIZE_LIMIT an error like an error
> like the following is thrown:
> 
> u-boot-dtb.imx exceeds file size limit:
>   limit:  503696 bytes
>   actual: 509720 bytes
>   excess: 6024 bytes
> make: *** [Makefile:1051: u-boot-dtb.imx] Error 1
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190607/e53cff7d/attachment.sig>

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

* [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT Heinrich Schuchardt
@ 2019-06-07 22:05   ` Tom Rini
  2019-06-07 22:13     ` Fabio Estevam
  0 siblings, 1 reply; 27+ messages in thread
From: Tom Rini @ 2019-06-07 22:05 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:19:06PM +0200, Heinrich Schuchardt wrote:

> A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> the board specific maximum size for the SPL file.
> 
> Use Makefile function size_check() to implement the test.
> 
> Depending on the size of CONFIG_SPL_SIZE_LIMIT an error like the following
> is thrown:
> 
> spl/u-boot-spl.bin exceeds file size limit:
>   limit:  30720 bytes
>   actual: 33426 bytes
>   excess: 2706 bytes
> make: *** [Makefile:1663: spl/u-boot-spl.bin] Error 1
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190607/a5602e4b/attachment.sig>

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

* [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB
  2019-04-02 17:19 ` [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
@ 2019-06-07 22:05   ` Tom Rini
  0 siblings, 0 replies; 27+ messages in thread
From: Tom Rini @ 2019-06-07 22:05 UTC (permalink / raw)
  To: u-boot

On Tue, Apr 02, 2019 at 07:19:07PM +0200, Heinrich Schuchardt wrote:

> The SPL image for the Tinker Board has to fit into 32 KiB. This includes
> up to 2 KiB for the file header.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190607/d8e57db1/attachment.sig>

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

* [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT
  2019-06-07 22:05   ` Tom Rini
@ 2019-06-07 22:13     ` Fabio Estevam
  2019-06-08  0:38       ` Tom Rini
  0 siblings, 1 reply; 27+ messages in thread
From: Fabio Estevam @ 2019-06-07 22:13 UTC (permalink / raw)
  To: u-boot

Hi Tom and Heinrich,

On Fri, Jun 7, 2019 at 7:06 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Apr 02, 2019 at 07:19:06PM +0200, Heinrich Schuchardt wrote:
>
> > A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> > the board specific maximum size for the SPL file.
> >
> > Use Makefile function size_check() to implement the test.
> >
> > Depending on the size of CONFIG_SPL_SIZE_LIMIT an error like the following
> > is thrown:
> >
> > spl/u-boot-spl.bin exceeds file size limit:
> >   limit:  30720 bytes
> >   actual: 33426 bytes
> >   excess: 2706 bytes
> > make: *** [Makefile:1663: spl/u-boot-spl.bin] Error 1
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Applied to u-boot/master, thanks!

Just tested this on mx6sabresd_defconfig, where the SPL size limit is
68 * 1024, so I passed:

CONFIG_SPL_SIZE_LIMIT=69632

but SPL builds without error and the generated size is 76800.

Any ideas why CONFIG_SPL_SIZE_LIMIT did not catch the SPL size
overflow for mx6sabresd_defconfig?

Thanks

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

* [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT
  2019-06-07 22:13     ` Fabio Estevam
@ 2019-06-08  0:38       ` Tom Rini
  0 siblings, 0 replies; 27+ messages in thread
From: Tom Rini @ 2019-06-08  0:38 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 07, 2019 at 07:13:47PM -0300, Fabio Estevam wrote:
> Hi Tom and Heinrich,
> 
> On Fri, Jun 7, 2019 at 7:06 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Apr 02, 2019 at 07:19:06PM +0200, Heinrich Schuchardt wrote:
> >
> > > A new configuration variable CONFIG_SPL_SIZE_LIMIT is introduced to define
> > > the board specific maximum size for the SPL file.
> > >
> > > Use Makefile function size_check() to implement the test.
> > >
> > > Depending on the size of CONFIG_SPL_SIZE_LIMIT an error like the following
> > > is thrown:
> > >
> > > spl/u-boot-spl.bin exceeds file size limit:
> > >   limit:  30720 bytes
> > >   actual: 33426 bytes
> > >   excess: 2706 bytes
> > > make: *** [Makefile:1663: spl/u-boot-spl.bin] Error 1
> > >
> > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > Applied to u-boot/master, thanks!
> 
> Just tested this on mx6sabresd_defconfig, where the SPL size limit is
> 68 * 1024, so I passed:
> 
> CONFIG_SPL_SIZE_LIMIT=69632
> 
> but SPL builds without error and the generated size is 76800.
> 
> Any ideas why CONFIG_SPL_SIZE_LIMIT did not catch the SPL size
> overflow for mx6sabresd_defconfig?

So, I hit this too.  SPL_SIZE_LIMIT is hex, not int.  Or rather, Simon's
patch changes it from int to hex, which, yeah, that's wrong.  Thanks for
the report, patch coming up!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190607/c156ec04/attachment.sig>

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

end of thread, other threads:[~2019-06-08  0:38 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-02 17:19 [U-Boot] [PATCH v4 0/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
2019-04-02 17:19 ` [U-Boot] [PATCH v4 1/4] Makefile: reusable function for BOARD_SIZE_CHECK Heinrich Schuchardt
2019-06-07 22:04   ` Tom Rini
2019-04-02 17:19 ` [U-Boot] [PATCH v4 2/4] imx: move BOARD_SIZE_CHECK to main Makefile Heinrich Schuchardt
2019-04-04 20:53   ` Heinrich Schuchardt
2019-04-05 13:05   ` Fabio Estevam
2019-04-07 21:10     ` Heinrich Schuchardt
2019-06-07 22:04   ` Tom Rini
2019-04-02 17:19 ` [U-Boot] [PATCH v4 3/4] configs: define CONFIG_SPL_SIZE_LIMIT Heinrich Schuchardt
2019-06-07 22:05   ` Tom Rini
2019-06-07 22:13     ` Fabio Estevam
2019-06-08  0:38       ` Tom Rini
2019-04-02 17:19 ` [U-Boot] [PATCH v4 4/4] configs: rk3288: Tinker Board SPL file must fit into 32 KiB Heinrich Schuchardt
2019-06-07 22:05   ` Tom Rini
2019-04-02 17:29 ` [U-Boot] [PATCH v4 0/4] " Tom Rini
2019-04-02 17:33   ` Heinrich Schuchardt
2019-04-02 17:47     ` Tom Rini
2019-04-02 18:17       ` Heinrich Schuchardt
2019-04-02 18:08     ` Simon Goldschmidt
2019-04-03  0:59 ` Kever Yang
2019-04-03  6:19   ` Heinrich Schuchardt
2019-04-03 20:50     ` Benoît Thébaudeau
2019-04-18 20:12 ` Simon Goldschmidt
2019-04-22 14:36   ` Tom Rini
2019-04-22 18:40     ` Simon Goldschmidt
2019-04-22 19:29       ` Tom Rini
2019-04-22 19:50         ` Simon Goldschmidt

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.