[PATCH] Makefile: doesn't need check stack size when dtb is not built

[Stephen Warren]

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)