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? 

Thanks,
Siew Chin

> >
> > --
> > 2.13.0
> >
> 
> --
> Tom

Reply via email to