Hi,
> -----Original Message-----
> From: Bin Meng <bmeng...@gmail.com>
> Sent: Tuesday, August 28, 2018 2:47 PM
> To: Jagdish Gediya <jagdish.ged...@nxp.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Prabhakar Kushwaha
> <prabhakar.kushw...@nxp.com>; York Sun <york....@nxp.com>; Poonam
> Aggrwal <poonam.aggr...@nxp.com>; Simon Glass <s...@chromium.org>;
> Tom Rini <tr...@konsulko.com>
> Subject: Re: [PATCH v2 6/8] powerpc: mpc85xx: Use binman to embed dtb
> inside u-boot
>
> On Tue, Aug 28, 2018 at 11:53 AM Jagdish Gediya <jagdish.ged...@nxp.com>
> wrote:
> >
> > Below is the sequence to embed dtb inside u-boot,
>
> nits: U-Boot
>
> > 1. Remove bootpg and resetvec section if needed 2. Append dtb 3.
> > Append bootpg and resetvec section back if removed in step 1
> >
> > Above procedure is required only when CONFIG_MPC85xx and
> > CONFIG_OF_SEPARATE are defined.
> >
> > Add new config CONFIG_MPC85XX_HAVE_RESET_VECTOR to indicate that
> image
> > have resetvec section. step 1 and step 3 described above are
>
> have -> has. step 1 -> Step 1
>
> > required only if this config is y.
> >
> > Signed-off-by: Jagdish Gediya <jagdish.ged...@nxp.com>
> > ---
> > Changes for v2:
> > - Don't change the generic target
> > - Add new config option to use binman
> >
> > Makefile | 23 ++++++++++++++++++++++-
> > arch/powerpc/cpu/mpc85xx/Kconfig | 4 ++++
> > 2 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index b5bf8ab..03baa74 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -861,6 +861,10 @@ ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> > ALL-y += init_sp_bss_offset_check
> > endif
> >
> > +ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
> > +ALL-y += u-boot-with-dtb.bin
> > +endif
> > +
> > LDFLAGS_u-boot += $(LDFLAGS_FINAL)
> >
> > # Avoid 'Not enough room for program headers' error on binutils 2.28
> onwards.
> > @@ -983,7 +987,8 @@ spl/u-boot-spl.srec: spl/u-boot-spl FORCE
> > $(call if_changed,objcopy)
> >
> > OBJCOPYFLAGS_u-boot-nodtb.bin := -O binary \
> > - $(if $(CONFIG_X86_16BIT_INIT),-R .start16 -R .resetvec)
> > + $(if $(CONFIG_X86_16BIT_INIT),-R .start16 -R .resetvec) \
> > + $(if $(CONFIG_MPC85XX_HAVE_RESET_VECTOR),-R .bootpg -R
> > + .resetvec)
> >
> > binary_size_check: u-boot-nodtb.bin FORCE
> > @file_size=$(shell wc -c u-boot-nodtb.bin | awk '{print $$1}')
> > ; \ @@ -1202,6 +1207,18 @@ u-boot-with-spl.sfp: spl/u-boot-spl.sfp u-
> boot.img FORCE
> > $(call if_changed,socboot)
> > endif
> >
> > +ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
>
> This looks odd. Both CONFIG_OF_SEPARATE and CONFIG_OF_EMBED should
> be supported by binman.
Entry type 'u-boot-dtb-with-ucode' requires file 'u-boot.dtb' which is built
only for CONFIG_OF_SEPARATE.
> > +u-boot-with-dtb.bin: u-boot.bin u-boot.dtb \
> > + $(if $(CONFIG_MPC85XX_HAVE_RESET_VECTOR), u-boot-br.bin)
> FORCE
> > + $(call if_changed,binman)
> > +
> > +ifeq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR),y)
> > +OBJCOPYFLAGS_u-boot-br.bin := -O binary -j .bootpg -j .resetvec
> > +u-boot-br.bin: u-boot FORCE
> > + $(call if_changed,objcopy)
> > +endif
> > +endif
> > +
> > # x86 uses a large ROM. We fill it with 0xff, put the 16-bit stuff
> > (including # reset vector) at the top, Intel ME descriptor at the
> > bottom, and U-Boot in # the middle. This is handled by binman based
> > on an image description in the @@ -1296,8 +1313,12 @@
> > spl/u-boot-spl.pbl: spl/u-boot-spl.bin FORCE ifeq ($(ARCH),arm)
> > UBOOT_BINLOAD := u-boot.img else
> > +ifeq ($(CONFIG_MPC85xx)$(CONFIG_OF_SEPARATE),yy)
> > +UBOOT_BINLOAD := u-boot-with-dtb.bin
> > +else
> > UBOOT_BINLOAD := u-boot.bin
> > endif
> > +endif
> >
> > OBJCOPYFLAGS_u-boot-with-spl-pbl.bin = -I binary -O binary --pad-
> to=$(CONFIG_SPL_PAD_TO) \
> > --gap-fill=0xff diff --git
> > a/arch/powerpc/cpu/mpc85xx/Kconfig
> b/arch/powerpc/cpu/mpc85xx/Kconfig
> > index 19e8d02..7d139ff 100644
> > --- a/arch/powerpc/cpu/mpc85xx/Kconfig
> > +++ b/arch/powerpc/cpu/mpc85xx/Kconfig
> > @@ -1143,6 +1143,10 @@ config ARCH_T4240
> > imply CMD_REGINFO
> > imply FSL_SATA
> >
> > +config MPC85XX_HAVE_RESET_VECTOR
> > + bool "Indicate reset vector at CONFIG_RESET_VECTOR_ADDRESS -
> 0xffc"
>
> I don't think you want people to turn this option on and off, no? You
> probably need move the "string" to the help paragraph.
This option need to be turned on/off. Only NOR boot have the reset vector and
bootpg section at CONFIG_RESET_VECTOR_ADDRESS - 0xffc. Other boot do not have
the reset vector but they have the bootpg section at some other address where
it don't need to be removed and joined.
This option indicates that reset vector and bootpg sections are placed at
CONFIG_RESET_VECTOR_ADDRESS - 0xffc and it need to be turned on/off according
to boot device.
> > + depends on MPC85xx
> > +
> > config BOOKE
> > bool
> > default y
> > --
>
> Regards,
> Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
