[U-Boot] [PATCH v2 1/2] Makefile: Add target to generate hex output for combined spl and dtb

[U-Boot] [PATCH v2 1/2] Makefile: Add target to generate hex output for combined spl and dtb

[Dalon Westergreen]

From: Dalon Westergreen <dalon.westergreen@intel.com>

Some architectures, Stratix10, require a hex formatted spl that combines
the spl image and dtb.  This adds a target to create said hex file with
and offset of SPL_TEXT_BASE.

Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>

---
Changes in v2:
 -> Move spl hex file generation to SPL Makefile
 -> Create hexfile from $(SPL_BIN).bin which will include the dtb
    ifneq(build_dtb,)
---
 Makefile             | 9 ++++-----
 scripts/Makefile.spl | 8 ++++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index 07106138e9..12e36ebb72 100644
--- a/Makefile
+++ b/Makefile
@@ -1124,11 +1124,6 @@ OBJCOPYFLAGS_u-boot-nodtb.bin := -O binary \
 		$(if $(CONFIG_X86_16BIT_INIT),-R .start16 -R .resetvec) \
 		$(if $(CONFIG_MPC85XX_HAVE_RESET_VECTOR),-R .bootpg -R .resetvec)
 
-OBJCOPYFLAGS_u-boot-spl.hex = $(OBJCOPYFLAGS_u-boot.hex)
-
-spl/u-boot-spl.hex: spl/u-boot-spl FORCE
-	$(call if_changed,objcopy)
-
 binary_size_check: u-boot-nodtb.bin FORCE
 	@file_size=$(shell wc -c u-boot-nodtb.bin | awk '{print $$1}') ; \
 	map_size=$(shell cat u-boot.map | \
@@ -1707,6 +1702,10 @@ u-boot.lds: $(LDSCRIPT) prepare FORCE
 
 spl/u-boot-spl.bin: spl/u-boot-spl
 	@:
+
+spl/u-boot-spl.hex: spl/u-boot-spl
+	@:
+
 spl/u-boot-spl: tools prepare \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_SPL_OF_PLATDATA),dts/dt.dtb) \
 		$(if $(CONFIG_OF_SEPARATE)$(CONFIG_OF_EMBED)$(CONFIG_TPL_OF_PLATDATA),dts/dt.dtb)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 7af6b120b6..419bb6e222 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -216,6 +216,8 @@ ifneq ($(CONFIG_TARGET_SOCFPGA_GEN5)$(CONFIG_TARGET_SOCFPGA_ARRIA10),)
 ALL-y	+= $(obj)/$(SPL_BIN).sfp
 endif
 
+ALL-$(CONFIG_TARGET_SOCFPGA_STRATIX10) += $(obj)/$(SPL_BIN).hex
+
 ifdef CONFIG_ARCH_SUNXI
 ALL-y	+= $(obj)/sunxi-spl.bin
 
@@ -363,6 +365,11 @@ endif
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
 	$(call if_changed,mkimage)
 
+OBJCOPYFLAGS_$(SPL_BIN).hex := -I binary -O ihex --change-address=$(CONFIG_SPL_TEXT_BASE)
+
+$(obj)/$(SPL_BIN).hex: $(obj)/u-boot-spl.bin FORCE
+	$(call if_changed,objcopy)
+
 quiet_cmd_mksunxiboot = MKSUNXI $@
 cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \
 			--default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@
@@ -463,3 +470,4 @@ ifdef CONFIG_ARCH_K3
 tispl.bin: $(obj)/u-boot-spl-nodtb.bin $(SHRUNK_ARCH_DTB) $(SPL_ITS) FORCE
 	$(call if_changed,mkfitimage)
 endif
+
-- 
2.21.0

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Dalon Westergreen]

From: Dalon Westergreen <dalon.westergreen@intel.com>

CONFIG_OF_EMBED was primarily enabled to support the stratix10
spl hex file requirements.  Since this option now produces a
warning during build, and the spl hex can be created using
alternate methods, CONFIG_OF_EMBED is no longer needed.

Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>

