diff mbox series

[3/3] kbuild: SPL/TPL: generate separate asm-offsets.h for SPL and TPL

Message ID 20200416043826.490120-4-masahiroy@kernel.org
State Superseded
Headers show
Series support separate asm-offsets.h for SPL and TPL | expand

Commit Message

Masahiro Yamada April 16, 2020, 4:38 a.m. UTC
Currently generic-asm-offsets.h and asm-offsets.h are generated based
on U-Boot proper config options. The same asm-offsets headers are used
for building U-Boot SPL/TPL, which causes potential offset mismatch if
U-Boot proper has different config options from U-Boot SPL/TPL.

This commit adds:
  spl/include/generated/(generic-)asm-offsets.h
  tpl/include/generated/(generic-)asm-offsets.h

spl/include/generated/(generic-)asm-offsets.h is generated if
CONFIG_SPL=y, and included when building SPL.

tpl/include/generated/(generic-)asm-offsets.h is generated if
CONFIG_TPL=y, and included when building TPL.

They are created before Kbuild descends into SPL/TPL object directories
and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c
includes a bunch of headers.

Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h
is searched in {spl,tpl}/include/generated/.

Requested-by: Bin Meng <bmeng.cn at gmail.com>
Signed-off-by: Masahiro Yamada <masahiroy at kernel.org>
---

 Kbuild               |  4 ++--
 scripts/Makefile.spl | 10 ++++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

Comments

Bin Meng April 16, 2020, 2:14 p.m. UTC | #1
Hi Masahiro,

On Thu, Apr 16, 2020 at 12:39 PM Masahiro Yamada <masahiroy at kernel.org> wrote:
>
> Currently generic-asm-offsets.h and asm-offsets.h are generated based
> on U-Boot proper config options. The same asm-offsets headers are used
> for building U-Boot SPL/TPL, which causes potential offset mismatch if
> U-Boot proper has different config options from U-Boot SPL/TPL.
>

Thank you very much for doing this.

> This commit adds:
>   spl/include/generated/(generic-)asm-offsets.h
>   tpl/include/generated/(generic-)asm-offsets.h
>
> spl/include/generated/(generic-)asm-offsets.h is generated if
> CONFIG_SPL=y, and included when building SPL.
>
> tpl/include/generated/(generic-)asm-offsets.h is generated if
> CONFIG_TPL=y, and included when building TPL.
>
> They are created before Kbuild descends into SPL/TPL object directories
> and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c
> includes a bunch of headers.
>
> Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h
> is searched in {spl,tpl}/include/generated/.
>

This patch does not apply on top of u-boot/master, unfortunately.

> Requested-by: Bin Meng <bmeng.cn at gmail.com>
> Signed-off-by: Masahiro Yamada <masahiroy at kernel.org>
> ---
>
>  Kbuild               |  4 ++--
>  scripts/Makefile.spl | 10 ++++++++--
>  2 files changed, 10 insertions(+), 4 deletions(-)
>

I manually applied the changes and had a test, but it failed to build.
See below:

> diff --git a/Kbuild b/Kbuild
> index 6014f7c34a..1eac091594 100644
> --- a/Kbuild
> +++ b/Kbuild
> @@ -10,7 +10,7 @@ generic-offsets-file := include/generated/generic-asm-offsets.h
>  always  := $(generic-offsets-file)
>  targets := lib/asm-offsets.s
>
> -$(obj)/$(generic-offsets-file): lib/asm-offsets.s FORCE
> +$(obj)/$(generic-offsets-file): $(obj)/lib/asm-offsets.s FORCE

  CC      spl/./lib/asm-offsets.s
cc1: fatal error: can't open 'spl/./lib/asm-offsets.s' for writing: No
such file or directory
compilation terminated.
scripts/Makefile.build:166: recipe for target 'spl/./lib/asm-offsets.s' failed
make[2]: *** [spl/./lib/asm-offsets.s] Error 1
scripts/Makefile.spl:432: recipe for target 'prepare' failed
make[1]: *** [prepare] Error 2
Makefile:1917: recipe for target 'spl/u-boot-spl' failed
make: *** [spl/u-boot-spl] Error 2

When I removed the changes for this line, and the line below, the
build could pass.
The spl/include/generated/(generic-)asm-offsets.h were generated, and
I verified the macro values are correct and different with the U-Boot
proper one.

But since there is no $(obj) prepended to lib/asm-offsets.s, I believe
this is still not perfect.

