| Message ID | 1535990712-8359-7-git-send-email-jagdish.gediya@nxp.com |
|---|---|
| State | Accepted |
| Delegated to: | York Sun |
| Headers | show |
| Series | Device tree support for PowerPC in U-Boot | expand |
On 3 September 2018 at 18:05, Jagdish Gediya <jagdish.gediya@nxp.com> wrote: > Below is the sequence to embed dtb inside 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 has resetvec section. Step 1 and step 3 described above are > required only if this config is y. > > Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com> > --- > Changes for v2: > - Don't change the generic target > - Add new config option to use binman Where is that config option? I don't see it in this patch. > > Changes for v3: > - Commit message change('u-boot' -> 'U-Boot') > > Makefile | 23 ++++++++++++++++++++++- > arch/powerpc/cpu/mpc85xx/Kconfig | 4 ++++ > 2 files changed, 26 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On 09/02/2018 10:08 PM, Jagdish Gediya wrote: > Below is the sequence to embed dtb inside 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 has resetvec section. Step 1 and step 3 described above are > required only if this config is y. > > Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com> > --- > Changes for v2: > - Don't change the generic target > - Add new config option to use binman > > Changes for v3: > - Commit message change('u-boot' -> 'U-Boot') > > 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 > + One thing confuses me is the output file. My understand is we will use u-boot-with-dtb.bin file. Other files are intermediate files. It would be helpful to explain each output file, especially for debugging, and let other know we switch from one file to another after this patch set. CONFIG_OF_SEPARATE seems not be selected explicitly anywhere. Are we relying on the first choice to be selected automatically? Is it guaranteed? York
Hi York, > -----Original Message----- > From: York Sun > Sent: Wednesday, September 26, 2018 12:19 AM > To: Jagdish Gediya <jagdish.gediya@nxp.com>; u-boot@lists.denx.de; > Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Poonam Aggrwal > <poonam.aggrwal@nxp.com>; sjg@chromium.org; bmeng.cn@gmail.com > Cc: trini@konsulko.com > Subject: Re: [PATCH v3 6/8] powerpc: mpc85xx: Use binman to embed dtb > inside U-Boot > > On 09/02/2018 10:08 PM, Jagdish Gediya wrote: > > Below is the sequence to embed dtb inside 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 > > has resetvec section. Step 1 and step 3 described above are required > > only if this config is y. > > > > Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com> > > --- > > Changes for v2: > > - Don't change the generic target > > - Add new config option to use binman > > > > Changes for v3: > > - Commit message change('u-boot' -> 'U-Boot') > > > > 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 > > + > > One thing confuses me is the output file. My understand is we will use u- > boot-with-dtb.bin file. Other files are intermediate files. It would be helpful > to explain each output file, especially for debugging, and let other know we > switch from one file to another after this patch set. I have already updated the board README for u-boot-with-dtb.bin. Apart from it, Only one new (u-boot-br.bin) intermediate file is generated with respect to the existing U-Boot compilations flow. Should I update board README or commit message for u-boot-br.bin? > > CONFIG_OF_SEPARATE seems not be selected explicitly anywhere. Are we > relying on the first choice to be selected automatically? Is it guaranteed? yes, We are relying on the first choice to be selected automatically. It should be guaranteed, I am seeing that no configs in U-Boot are enabling it explicitly but many board uses u-boot-dtb.bin or files based on u-boot-dtb.bin for booting which are only generated if CONFIG_OF_SEPARATE is enabled. Regards, Jagdish
On 09/26/2018 03:17 AM, Jagdish Gediya wrote: > Hi York, > >> -----Original Message----- >> From: York Sun >> Sent: Wednesday, September 26, 2018 12:19 AM >> To: Jagdish Gediya <jagdish.gediya@nxp.com>; u-boot@lists.denx.de; >> Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>; Poonam Aggrwal >> <poonam.aggrwal@nxp.com>; sjg@chromium.org; bmeng.cn@gmail.com >> Cc: trini@konsulko.com >> Subject: Re: [PATCH v3 6/8] powerpc: mpc85xx: Use binman to embed dtb >> inside U-Boot >> >> On 09/02/2018 10:08 PM, Jagdish Gediya wrote: >>> Below is the sequence to embed dtb inside 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 >>> has resetvec section. Step 1 and step 3 described above are required >>> only if this config is y. >>> >>> Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com> >>> --- >>> Changes for v2: >>> - Don't change the generic target >>> - Add new config option to use binman >>> >>> Changes for v3: >>> - Commit message change('u-boot' -> 'U-Boot') >>> >>> 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 >>> + >> >> One thing confuses me is the output file. My understand is we will use u- >> boot-with-dtb.bin file. Other files are intermediate files. It would be helpful >> to explain each output file, especially for debugging, and let other know we >> switch from one file to another after this patch set. > I have already updated the board README for u-boot-with-dtb.bin. Apart from it, Only one new (u-boot-br.bin) intermediate file is generated with respect to the existing U-Boot compilations flow. Should I update board README or commit message for u-boot-br.bin? I see the updated README for T2080QDS. I can see u-boot-br.bin from the change history so it is clear to me. Maybe you can suggest additional language to the commit message to make it more clear. >> >> CONFIG_OF_SEPARATE seems not be selected explicitly anywhere. Are we >> relying on the first choice to be selected automatically? Is it guaranteed? > yes, We are relying on the first choice to be selected automatically. It should be guaranteed, I am seeing that no configs in U-Boot are enabling it explicitly but many board uses u-boot-dtb.bin or files based on u-boot-dtb.bin for booting which are only generated if CONFIG_OF_SEPARATE is enabled. > I didn't find anywhere saying the first choice is guaranteed if nothing is explicitly selected. York
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) +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" + depends on MPC85xx + config BOOKE bool default y
Below is the sequence to embed dtb inside 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 has resetvec section. Step 1 and step 3 described above are required only if this config is y. Signed-off-by: Jagdish Gediya <jagdish.gediya@nxp.com> --- Changes for v2: - Don't change the generic target - Add new config option to use binman Changes for v3: - Commit message change('u-boot' -> 'U-Boot') Makefile | 23 ++++++++++++++++++++++- arch/powerpc/cpu/mpc85xx/Kconfig | 4 ++++ 2 files changed, 26 insertions(+), 1 deletion(-)