[U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL

[U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL

[Kever Yang]

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 <kever.yang@rock-chips.com>
---

 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
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 655727f431..19a6775b0d 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -35,8 +35,8 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
 obj-$(CONFIG_CMD_BOOTZ) += bootm.o zimage.o
 obj-$(CONFIG_SYS_L2_PL310) += cache-pl310.o
 else
-obj-$(CONFIG_SPL_FRAMEWORK) += spl.o
-obj-$(CONFIG_SPL_FRAMEWORK) += zimage.o
+obj-$(CONFIG_$(SPL_TPL_)FRAMEWORK) += spl.o
+obj-$(CONFIG_$(SPL_TPL_)FRAMEWORK) += zimage.o
 obj-$(CONFIG_OF_LIBFDT) += bootm-fdt.o
 endif
 obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o
diff --git a/arch/arm/lib/crt0.S b/arch/arm/lib/crt0.S
index 538d78c0b8..f1a8a7e7dc 100644
--- a/arch/arm/lib/crt0.S
+++ b/arch/arm/lib/crt0.S
@@ -120,7 +120,11 @@ here:
 
 	bl	c_runtime_cpu_setup	/* we still call old routine here */
 #endif
-#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_FRAMEWORK)
+#if !defined(CONFIG_SPL_BUILD) || \
+	(defined(CONFIG_TPL_BUILD) && defined(CONFIG_TPL_FRAMEWORK)) || \
+	(defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) && \
+	defined(CONFIG_SPL_FRAMEWORK)) \
+
 # ifdef CONFIG_SPL_BUILD
 	/* Use a DRAM stack for the rest of SPL, if requested */
 	bl	spl_relocate_stack_gd
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.
+
 if TPL
 
 config TPL_BOARD_INIT
diff --git a/common/spl/Makefile b/common/spl/Makefile
index a130a5be4b..dd2fc7a5d3 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -7,7 +7,7 @@
 #
 
 ifdef CONFIG_SPL_BUILD
-obj-$(CONFIG_SPL_FRAMEWORK) += spl.o
+obj-$(CONFIG_$(SPL_TPL_)FRAMEWORK) += spl.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTROM_SUPPORT) += spl_bootrom.o
 obj-$(CONFIG_$(SPL_TPL_)LOAD_FIT) += spl_fit.o
 obj-$(CONFIG_$(SPL_TPL_)NOR_SUPPORT) += spl_nor.o
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 252f13826d..43949cdba1 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -64,7 +64,11 @@ HAVE_VENDOR_COMMON_LIB = $(if $(wildcard $(srctree)/board/$(VENDOR)/common/Makef
 libs-y += $(if $(BOARDDIR),board/$(BOARDDIR)/)
 libs-$(HAVE_VENDOR_COMMON_LIB) += board/$(VENDOR)/common/
 
+ifeq ($(CONFIG_TPL_BUILD),y)
+libs-$(CONFIG_TPL_FRAMEWORK) += common/spl/
+else
 libs-$(CONFIG_SPL_FRAMEWORK) += common/spl/
+endif
 libs-y += common/init/
 
 # Special handling for a few options which support SPL/TPL
-- 
2.18.0

[U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL

[Tom Rini]

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 <kever.yang@rock-chips.com>
> ---
> 
>  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.

[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!

-- 
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/20180828/a466498d/attachment.sig>

[U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL

[Kever Yang]

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 <kever.yang@rock-chips.com>
>> ---
>>
>>  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.

Thanks,
- Kever

[U-Boot] [PATCH] TPL/SPL: add separate CONFIG_TPL_FRAMEWORK for TPL

[Tom Rini]

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 <kever.yang@rock-chips.com>
> >> ---
> >>
> >>  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: <http://lists.denx.de/pipermail/u-boot/attachments/20180828/43664362/attachment.sig>