>         $(call filechk,offsets,__GENERIC_ASM_OFFSETS_H__)
>
>  #####
> @@ -25,5 +25,5 @@ targets += arch/$(ARCH)/lib/asm-offsets.s
>
>  CFLAGS_asm-offsets.o := -DDO_DEPS_ONLY
>
> -$(obj)/$(offsets-file): arch/$(ARCH)/lib/asm-offsets.s FORCE
> +$(obj)/$(offsets-file): $(obj)/arch/$(ARCH)/lib/asm-offsets.s FORCE
>         $(call filechk,offsets,__ASM_OFFSETS_H__)
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 98c04bbbfc..c3fa45f520 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -22,6 +22,8 @@ include $(srctree)/scripts/Kbuild.include
>  -include include/config/auto.conf
>  -include $(obj)/include/autoconf.mk
>
> +UBOOTINCLUDE := -I$(obj)/include $(UBOOTINCLUDE)
> +
>  KBUILD_CPPFLAGS += -DCONFIG_SPL_BUILD
>  ifeq ($(CONFIG_TPL_BUILD),y)
>  KBUILD_CPPFLAGS += -DCONFIG_TPL_BUILD
> @@ -311,7 +313,7 @@ cmd_plat = $(CC) $(c_flags) -c $< -o $(filter-out $(PHONY),$@)
>
>  targets += $(obj)/dts/dt-platdata.o
>  $(obj)/dts/dt-platdata.o: $(obj)/dts/dt-platdata.c \
> -               include/generated/dt-structs-gen.h FORCE
> +               include/generated/dt-structs-gen.h prepare FORCE
>         $(call if_changed,plat)
>
>  PHONY += dts_dir
> @@ -422,9 +424,13 @@ $(obj)/$(SPL_BIN): $(u-boot-spl-platdata) $(u-boot-spl-init) \
>  $(sort $(u-boot-spl-init) $(u-boot-spl-main)): $(u-boot-spl-dirs) ;
>
>  PHONY += $(u-boot-spl-dirs)
> -$(u-boot-spl-dirs): $(u-boot-spl-platdata)
> +$(u-boot-spl-dirs): $(u-boot-spl-platdata) prepare
>         $(Q)$(MAKE) $(build)=$@
>
> +PHONY += prepare
> +prepare:
> +       $(Q)$(MAKE) $(build)=$(obj)/.
> +
>  quiet_cmd_cpp_lds = LDS     $@
>  cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
>                 -D__ASSEMBLY__ -x assembler-with-cpp -std=c99 -P -o $@ $<
> --

Regards,
Bin
Masahiro Yamada April 17, 2020, 7:01 a.m. UTC | #2
On Thu, Apr 16, 2020 at 11:15 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Masahiro,
>
> On Thu, Apr 16, 2020 at 12:39 PM Masahiro Yamada <masahiroy at kernel.org> wrote:
> >
> > Currently generic-asm-offsets.h and asm-offsets.h are generated based
> > on U-Boot proper config options. The same asm-offsets headers are used
> > for building U-Boot SPL/TPL, which causes potential offset mismatch if
> > U-Boot proper has different config options from U-Boot SPL/TPL.
> >
>
> Thank you very much for doing this.
>
> > This commit adds:
> >   spl/include/generated/(generic-)asm-offsets.h
> >   tpl/include/generated/(generic-)asm-offsets.h
> >
> > spl/include/generated/(generic-)asm-offsets.h is generated if
> > CONFIG_SPL=y, and included when building SPL.
> >
> > tpl/include/generated/(generic-)asm-offsets.h is generated if
> > CONFIG_TPL=y, and included when building TPL.
> >
> > They are created before Kbuild descends into SPL/TPL object directories
> > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c
> > includes a bunch of headers.
> >
> > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h
> > is searched in {spl,tpl}/include/generated/.
> >
>
> This patch does not apply on top of u-boot/master, unfortunately.
>
> > Requested-by: Bin Meng <bmeng.cn at gmail.com>
> > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org>
> > ---
> >
> >  Kbuild               |  4 ++--
> >  scripts/Makefile.spl | 10 ++++++++--
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> >
>
> I manually applied the changes and had a test, but it failed to build.
> See below:
>
> > diff --git a/Kbuild b/Kbuild
> > index 6014f7c34a..1eac091594 100644
> > --- a/Kbuild
> > +++ b/Kbuild
> > @@ -10,7 +10,7 @@ generic-offsets-file := include/generated/generic-asm-offsets.h
> >  always  := $(generic-offsets-file)
> >  targets := lib/asm-offsets.s
> >
> > -$(obj)/$(generic-offsets-file): lib/asm-offsets.s FORCE
> > +$(obj)/$(generic-offsets-file): $(obj)/lib/asm-offsets.s FORCE
>
>   CC      spl/./lib/asm-offsets.s
> cc1: fatal error: can't open 'spl/./lib/asm-offsets.s' for writing: No
> such file or directory
> compilation terminated.
> scripts/Makefile.build:166: recipe for target 'spl/./lib/asm-offsets.s' failed
> make[2]: *** [spl/./lib/asm-offsets.s] Error 1
> scripts/Makefile.spl:432: recipe for target 'prepare' failed
> make[1]: *** [prepare] Error 2
> Makefile:1917: recipe for target 'spl/u-boot-spl' failed
> make: *** [spl/u-boot-spl] Error 2
>
> When I removed the changes for this line, and the line below, the
> build could pass.
> The spl/include/generated/(generic-)asm-offsets.h were generated, and
> I verified the macro values are correct and different with the U-Boot
> proper one.
>
> But since there is no $(obj) prepended to lib/asm-offsets.s, I believe
> this is still not perfect.


