[U-Boot] [PATCH v2 6/8] powerpc: mpc85xx: Use binman to embed dtb inside u-boot

[Bin Meng]

On Tue, Aug 28, 2018 at 11:53 AM Jagdish Gediya <jagdish.gediya at 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.gediya at 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.

> +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.

> +       depends on MPC85xx
> +
>  config BOOKE
>         bool
>         default y
> --

Regards,
Bin