From mboxrd@z Thu Jan  1 00:00:00 1970
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Thu, 5 Mar 2020 09:39:18 +0900
Subject: [PATCH] Makefile: doesn't need check stack size when dtb is not
 built
In-Reply-To: <fc0853a9-3c7d-b3f6-141d-e6509a179e21@wwwdotorg.org>
References: <20200304065450.1068-1-takahiro.akashi@linaro.org>
 <a8def7d9-9479-fbe0-05cf-5dcafbf4fc9c@wwwdotorg.org>
 <20200305001541.GG13880@linaro.org>
 <fc0853a9-3c7d-b3f6-141d-e6509a179e21@wwwdotorg.org>
Message-ID: <20200305003917.GH13880@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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?)

Thanks,
-Takahiro Akashi