This is because you remove the needed changes.
Bin Meng April 17, 2020, 7:10 a.m. UTC | #3
Hi Masahiro,

On Fri, Apr 17, 2020 at 3:02 PM Masahiro Yamada <masahiroy at kernel.org> wrote:
>
> On Thu, Apr 16, 2020 at 11:15 PM Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Masahiro,
> >
> > On Thu, Apr 16, 2020 at 12:39 PM Masahiro Yamada <masahiroy at kernel.org> wrote:
> > >
> > > Currently generic-asm-offsets.h and asm-offsets.h are generated based
> > > on U-Boot proper config options. The same asm-offsets headers are used
> > > for building U-Boot SPL/TPL, which causes potential offset mismatch if
> > > U-Boot proper has different config options from U-Boot SPL/TPL.
> > >
> >
> > Thank you very much for doing this.
> >
> > > This commit adds:
> > >   spl/include/generated/(generic-)asm-offsets.h
> > >   tpl/include/generated/(generic-)asm-offsets.h
> > >
> > > spl/include/generated/(generic-)asm-offsets.h is generated if
> > > CONFIG_SPL=y, and included when building SPL.
> > >
> > > tpl/include/generated/(generic-)asm-offsets.h is generated if
> > > CONFIG_TPL=y, and included when building TPL.
> > >
> > > They are created before Kbuild descends into SPL/TPL object directories
> > > and builds $(obj)/dts/dt-platdata.o because $(obj)/dts/dt-platdata.c
> > > includes a bunch of headers.
> > >
> > > Prepend -I$(obj)/include to $(UBOOTINCLUDE) so (generic-)asm-offsets.h
> > > is searched in {spl,tpl}/include/generated/.
> > >
> >
> > This patch does not apply on top of u-boot/master, unfortunately.
> >
> > > Requested-by: Bin Meng <bmeng.cn at gmail.com>
> > > Signed-off-by: Masahiro Yamada <masahiroy at kernel.org>
> > > ---
> > >
> > >  Kbuild               |  4 ++--
> > >  scripts/Makefile.spl | 10 ++++++++--
> > >  2 files changed, 10 insertions(+), 4 deletions(-)
> > >
> >
> > I manually applied the changes and had a test, but it failed to build.
> > See below:
> >
> > > diff --git a/Kbuild b/Kbuild
> > > index 6014f7c34a..1eac091594 100644
> > > --- a/Kbuild
> > > +++ b/Kbuild
> > > @@ -10,7 +10,7 @@ generic-offsets-file := include/generated/generic-asm-offsets.h
> > >  always  := $(generic-offsets-file)
> > >  targets := lib/asm-offsets.s
> > >
> > > -$(obj)/$(generic-offsets-file): lib/asm-offsets.s FORCE
> > > +$(obj)/$(generic-offsets-file): $(obj)/lib/asm-offsets.s FORCE
> >
> >   CC      spl/./lib/asm-offsets.s
> > cc1: fatal error: can't open 'spl/./lib/asm-offsets.s' for writing: No
> > such file or directory
> > compilation terminated.
> > scripts/Makefile.build:166: recipe for target 'spl/./lib/asm-offsets.s' failed
> > make[2]: *** [spl/./lib/asm-offsets.s] Error 1
> > scripts/Makefile.spl:432: recipe for target 'prepare' failed
> > make[1]: *** [prepare] Error 2
> > Makefile:1917: recipe for target 'spl/u-boot-spl' failed
> > make: *** [spl/u-boot-spl] Error 2
> >
> > When I removed the changes for this line, and the line below, the
> > build could pass.
> > The spl/include/generated/(generic-)asm-offsets.h were generated, and
> > I verified the macro values are correct and different with the U-Boot
> > proper one.
> >
> > But since there is no $(obj) prepended to lib/asm-offsets.s, I believe
> > this is still not perfect.
>
>
> This is because you remove the needed changes.

