| Message ID | 1557756964-13087-1-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] powerpc/boot: pass CONFIG options in a simpler and morerobust way | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 50 lines checked |
On Mon, May 13, 2019 at 11:24 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures > with -j 1") was also wrong. > > The correct dependency is: > > $(obj)/serial.o: $(obj)/autoconf.h > > However, I do not see the reason why we need to copy autoconf.h to > arch/power/boot/. Nor do I see consistency in the way of passing > CONFIG options. > > decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which > are passed via the command line. > > serial.c includes autoconf.h to reference a couple of CONFIG options, > but this is fragile because we often forget to include "autoconf.h" > from source files. > > In fact, it is already broken. > > ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way > to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. > > Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from > all .c and .S files in arch/powerpc/boot/. > > I also removed the -traditional flag to make include/linux/kconfig.h > work. This flag makes the preprocessor imitate the behavior of the > pre-standard C compiler, but I do not understand why it is necessary. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > Ping. > Changes in v2: > - reword commit log > > arch/powerpc/boot/.gitignore | 2 -- > arch/powerpc/boot/Makefile | 14 +++----------- > arch/powerpc/boot/serial.c | 1 - > 3 files changed, 3 insertions(+), 14 deletions(-) > > diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore > index 32034a0c..6610665 100644 > --- a/arch/powerpc/boot/.gitignore > +++ b/arch/powerpc/boot/.gitignore > @@ -44,5 +44,3 @@ fdt_sw.c > fdt_wip.c > libfdt.h > libfdt_internal.h > -autoconf.h > - > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile > index 73d1f35..b8a82be 100644 > --- a/arch/powerpc/boot/Makefile > +++ b/arch/powerpc/boot/Makefile > @@ -20,9 +20,6 @@ > > all: $(obj)/zImage > > -compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP > -compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ > - > ifdef CROSS32_COMPILE > BOOTCC := $(CROSS32_COMPILE)gcc > BOOTAR := $(CROSS32_COMPILE)ar > @@ -34,7 +31,7 @@ endif > BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ > -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \ > -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \ > - -D$(compress-y) > + $(LINUXINCLUDE) > > ifdef CONFIG_PPC64_BOOT_WRAPPER > BOOTCFLAGS += -m64 > @@ -51,7 +48,7 @@ BOOTCFLAGS += -mlittle-endian > BOOTCFLAGS += $(call cc-option,-mabi=elfv2) > endif > > -BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc > +BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc > > BOOTARFLAGS := -cr$(KBUILD_ARFLAGS) > > @@ -202,14 +199,9 @@ $(obj)/empty.c: > $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S > $(Q)cp $< $@ > > -$(srctree)/$(src)/serial.c: $(obj)/autoconf.h > - > -$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/% > - $(Q)cp $< $@ > - > clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \ > $(zlib-decomp-) $(libfdt) $(libfdtheader) \ > - autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds > + empty.c zImage.coff.lds zImage.ps3.lds zImage.lds > > quiet_cmd_bootcc = BOOTCC $@ > cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $< > diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c > index b0491b8..9457863 100644 > --- a/arch/powerpc/boot/serial.c > +++ b/arch/powerpc/boot/serial.c > @@ -18,7 +18,6 @@ > #include "stdio.h" > #include "io.h" > #include "ops.h" > -#include "autoconf.h" > > static int serial_open(void) > { > -- > 2.7.4 >
Masahiro Yamada <yamada.masahiro@socionext.com> writes: > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures > with -j 1") was also wrong. > > The correct dependency is: > > $(obj)/serial.o: $(obj)/autoconf.h > > However, I do not see the reason why we need to copy autoconf.h to > arch/power/boot/. Nor do I see consistency in the way of passing > CONFIG options. > > decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which > are passed via the command line. > > serial.c includes autoconf.h to reference a couple of CONFIG options, > but this is fragile because we often forget to include "autoconf.h" > from source files. > > In fact, it is already broken. > > ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way > to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. > > Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from > all .c and .S files in arch/powerpc/boot/. This breaks our skiroot_defconfig, I don't know why yet: In file included from /kisskb/src/arch/powerpc/boot/../../../lib/decompress_unxz.c:236:0, from /kisskb/src/arch/powerpc/boot/decompress.c:42: /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c: In function 'bcj_powerpc': /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c:166:11: warning: implicit declaration of function 'get_unaligned_be32' [-Wimplicit-function-declaration] instr = get_unaligned_be32(buf + i); http://kisskb.ellerman.id.au/kisskb/buildresult/13862914/ cheers
On Thu, Jul 4, 2019 at 9:26 AM Michael Ellerman <mpe@ellerman.id.au> wrote: > > Masahiro Yamada <yamada.masahiro@socionext.com> writes: > > > Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") > > was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures > > with -j 1") was also wrong. > > > > The correct dependency is: > > > > $(obj)/serial.o: $(obj)/autoconf.h > > > > However, I do not see the reason why we need to copy autoconf.h to > > arch/power/boot/. Nor do I see consistency in the way of passing > > CONFIG options. > > > > decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which > > are passed via the command line. > > > > serial.c includes autoconf.h to reference a couple of CONFIG options, > > but this is fragile because we often forget to include "autoconf.h" > > from source files. > > > > In fact, it is already broken. > > > > ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way > > to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. > > > > Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from > > all .c and .S files in arch/powerpc/boot/. > > This breaks our skiroot_defconfig, I don't know why yet: > > In file included from /kisskb/src/arch/powerpc/boot/../../../lib/decompress_unxz.c:236:0, > from /kisskb/src/arch/powerpc/boot/decompress.c:42: > /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c: In function 'bcj_powerpc': > /kisskb/src/arch/powerpc/boot/../../../lib/xz/xz_dec_bcj.c:166:11: warning: implicit declaration of function 'get_unaligned_be32' [-Wimplicit-function-declaration] > instr = get_unaligned_be32(buf + i); OK, now I see the cause of the error. I will insert a new patch in v3. Thanks. > > http://kisskb.ellerman.id.au/kisskb/buildresult/13862914/ > > cheers
diff --git a/arch/powerpc/boot/.gitignore b/arch/powerpc/boot/.gitignore index 32034a0c..6610665 100644 --- a/arch/powerpc/boot/.gitignore +++ b/arch/powerpc/boot/.gitignore @@ -44,5 +44,3 @@ fdt_sw.c fdt_wip.c libfdt.h libfdt_internal.h -autoconf.h - diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 73d1f35..b8a82be 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -20,9 +20,6 @@ all: $(obj)/zImage -compress-$(CONFIG_KERNEL_GZIP) := CONFIG_KERNEL_GZIP -compress-$(CONFIG_KERNEL_XZ) := CONFIG_KERNEL_XZ - ifdef CROSS32_COMPILE BOOTCC := $(CROSS32_COMPILE)gcc BOOTAR := $(CROSS32_COMPILE)ar @@ -34,7 +31,7 @@ endif BOOTCFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \ -fno-strict-aliasing -O2 -msoft-float -mno-altivec -mno-vsx \ -pipe -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \ - -D$(compress-y) + $(LINUXINCLUDE) ifdef CONFIG_PPC64_BOOT_WRAPPER BOOTCFLAGS += -m64 @@ -51,7 +48,7 @@ BOOTCFLAGS += -mlittle-endian BOOTCFLAGS += $(call cc-option,-mabi=elfv2) endif -BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -traditional -nostdinc +BOOTAFLAGS := -D__ASSEMBLY__ $(BOOTCFLAGS) -nostdinc BOOTARFLAGS := -cr$(KBUILD_ARFLAGS) @@ -202,14 +199,9 @@ $(obj)/empty.c: $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S $(Q)cp $< $@ -$(srctree)/$(src)/serial.c: $(obj)/autoconf.h - -$(obj)/autoconf.h: $(obj)/%: $(objtree)/include/generated/% - $(Q)cp $< $@ - clean-files := $(zlib-) $(zlibheader-) $(zliblinuxheader-) \ $(zlib-decomp-) $(libfdt) $(libfdtheader) \ - autoconf.h empty.c zImage.coff.lds zImage.ps3.lds zImage.lds + empty.c zImage.coff.lds zImage.ps3.lds zImage.lds quiet_cmd_bootcc = BOOTCC $@ cmd_bootcc = $(BOOTCC) -Wp,-MD,$(depfile) $(BOOTCFLAGS) -c -o $@ $< diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c index b0491b8..9457863 100644 --- a/arch/powerpc/boot/serial.c +++ b/arch/powerpc/boot/serial.c @@ -18,7 +18,6 @@ #include "stdio.h" #include "io.h" #include "ops.h" -#include "autoconf.h" static int serial_open(void) {
Commit 5e9dcb6188a4 ("powerpc/boot: Expose Kconfig symbols to wrapper") was wrong, but commit e41b93a6be57 ("powerpc/boot: Fix build failures with -j 1") was also wrong. The correct dependency is: $(obj)/serial.o: $(obj)/autoconf.h However, I do not see the reason why we need to copy autoconf.h to arch/power/boot/. Nor do I see consistency in the way of passing CONFIG options. decompress.c references CONFIG_KERNEL_GZIP and CONFIG_KERNEL_XZ, which are passed via the command line. serial.c includes autoconf.h to reference a couple of CONFIG options, but this is fragile because we often forget to include "autoconf.h" from source files. In fact, it is already broken. ppc_asm.h references CONFIG_PPC_8xx, but utils.S is not given any way to access CONFIG options. So, CONFIG_PPC_8xx is never defined here. Pass $(LINUXINCLUDE) to make sure CONFIG options are accessible from all .c and .S files in arch/powerpc/boot/. I also removed the -traditional flag to make include/linux/kconfig.h work. This flag makes the preprocessor imitate the behavior of the pre-standard C compiler, but I do not understand why it is necessary. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- Changes in v2: - reword commit log arch/powerpc/boot/.gitignore | 2 -- arch/powerpc/boot/Makefile | 14 +++----------- arch/powerpc/boot/serial.c | 1 - 3 files changed, 3 insertions(+), 14 deletions(-)