On 3/4/20 5:39 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 05:22:25PM -0700, Stephen Warren wrote:
On 3/4/20 5:15 PM, AKASHI Takahiro wrote:
On Wed, Mar 04, 2020 at 09:21:29AM -0700, Stephen Warren wrote:
On 3/3/20 11:54 PM, AKASHI Takahiro wrote:
The commit 5fed97af20da ("Makefile: ensure DTB doesn't overflow into
initial stack") adds an extra check for stack size in BSS if
CONFIG_SYS_INIT_SP_BSS_OFFSET is enabled.
This check, however, doesn't make sense under the configuration where
control dtb won't be built in and it should be void in such cases.
Don't you want to edit the following hunk from the original patch instead or
as well?
+ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
+ALL-y += init_sp_bss_offset_check
+endif
That's why there's no rule for init_sp_bss_offset_check in the else case.
I intentionally left it as it is because someone may in the future
want to add other *sanity checks* whether dtb is used or not.
I'd probably expect the following in that case:
ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)
ALL-y += init_sp_bss_offset_check
endif
ALL-y += some_other_check
endif
Rather, my concern is:
Is "ifneq ($(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_OF_HOSTFILE),)"
sufficient and appropriate to guard the check?
I think OF_SEPARATE is the only case this check makes sense. OF_EMBED sounds
like the build process puts the DTB into .data or similar, so the issue
doesn't apply. Since OF_HOSTFILE is a sandbox thing where the DT is read
from a file, the check definitely doesn't apply.
Okay, sounds reasonable.
(and the target should be dts/dt.dtb rather than u-boot.dtb for clarity?)
By "target" I assume you mean the DTB filename in the following?
@dtb_size=$(shell wc -c u-boot.dtb | awk '{print $$1}') ; \
If so, it doesn't make any difference; u-boot.dtb is just a copy of
dts/dt.dtb:
u-boot.dtb: dts/dt.dtb
$(call cmd,copy)