I applied patch 1, and manually applied patch 3 in this series. Do you
mean patch 2 is needed for patch 3 to work properly?

Regards,
Bin
Masahiro Yamada April 17, 2020, 11:19 a.m. UTC | #4
Hi Bin,

On Fri, Apr 17, 2020 at 4:10 PM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Masahiro,
>

> >
> >
> > This is because you remove the needed changes.
>
> I applied patch 1, and manually applied patch 3 in this series. Do you
> mean patch 2 is needed for patch 3 to work properly?

Yes, that's why I sent the 3 patches in a series.
diff mbox series

Patch

diff --git a/Kbuild b/Kbuild
index 6014f7c34a..1eac091594 100644
--- a/Kbuild
+++ b/Kbuild
@@ -10,7 +10,7 @@  generic-offsets-file := include/generated/generic-asm-offsets.h
 always  := $(generic-offsets-file)
 targets := lib/asm-offsets.s
 
-$(obj)/$(generic-offsets-file): lib/asm-offsets.s FORCE
+$(obj)/$(generic-offsets-file): $(obj)/lib/asm-offsets.s FORCE
 	$(call filechk,offsets,__GENERIC_ASM_OFFSETS_H__)
 
 #####
@@ -25,5 +25,5 @@  targets += arch/$(ARCH)/lib/asm-offsets.s
 
 CFLAGS_asm-offsets.o := -DDO_DEPS_ONLY
 
-$(obj)/$(offsets-file): arch/$(ARCH)/lib/asm-offsets.s FORCE
+$(obj)/$(offsets-file): $(obj)/arch/$(ARCH)/lib/asm-offsets.s FORCE
 	$(call filechk,offsets,__ASM_OFFSETS_H__)
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 98c04bbbfc..c3fa45f520 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -22,6 +22,8 @@  include $(srctree)/scripts/Kbuild.include
 -include include/config/auto.conf
 -include $(obj)/include/autoconf.mk
 
+UBOOTINCLUDE := -I$(obj)/include $(UBOOTINCLUDE)
+
 KBUILD_CPPFLAGS += -DCONFIG_SPL_BUILD
 ifeq ($(CONFIG_TPL_BUILD),y)
 KBUILD_CPPFLAGS += -DCONFIG_TPL_BUILD
@@ -311,7 +313,7 @@  cmd_plat = $(CC) $(c_flags) -c $< -o $(filter-out $(PHONY),$@)
 
 targets += $(obj)/dts/dt-platdata.o
 $(obj)/dts/dt-platdata.o: $(obj)/dts/dt-platdata.c \
-		include/generated/dt-structs-gen.h FORCE
+		include/generated/dt-structs-gen.h prepare FORCE
 	$(call if_changed,plat)
 
 PHONY += dts_dir
@@ -422,9 +424,13 @@  $(obj)/$(SPL_BIN): $(u-boot-spl-platdata) $(u-boot-spl-init) \
 $(sort $(u-boot-spl-init) $(u-boot-spl-main)): $(u-boot-spl-dirs) ;
 
 PHONY += $(u-boot-spl-dirs)
-$(u-boot-spl-dirs): $(u-boot-spl-platdata)
+$(u-boot-spl-dirs): $(u-boot-spl-platdata) prepare
 	$(Q)$(MAKE) $(build)=$@
 
+PHONY += prepare
+prepare:
+	$(Q)$(MAKE) $(build)=$(obj)/.
+
 quiet_cmd_cpp_lds = LDS     $@
 cmd_cpp_lds = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \
 		-D__ASSEMBLY__ -x assembler-with-cpp -std=c99 -P -o $@ $<