---
Changes in v2:
 -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex
---
 configs/socfpga_stratix10_defconfig       | 1 -
 include/configs/socfpga_stratix10_socdk.h | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
index fbab388b43..f27180385d 100644
--- a/configs/socfpga_stratix10_defconfig
+++ b/configs/socfpga_stratix10_defconfig
@@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y
 CONFIG_CMD_EXT4=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
-CONFIG_OF_EMBED=y
 CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_NET_RANDOM_ETHADDR=y
diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h
index 39d757d737..66855ff0d8 100644
--- a/include/configs/socfpga_stratix10_socdk.h
+++ b/include/configs/socfpga_stratix10_socdk.h
@@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void);
 
 /* SPL SDMMC boot support */
 #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION	1
-#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot.img"
+#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME		"u-boot-dtb.img"
 
 #endif	/* __CONFIG_H */
-- 
2.21.0

[U-Boot] [PATCH v2 1/2] Makefile: Add target to generate hex output for combined spl and dtb

[Marek Vasut]

On 6/4/19 1:57 AM, Dalon Westergreen wrote:
> From: Dalon Westergreen <dalon.westergreen@intel.com>
> 
> Some architectures, Stratix10, require a hex formatted spl that combines
> the spl image and dtb.  This adds a target to create said hex file with
> and offset of SPL_TEXT_BASE.
> 
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> 

[...]

> @@ -363,6 +365,11 @@ endif
>  $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
>  	$(call if_changed,mkimage)
>  
> +OBJCOPYFLAGS_$(SPL_BIN).hex := -I binary -O ihex --change-address=$(CONFIG_SPL_TEXT_BASE)

Do we really need to do it here ? The commit message is not clear why
this is needed ; I think if you link the SPl against the correct
address, this should not be needed.

> +$(obj)/$(SPL_BIN).hex: $(obj)/u-boot-spl.bin FORCE
> +	$(call if_changed,objcopy)
> +
>  quiet_cmd_mksunxiboot = MKSUNXI $@
>  cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \
>  			--default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@
> @@ -463,3 +470,4 @@ ifdef CONFIG_ARCH_K3
>  tispl.bin: $(obj)/u-boot-spl-nodtb.bin $(SHRUNK_ARCH_DTB) $(SPL_ITS) FORCE
>  	$(call if_changed,mkfitimage)
>  endif
> +

Drop this hunk

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 1/2] Makefile: Add target to generate hex output for combined spl and dtb

[Dalon L Westergreen]

On Tue, 2019-06-04 at 02:00 +0200, Marek Vasut wrote:
> On 6/4/19 1:57 AM, Dalon Westergreen wrote:
> > From: Dalon Westergreen <
> > dalon.westergreen at intel.com
> > >
> > 
> > Some architectures, Stratix10, require a hex formatted spl that combines
> > the spl image and dtb.  This adds a target to create said hex file with
> > and offset of SPL_TEXT_BASE.
> > 
> > Signed-off-by: Dalon Westergreen <
> > dalon.westergreen at intel.com
> > >
> > 
> 
> [...]
> 
> > @@ -363,6 +365,11 @@ endif
> >  $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
> >  	$(call if_changed,mkimage)
> >  
> > +OBJCOPYFLAGS_$(SPL_BIN).hex := -I binary -O ihex --change-
> > address=$(CONFIG_SPL_TEXT_BASE)
> 
> Do we really need to do it here ? The commit message is not clear why
> this is needed ; I think if you link the SPl against the correct
> address, this should not be needed.
> 

This objcopy is from the binary including the dtb and not the elf.  If you
objcopy using the elf, and link to the correct address, you are correct.  It
is not true when just taking a binary and converting to a hex file.  The
binary combined with the dtb is what is needed.

I can try be more descriptive in the commit message.

perhaps..

---
Stratix10 requires a hex image of the spl plus spl devicetree offset to 
the Stratix10 onchip memory located at SPL_TEXT_BASE.  This patch adds
a target to generate a hex file from the u-boot-spl binary including the
dtb offset at SPL_TEST_BASE.
---

> > +$(obj)/$(SPL_BIN).hex: $(obj)/u-boot-spl.bin FORCE
> > +	$(call if_changed,objcopy)
> > +
> >  quiet_cmd_mksunxiboot = MKSUNXI $@
> >  cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \
> >  			--default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@
> > @@ -463,3 +470,4 @@ ifdef CONFIG_ARCH_K3
> >  tispl.bin: $(obj)/u-boot-spl-nodtb.bin $(SHRUNK_ARCH_DTB) $(SPL_ITS) FORCE
> >  	$(call if_changed,mkfitimage)
> >  endif
> > +
> 
> Drop this hunk

Will do, and i likely should have used (SPL_BIN).bin rather than
u-boot-spl.bin.

--dalon
> 
> 

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Simon Goldschmidt]

On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen
<dalon.westergreen@linux.intel.com> wrote:
>
> From: Dalon Westergreen <dalon.westergreen@intel.com>
>
> CONFIG_OF_EMBED was primarily enabled to support the stratix10
> spl hex file requirements.  Since this option now produces a
> warning during build, and the spl hex can be created using
> alternate methods, CONFIG_OF_EMBED is no longer needed.
>
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
>
> ---
> Changes in v2:
>  -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex
> ---
>  configs/socfpga_stratix10_defconfig       | 1 -
>  include/configs/socfpga_stratix10_socdk.h | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig
> index fbab388b43..f27180385d 100644
> --- a/configs/socfpga_stratix10_defconfig
> +++ b/configs/socfpga_stratix10_defconfig
> @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y
>  CONFIG_CMD_EXT4=y
>  CONFIG_CMD_FAT=y
>  CONFIG_CMD_FS_GENERIC=y
> -CONFIG_OF_EMBED=y
>  CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
>  CONFIG_ENV_IS_IN_MMC=y
>  CONFIG_NET_RANDOM_ETHADDR=y
> diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h
> index 39d757d737..66855ff0d8 100644
> --- a/include/configs/socfpga_stratix10_socdk.h
> +++ b/include/configs/socfpga_stratix10_socdk.h
> @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void);
>
>  /* SPL SDMMC boot support */
>  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
> -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot.img"
> +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-dtb.img"

Is that really necessary? I don't have the aarch64 compiler at hand,
but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"
are the same. Changing to "u-boot-dtb.img" here only complicates
things for the user, I think.

Regards,
Simon

>
>  #endif /* __CONFIG_H */
> --
> 2.21.0
>

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[See, Chin Liang]

On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote:
> On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen
> <dalon.westergreen@linux.intel.com> wrote:
> > 
> > 
> > From: Dalon Westergreen <dalon.westergreen@intel.com>
> > 
> > CONFIG_OF_EMBED was primarily enabled to support the stratix10
> > spl hex file requirements.  Since this option now produces a
> > warning during build, and the spl hex can be created using
> > alternate methods, CONFIG_OF_EMBED is no longer needed.
> > 
> > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > 
> > ---
> > Changes in v2:
> >  -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex
> > ---
> >  configs/socfpga_stratix10_defconfig       | 1 -
> >  include/configs/socfpga_stratix10_socdk.h | 2 +-
> >  2 files changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/configs/socfpga_stratix10_defconfig
> > b/configs/socfpga_stratix10_defconfig
> > index fbab388b43..f27180385d 100644
> > --- a/configs/socfpga_stratix10_defconfig
> > +++ b/configs/socfpga_stratix10_defconfig
> > @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y
> >  CONFIG_CMD_EXT4=y
> >  CONFIG_CMD_FAT=y
> >  CONFIG_CMD_FS_GENERIC=y
> > -CONFIG_OF_EMBED=y
> >  CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
> >  CONFIG_ENV_IS_IN_MMC=y
> >  CONFIG_NET_RANDOM_ETHADDR=y
> > diff --git a/include/configs/socfpga_stratix10_socdk.h
> > b/include/configs/socfpga_stratix10_socdk.h
> > index 39d757d737..66855ff0d8 100644
> > --- a/include/configs/socfpga_stratix10_socdk.h
> > +++ b/include/configs/socfpga_stratix10_socdk.h
> > @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void);
> > 
> >  /* SPL SDMMC boot support */
> >  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
> > -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-
> > boot.img"
> > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-
> > dtb.img"
> Is that really necessary? I don't have the aarch64 compiler at hand,
> but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"
> are the same. Changing to "u-boot-dtb.img" here only complicates
> things for the user, I think.

I would agree with Dalon since we want to make sure we use same name as
socfpga_common.h, which is for CV, A10 SoCs. This would help to
standardize our internal test infra.

Thanks
Chin Liang

> 
> Regards,
> Simon
> 
> > 
> > 
> >  #endif /* __CONFIG_H */
> > --
> > 2.21.0
> > 

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Simon Goldschmidt]

On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com> wrote:
>
> On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote:
> > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen
> > <dalon.westergreen@linux.intel.com> wrote:
> > >
> > >
> > > From: Dalon Westergreen <dalon.westergreen@intel.com>
> > >
> > > CONFIG_OF_EMBED was primarily enabled to support the stratix10
> > > spl hex file requirements.  Since this option now produces a
> > > warning during build, and the spl hex can be created using
> > > alternate methods, CONFIG_OF_EMBED is no longer needed.
> > >
> > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > >
> > > ---
> > > Changes in v2:
> > >  -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex
> > > ---
> > >  configs/socfpga_stratix10_defconfig       | 1 -
> > >  include/configs/socfpga_stratix10_socdk.h | 2 +-
> > >  2 files changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/configs/socfpga_stratix10_defconfig
> > > b/configs/socfpga_stratix10_defconfig
> > > index fbab388b43..f27180385d 100644
> > > --- a/configs/socfpga_stratix10_defconfig
> > > +++ b/configs/socfpga_stratix10_defconfig
> > > @@ -26,7 +26,6 @@ CONFIG_CMD_CACHE=y
> > >  CONFIG_CMD_EXT4=y
> > >  CONFIG_CMD_FAT=y
> > >  CONFIG_CMD_FS_GENERIC=y
> > > -CONFIG_OF_EMBED=y
> > >  CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
> > >  CONFIG_ENV_IS_IN_MMC=y
> > >  CONFIG_NET_RANDOM_ETHADDR=y
> > > diff --git a/include/configs/socfpga_stratix10_socdk.h
> > > b/include/configs/socfpga_stratix10_socdk.h
> > > index 39d757d737..66855ff0d8 100644
> > > --- a/include/configs/socfpga_stratix10_socdk.h
> > > +++ b/include/configs/socfpga_stratix10_socdk.h
> > > @@ -210,6 +210,6 @@ unsigned int cm_get_l4_sys_free_clk_hz(void);
> > >
> > >  /* SPL SDMMC boot support */
> > >  #define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1
> > > -#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-
> > > boot.img"
> > > +#define CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-
> > > dtb.img"
> > Is that really necessary? I don't have the aarch64 compiler at hand,
> > but when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"
> > are the same. Changing to "u-boot-dtb.img" here only complicates
> > things for the user, I think.
>
> I would agree with Dalon since we want to make sure we use same name as
> socfpga_common.h, which is for CV, A10 SoCs. This would help to
> standardize our internal test infra.

But that 'dtb' thing is an implementation detail. Who of the testers cares
whether the devicetree is embedded or not? "u-boot.img" exists with
OF_EMBED and without it, or doesn't it?

Regards,
Simon

>
> Thanks
> Chin Liang
>
> >
> > Regards,
> > Simon
> >
> > >
> > >
> > >  #endif /* __CONFIG_H */
> > > --
> > > 2.21.0
> > >

[U-Boot] [PATCH v2 1/2] Makefile: Add target to generate hex output for combined spl and dtb

[Marek Vasut]

