From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Tue, 28 Aug 2018 21:55:44 -0400 Subject: [U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL In-Reply-To: <75a74c76-09ca-8d8a-0a48-733e04df37d0@rock-chips.com> References: <20180828085126.27562-1-kever.yang@rock-chips.com> <20180828124939.GT26633@bill-the-cat> <75a74c76-09ca-8d8a-0a48-733e04df37d0@rock-chips.com> Message-ID: <20180829015544.GY26633@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Tue, Aug 28, 2018 at 09:11:35PM +0800, Kever Yang wrote: > Hi Tom, > > > On 08/28/2018 08:49 PM, Tom Rini wrote: > > On Tue, Aug 28, 2018 at 04:51:26PM +0800, Kever Yang wrote: > > > >> SPL_FRAMEWORK is a set of framework feature, we may not need the > >> fromework for both TPL and SPL at the same time, so add a separate > >> one for TPL. > >> > >> Signed-off-by: Kever Yang > >> --- > >> > >> Makefile | 2 +- > >> arch/arm/lib/Makefile | 4 ++-- > >> arch/arm/lib/crt0.S | 6 +++++- > >> common/spl/Kconfig | 9 +++++++++ > >> common/spl/Makefile | 2 +- > >> scripts/Makefile.spl | 4 ++++ > >> 6 files changed, 22 insertions(+), 5 deletions(-) > >> > >> diff --git a/Makefile b/Makefile > >> index 4b3023b259..68b77d1e43 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -822,7 +822,7 @@ endif > >> endif > >> ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin > >> ALL-$(CONFIG_OF_SEPARATE) += u-boot.dtb > >> -ifeq ($(CONFIG_SPL_FRAMEWORK),y) > >> +ifeq ($(CONFIG_TPL_FRAMEWORK),y) > >> ALL-$(CONFIG_OF_SEPARATE) += u-boot-dtb.img > >> endif > >> ALL-$(CONFIG_OF_HOSTFILE) += u-boot.dtb > > We should be adding a new stanza here and not replacing the SPL one I > > would think. > This may not need,  I though this is for TPL because it's just after > target for CONFIG_TPL. > Will drop this part next version. > > > > [snip] > >> diff --git a/common/spl/Kconfig b/common/spl/Kconfig > >> index db1915fe5c..1a2e10b892 100644 > >> --- a/common/spl/Kconfig > >> +++ b/common/spl/Kconfig > >> @@ -813,6 +813,15 @@ config TPL > >> help > >> If you want to build TPL as well as the normal image and SPL, say Y. > >> > >> +config TPL_FRAMEWORK > >> + bool "Support TPL based upon the common TPL framework" > >> + depends on TPL > >> + default y > >> + help > >> + Enable the TPL framework under common/spl/. This framework > >> + re-use the all the framework feature from SPL but enable separetely. > >> + If unsure, say Y. > > This shouldn't be default y, and the help shouldn't suggest that either > > I think. My concern with this series is TPL wasn't intended to get too > > featureful. Our initial constraints are enough that we need something > > to init DDR and load SPL into that as we can't do enough with our > > limited resources to load full U-Boot. In fact, looking at the follow > > up rockchip specific patch that's more in line what my expectations. So > > I think you should be able to get TPL to do what you want without > > introducing TPL_FRAMEWORK. Thanks! > I have to introducing  TPL_FRAMEWORK because the SPL_FRAMEWORK > is shared by TPL and SPL, if I have enable TPL and SPL, then: > - I would like to use TPL without framework(which refers to DM, common > lib and FDT); > - I would like to use SPL with SPL_FRAMEWORK > If there is no TPL_FRAMEWORK, then I'm not sure if I can use the > TINY_FRAMEWORK > to overwrite everything. > > > I introduce TINY_FRAMEWORK is to remove other program like vector table, > runtime C init, which is not include in SPL_FRAMEWORK now. First, please make sure that your next series here has been through travis-ci or you've built some of the PowerPC targets (say ./tools/buildman/buildman.py p1_p2) as I'm pretty sure TPL_FRAMEWORK being on by default breaks them. Next, I'm still not quite following, sorry. The way TPL is used normally today (again, see PowerPC for the non-rockchip examples) does not rely on the spl framework, and that's intentional as they have extremely limited resources. I still think you should be able to rework things such that you don't need TPL_FRAMEWORK being set to them overwrite most of it. Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: