On Tue, Feb 23, 2021 at 07:00:47AM +0000, Lim, Elly Siew Chin wrote:
> Hi Tom,
> 
> > -----Original Message-----
> > From: Tom Rini <tr...@konsulko.com>
> > Sent: Saturday, February 20, 2021 5:02 AM
> > To: Lim, Elly Siew Chin <elly.siew.chin....@intel.com>
> > Cc: u-boot@lists.denx.de; Marek Vasut <ma...@denx.de>; Tan, Ley Foon
> > <ley.foon....@intel.com>; See, Chin Liang <chin.liang....@intel.com>; Simon
> > Goldschmidt <simon.k.r.goldschm...@gmail.com>; Chee, Tien Fong
> > <tien.fong.c...@intel.com>; Westergreen, Dalon
> > <dalon.westergr...@intel.com>; Simon Glass <s...@chromium.org>; Gan, Yau
> > Wai <yau.wai....@intel.com>
> > Subject: Re: [PATCH] Makefile: Add target to generate hex output for 
> > combined
> > spl and dtb
> > 
> > On Fri, Feb 19, 2021 at 01:55:44PM +0800, Siew Chin Lim wrote:
> > 
> > > From: Dalon Westergreen <dalon.westergr...@intel.com>
> > >
> > > Some architectures, Stratix10/Agilex, 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.westergr...@intel.com>
> > > Signed-off-by: Siew Chin Lim <elly.siew.chin....@intel.com>
> > > ---
> > >  Makefile                               | 11 ++++++-----
> > >  include/configs/socfpga_soc64_common.h |  2 +-
> > >  scripts/Makefile.spl                   |  8 ++++++++
> > >  3 files changed, 15 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 4da46dea39..f1adc9aa23 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1263,11 +1263,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 | \
> > > @@ -1935,6 +1930,12 @@ spl/u-boot-spl.bin: spl/u-boot-spl
> > >   @:
> > >   $(SPL_SIZE_CHECK)
> > >
> > > +spl/u-boot-spl-dtb.bin: spl/u-boot-spl
> > > + @:
> > > +
> > > +spl/u-boot-spl-dtb.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/d
> > > t.dtb) diff --git a/include/configs/socfpga_soc64_common.h
> > > b/include/configs/socfpga_soc64_common.h
> > > index fdcd7d3e9a..1af359466c 100644
> > > --- a/include/configs/socfpga_soc64_common.h
> > > +++ b/include/configs/socfpga_soc64_common.h
> > > @@ -200,7 +200,7 @@ unsigned int cm_get_l4_sys_free_clk_hz(void);
> > >   * 0x8000_0000 ...... End of SDRAM_1 (assume 2GB)
> > >   *
> > >   */
> > > -#define CONFIG_SPL_TARGET                "spl/u-boot-spl.hex"
> > > +#define CONFIG_SPL_TARGET                "spl/u-boot-spl-dtb.hex"
> > >  #define CONFIG_SPL_MAX_SIZE              CONFIG_SYS_INIT_RAM_SIZE
> > >  #define CONFIG_SPL_STACK         CONFIG_SYS_INIT_SP_ADDR
> > >  #define CONFIG_SPL_BSS_MAX_SIZE          0x100000        /* 1 MB */
> > > diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index
> > > ea4e045769..625e06d0d9 100644
> > > --- a/scripts/Makefile.spl
> > > +++ b/scripts/Makefile.spl
> > > @@ -229,6 +229,9 @@ ifneq
> > ($(CONFIG_TARGET_SOCFPGA_GEN5)$(CONFIG_TARGET_SOCFPGA_ARRIA10),)
> > >  INPUTS-y += $(obj)/$(SPL_BIN).sfp
> > >  endif
> > >
> > > +INPUTS-$(CONFIG_TARGET_SOCFPGA_STRATIX10)        += $(obj)/u-boot-spl-
> > dtb.hex
> > > +INPUTS-$(CONFIG_TARGET_SOCFPGA_AGILEX)           += $(obj)/u-
> > boot-spl-dtb.hex
> > > +
> > >  ifdef CONFIG_ARCH_SUNXI
> > >  INPUTS-y += $(obj)/sunxi-spl.bin
> > >
> > > @@ -389,6 +392,11 @@ $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin
> > > FORCE  MKIMAGEFLAGS_sunxi-spl.bin = -T sunxi_egon \
> > >   -n $(CONFIG_DEFAULT_DEVICE_TREE)
> > >
> > > +OBJCOPYFLAGS_u-boot-spl-dtb.hex := -I binary -O ihex
> > > +--change-address=$(CONFIG_SPL_TEXT_BASE)
> > > +
> > > +$(obj)/u-boot-spl-dtb.hex: $(obj)/u-boot-spl-dtb.bin FORCE
> > > + $(call if_changed,objcopy)
> > > +
> > >  $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin FORCE
> > >   $(call if_changed,mkimage)
> > 
> > The commit message / subject doesn't make it clear that you're doing
> > something for SoCFPGA here, and I seem to recall this change being proposed 
> > a
> > while ago and it wasn't clear that this is really the best default path 
> > forward I
> > thought?
> > 
> 
> Noted. I will resend the patch and enhance the message / subject . 
> 
> I have checked again with the team about this, we still would like to 
> generate the 'u-boot-spl-dtb.hex' for socfpga soc64 devices by default during 
> u-boot compilation which is always needed by customer to generate the final 
> configuration bitstream. 
> Can you please advise if this is not the right way for the implementation? 

I think the changes are fine then, please just update the commit
message, thanks!

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to