On 6/4/19 5:12 AM, Dalon L Westergreen wrote:
> On Tue, 2019-06-04 at 02:00 +0200, Marek Vasut wrote:
>> On 6/4/19 1:57 AM, Dalon Westergreen wrote:
>>> From: Dalon Westergreen <
>>> dalon.westergreen at intel.com
>>>>
>>>
>>> Some architectures, Stratix10, require a hex formatted spl that combines
>>> the spl image and dtb.  This adds a target to create said hex file with
>>> and offset of SPL_TEXT_BASE.
>>>
>>> Signed-off-by: Dalon Westergreen <
>>> dalon.westergreen at intel.com
>>>>
>>>
>>
>> [...]
>>
>>> @@ -363,6 +365,11 @@ endif
>>>  $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
>>>  	$(call if_changed,mkimage)
>>>  
>>> +OBJCOPYFLAGS_$(SPL_BIN).hex := -I binary -O ihex --change-
>>> address=$(CONFIG_SPL_TEXT_BASE)
>>
>> Do we really need to do it here ? The commit message is not clear why
>> this is needed ; I think if you link the SPl against the correct
>> address, this should not be needed.
>>
> 
> This objcopy is from the binary including the dtb and not the elf.  If you
> objcopy using the elf, and link to the correct address, you are correct.  It
> is not true when just taking a binary and converting to a hex file.  The
> binary combined with the dtb is what is needed.
> 
> I can try be more descriptive in the commit message.
> 
> perhaps..
> 
> ---
> Stratix10 requires a hex image of the spl plus spl devicetree offset to 
> the Stratix10 onchip memory located at SPL_TEXT_BASE.  This patch adds
> a target to generate a hex file from the u-boot-spl binary including the
> dtb offset at SPL_TEST_BASE.

I think that's better, thanks. You could even include your explanation
above.

> ---
> 
>>> +$(obj)/$(SPL_BIN).hex: $(obj)/u-boot-spl.bin FORCE
>>> +	$(call if_changed,objcopy)
>>> +
>>>  quiet_cmd_mksunxiboot = MKSUNXI $@
>>>  cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \
>>>  			--default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@
>>> @@ -463,3 +470,4 @@ ifdef CONFIG_ARCH_K3
>>>  tispl.bin: $(obj)/u-boot-spl-nodtb.bin $(SHRUNK_ARCH_DTB) $(SPL_ITS) FORCE
>>>  	$(call if_changed,mkfitimage)
>>>  endif
>>> +
>>
>> Drop this hunk
> 
> Will do, and i likely should have used (SPL_BIN).bin rather than
> u-boot-spl.bin.
> 
> --dalon
>>
>>
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Dalon L Westergreen]

On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote:
> On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com>
> wrote:
> > On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote:
> > > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen<
> > > dalon.westergreen at linux.intel.com> wrote:
> > > > From: Dalon Westergreen <dalon.westergreen@intel.com>
> > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex
> > > > file requirements.  Since this option now produces awarning during
> > > > build, and the spl hex can be created usingalternate methods,
> > > > CONFIG_OF_EMBED is no longer needed.
> > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > > > ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex---
> > > > configs/socfpga_stratix10_defconfig       | 1 -
> > > > include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1
> > > > insertion(+), 2 deletions(-)
> > > > diff --git
> > > > a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf
> > > > igindex fbab388b43..f27180385d 100644---
> > > > a/configs/socfpga_stratix10_defconfig+++
> > > > b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@
> > > > CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y
> > > > CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y
> > > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
> > > > CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git
> > > > a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str
> > > > atix10_socdk.hindex 39d757d737..66855ff0d8 100644---
> > > > a/include/configs/socfpga_stratix10_socdk.h+++
> > > > b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned
> > > > int cm_get_l4_sys_free_clk_hz(void);
> > > >  /* SPL SDMMC boot support */ #define
> > > > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1-#define
> > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot.img"+#define
> > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-dtb.img"
> > > Is that really necessary? I don't have the aarch64 compiler at hand,but
> > > when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the
> > > same. Changing to "u-boot-dtb.img" here only complicatesthings for the
> > > user, I think.
> > 
> > I would agree with Dalon since we want to make sure we use same name
> > assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize
> > our internal test infra.
> 
> But that 'dtb' thing is an implementation detail. Who of the testers
> careswhether the devicetree is embedded or not? "u-boot.img" exists
> withOF_EMBED and without it, or doesn't it?
> Regards,Simon

Yes, it exists either way.  I have no issue leaving it as u-boot.img but i do
agree that it should bethe same across the socfpga family.  As u-boot.img exists
regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed?  I
will submit another patch to changesocfpga_common.h?
--dalon
> > ThanksChin Liang
> > > Regards,Simon
> > > >  #endif /* __CONFIG_H */--2.21.0

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Tom Rini]

On Tue, Jun 04, 2019 at 09:10:27AM -0700, Dalon L Westergreen wrote:
> On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote:
> > On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com>
> > wrote:
> > > On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote:
> > > > On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen<
> > > > dalon.westergreen at linux.intel.com> wrote:
> > > > > From: Dalon Westergreen <dalon.westergreen@intel.com>
> > > > > CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex
> > > > > file requirements.  Since this option now produces awarning during
> > > > > build, and the spl hex can be created usingalternate methods,
> > > > > CONFIG_OF_EMBED is no longer needed.
> > > > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > > > > ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex---
> > > > > configs/socfpga_stratix10_defconfig       | 1 -
> > > > > include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1
> > > > > insertion(+), 2 deletions(-)
> > > > > diff --git
> > > > > a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf
> > > > > igindex fbab388b43..f27180385d 100644---
> > > > > a/configs/socfpga_stratix10_defconfig+++
> > > > > b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@
> > > > > CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y
> > > > > CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y
> > > > > CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
> > > > > CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git
> > > > > a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str
> > > > > atix10_socdk.hindex 39d757d737..66855ff0d8 100644---
> > > > > a/include/configs/socfpga_stratix10_socdk.h+++
> > > > > b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned
> > > > > int cm_get_l4_sys_free_clk_hz(void);
> > > > >  /* SPL SDMMC boot support */ #define
> > > > > CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1-#define
> > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot.img"+#define
> > > > > CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-dtb.img"
> > > > Is that really necessary? I don't have the aarch64 compiler at hand,but
> > > > when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the
> > > > same. Changing to "u-boot-dtb.img" here only complicatesthings for the
> > > > user, I think.
> > > 
> > > I would agree with Dalon since we want to make sure we use same name
> > > assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize
> > > our internal test infra.
> > 
> > But that 'dtb' thing is an implementation detail. Who of the testers
> > careswhether the devicetree is embedded or not? "u-boot.img" exists
> > withOF_EMBED and without it, or doesn't it?
> > Regards,Simon
> 
> Yes, it exists either way.  I have no issue leaving it as u-boot.img but i do
> agree that it should bethe same across the socfpga family.  As u-boot.img exists
> regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed?  I
> will submit another patch to changesocfpga_common.h?

Using "u-boot.img" is the right choice, it's what's used on all other
platforms for "u-boot with dtb" and we have logic in the Makefile to do
the right thing so that users can upgrade from a version of U-Boot where
the dtb wasn't in use to one where it is without their scripts breaking.

-- 
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/20190604/fa547c19/attachment.sig>

[U-Boot] [PATCH v2 2/2] ARM: socfpga: stratix10: Remove CONFIG_OF_EMBED

[Simon Goldschmidt]

Am 04.06.2019 um 20:15 schrieb Tom Rini:
> On Tue, Jun 04, 2019 at 09:10:27AM -0700, Dalon L Westergreen wrote:
>> On Tue, 2019-06-04 at 08:12 +0200, Simon Goldschmidt wrote:
>>> On Tue, Jun 4, 2019 at 7:58 AM See, Chin Liang <chin.liang.see@intel.com>
>>> wrote:
>>>> On Tue, 2019-06-04 at 07:13 +0200, Simon Goldschmidt wrote:
>>>>> On Tue, Jun 4, 2019 at 1:57 AM Dalon Westergreen<
>>>>> dalon.westergreen at linux.intel.com> wrote:
>>>>>> From: Dalon Westergreen <dalon.westergreen@intel.com>
>>>>>> CONFIG_OF_EMBED was primarily enabled to support the stratix10spl hex
>>>>>> file requirements.  Since this option now produces awarning during
>>>>>> build, and the spl hex can be created usingalternate methods,
>>>>>> CONFIG_OF_EMBED is no longer needed.
>>>>>> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
>>>>>> ---Changes in v2: -> Change CONFIG_SPL_TARGET back to u-boot-spl.hex---
>>>>>> configs/socfpga_stratix10_defconfig       | 1 -
>>>>>> include/configs/socfpga_stratix10_socdk.h | 2 +- 2 files changed, 1
>>>>>> insertion(+), 2 deletions(-)
>>>>>> diff --git
>>>>>> a/configs/socfpga_stratix10_defconfigb/configs/socfpga_stratix10_defconf
>>>>>> igindex fbab388b43..f27180385d 100644---
>>>>>> a/configs/socfpga_stratix10_defconfig+++
>>>>>> b/configs/socfpga_stratix10_defconfig@@ -26,7 +26,6 @@
>>>>>> CONFIG_CMD_CACHE=y CONFIG_CMD_EXT4=y CONFIG_CMD_FAT=y
>>>>>> CONFIG_CMD_FS_GENERIC=y-CONFIG_OF_EMBED=y
>>>>>> CONFIG_DEFAULT_DEVICE_TREE="socfpga_stratix10_socdk"
>>>>>> CONFIG_ENV_IS_IN_MMC=y CONFIG_NET_RANDOM_ETHADDR=ydiff --git
>>>>>> a/include/configs/socfpga_stratix10_socdk.hb/include/configs/socfpga_str
>>>>>> atix10_socdk.hindex 39d757d737..66855ff0d8 100644---
>>>>>> a/include/configs/socfpga_stratix10_socdk.h+++
>>>>>> b/include/configs/socfpga_stratix10_socdk.h@@ -210,6 +210,6 @@ unsigned
>>>>>> int cm_get_l4_sys_free_clk_hz(void);
>>>>>>   /* SPL SDMMC boot support */ #define
>>>>>> CONFIG_SYS_MMCSD_FS_BOOT_PARTITION     1-#define
>>>>>> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot.img"+#define
>>>>>> CONFIG_SPL_FS_LOAD_PAYLOAD_NAME                "u-boot-dtb.img"
>>>>> Is that really necessary? I don't have the aarch64 compiler at hand,but
>>>>> when compiling a gen5 board, "u-boot.img" and "u-boot-dtb.img"are the
>>>>> same. Changing to "u-boot-dtb.img" here only complicatesthings for the
>>>>> user, I think.
>>>>
>>>> I would agree with Dalon since we want to make sure we use same name
>>>> assocfpga_common.h, which is for CV, A10 SoCs. This would help tostandardize
>>>> our internal test infra.
>>>
>>> But that 'dtb' thing is an implementation detail. Who of the testers
>>> careswhether the devicetree is embedded or not? "u-boot.img" exists
>>> withOF_EMBED and without it, or doesn't it?
>>> Regards,Simon
>>
>> Yes, it exists either way.  I have no issue leaving it as u-boot.img but i do
>> agree that it should bethe same across the socfpga family.  As u-boot.img exists
>> regardless of CONFIG_OF_EMBEDi would suggest just using u-boot.img, agreed?  I
>> will submit another patch to changesocfpga_common.h?
> 
> Using "u-boot.img" is the right choice, it's what's used on all other
> platforms for "u-boot with dtb" and we have logic in the Makefile to do
> the right thing so that users can upgrade from a version of U-Boot where
> the dtb wasn't in use to one where it is without their scripts breaking.

That's what I thought. Thanks for clarifying this, Tom.

Regards,
Simon