* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-22 7:44 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2020-04-22 7:44 UTC (permalink / raw)
To: Masahiro Yamada, Kees Cook, Russell King
Cc: Linux Kernel Mailing List, Linux ARM, Ard Biesheuvel
Hi Yamada-san, Kees, Russell,
-CC RMK's patch system
+CC lakml
On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> Copying source files during the build time may not end up with
> as clean code as expected.
>
> lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> nicely. Let's follow this approach for the arm decompressor, too.
>
> Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> the Makefile messes. Another nice thing is we no longer need to
> maintain the own libfdt_env.h because the decompressor can include
> <linux/libfdt_env.h>.
>
> There is a subtle problem when generated files are turned into
> check-in files.
>
> When you are doing a rebuild of an existing object tree with O=
> option, there exists stale "shipped" copies that the old Makefile
> implementation created. The build system ends up with compiling the
> stale generated files because Make searches for prerequisites in the
> current directory, i.e. $(objtree) first, and then the directory
> listed in VPATH, i.e. $(srctree).
>
> To mend this issue, I added the following code:
>
> ifdef building_out_of_srctree
> $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> endif
>
> This will need to stay for a while because "git bisect" crossing this
> commit, otherwise, would result in a build error.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
simplify libfdt builds") in arm/for-next.
In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
memory from DTB"[1] on top of this, which would conditionally add
another source file to libfdt_objs, I have a few questions.
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> compress-$(CONFIG_KERNEL_XZ) = xzkern
> compress-$(CONFIG_KERNEL_LZ4) = lz4
>
> -# Borrowed libfdt files for the ATAG compatibility mode
> -
> -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> -
> -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
>
I guess the code below can be moved out of the ifeq block, as it doesn't
really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
becomes empty?
If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
> -$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
> - $(call cmd,shipped)
> +OBJS += $(libfdt_objs)
>
> -$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
> - $(addprefix $(obj)/,$(libfdt_hdrs))
> +# -fstack-protector-strong triggers protection checks in this code,
> +# but it is being used too early to link to meaningful stack_chk logic.
> +nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> +$(foreach o, $(libfdt_objs), \
> + $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
Is there a real reason this is only applied to a subset of the C object
files, and not to all of them? Or have we been lucky so far, by not
triggering the issue in decompressed.c, misc.c, and string.c (yet)?
Thanks!
> +
> +# These were previously generated C files. When you are building the kernel
> +# with O=, make sure to remove the stale files in the output tree. Otherwise,
> +# the build system wrongly compiles the stale ones.
> +ifdef building_out_of_srctree
> +$(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> +endif
>
> -ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> -OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
>
> targets := vmlinux vmlinux.lds piggy_data piggy.o \
> lib1funcs.o ashldi3.o bswapsdi2.o \
> head.o $(OBJS)
>
> -clean-files += piggy_data lib1funcs.S ashldi3.S bswapsdi2.S \
> - $(libfdt) $(libfdt_hdrs) hyp-stub.S
> +clean-files += piggy_data lib1funcs.S ashldi3.S bswapsdi2.S hyp-stub.S
>
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
>
> @@ -107,15 +109,6 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
> KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
> endif
>
> -# -fstack-protector-strong triggers protection checks in this code,
> -# but it is being used too early to link to meaningful stack_chk logic.
> -nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> -CFLAGS_atags_to_fdt.o := $(nossp-flags-y)
> -CFLAGS_fdt.o := $(nossp-flags-y)
> -CFLAGS_fdt_ro.o := $(nossp-flags-y)
> -CFLAGS_fdt_rw.o := $(nossp-flags-y)
> -CFLAGS_fdt_wip.o := $(nossp-flags-y)
> -
> ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
> -I$(obj) $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
> asflags-y := -DZIMAGE
[1] https://lore.kernel.org/r/20200415153409.30112-1-geert+renesas@glider.be
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
2020-04-22 7:44 ` Geert Uytterhoeven
@ 2020-04-22 7:58 ` Russell King - ARM Linux admin
-1 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-22 7:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Masahiro Yamada, Kees Cook, Ard Biesheuvel, Linux ARM,
Linux Kernel Mailing List
On Wed, Apr 22, 2020 at 09:44:38AM +0200, Geert Uytterhoeven wrote:
> Hi Yamada-san, Kees, Russell,
>
> -CC RMK's patch system
> +CC lakml
>
> On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > Copying source files during the build time may not end up with
> > as clean code as expected.
> >
> > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > nicely. Let's follow this approach for the arm decompressor, too.
> >
> > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > the Makefile messes. Another nice thing is we no longer need to
> > maintain the own libfdt_env.h because the decompressor can include
> > <linux/libfdt_env.h>.
> >
> > There is a subtle problem when generated files are turned into
> > check-in files.
> >
> > When you are doing a rebuild of an existing object tree with O=
> > option, there exists stale "shipped" copies that the old Makefile
> > implementation created. The build system ends up with compiling the
> > stale generated files because Make searches for prerequisites in the
> > current directory, i.e. $(objtree) first, and then the directory
> > listed in VPATH, i.e. $(srctree).
> >
> > To mend this issue, I added the following code:
> >
> > ifdef building_out_of_srctree
> > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > endif
> >
> > This will need to stay for a while because "git bisect" crossing this
> > commit, otherwise, would result in a build error.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> simplify libfdt builds") in arm/for-next.
>
> In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> memory from DTB"[1] on top of this, which would conditionally add
> another source file to libfdt_objs, I have a few questions.
>
> > --- a/arch/arm/boot/compressed/Makefile
> > +++ b/arch/arm/boot/compressed/Makefile
> > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > compress-$(CONFIG_KERNEL_LZ4) = lz4
> >
> > -# Borrowed libfdt files for the ATAG compatibility mode
> > -
> > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > -
> > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> >
>
> I guess the code below can be moved out of the ifeq block, as it doesn't
> really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> becomes empty?
> If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
> > -$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
> > - $(call cmd,shipped)
> > +OBJS += $(libfdt_objs)
> >
> > -$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
> > - $(addprefix $(obj)/,$(libfdt_hdrs))
> > +# -fstack-protector-strong triggers protection checks in this code,
> > +# but it is being used too early to link to meaningful stack_chk logic.
> > +nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> > +$(foreach o, $(libfdt_objs), \
> > + $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> Is there a real reason this is only applied to a subset of the C object
> files, and not to all of them? Or have we been lucky so far, by not
> triggering the issue in decompressed.c, misc.c, and string.c (yet)?
I don't remember the details. See commit 7f66cd3f5420, which came from
Kees which introduced this.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-22 7:58 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-22 7:58 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Kernel Mailing List, Masahiro Yamada, Kees Cook, Linux ARM,
Ard Biesheuvel
On Wed, Apr 22, 2020 at 09:44:38AM +0200, Geert Uytterhoeven wrote:
> Hi Yamada-san, Kees, Russell,
>
> -CC RMK's patch system
> +CC lakml
>
> On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > Copying source files during the build time may not end up with
> > as clean code as expected.
> >
> > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > nicely. Let's follow this approach for the arm decompressor, too.
> >
> > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > the Makefile messes. Another nice thing is we no longer need to
> > maintain the own libfdt_env.h because the decompressor can include
> > <linux/libfdt_env.h>.
> >
> > There is a subtle problem when generated files are turned into
> > check-in files.
> >
> > When you are doing a rebuild of an existing object tree with O=
> > option, there exists stale "shipped" copies that the old Makefile
> > implementation created. The build system ends up with compiling the
> > stale generated files because Make searches for prerequisites in the
> > current directory, i.e. $(objtree) first, and then the directory
> > listed in VPATH, i.e. $(srctree).
> >
> > To mend this issue, I added the following code:
> >
> > ifdef building_out_of_srctree
> > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > endif
> >
> > This will need to stay for a while because "git bisect" crossing this
> > commit, otherwise, would result in a build error.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> simplify libfdt builds") in arm/for-next.
>
> In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> memory from DTB"[1] on top of this, which would conditionally add
> another source file to libfdt_objs, I have a few questions.
>
> > --- a/arch/arm/boot/compressed/Makefile
> > +++ b/arch/arm/boot/compressed/Makefile
> > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > compress-$(CONFIG_KERNEL_LZ4) = lz4
> >
> > -# Borrowed libfdt files for the ATAG compatibility mode
> > -
> > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > -
> > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> >
>
> I guess the code below can be moved out of the ifeq block, as it doesn't
> really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> becomes empty?
> If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
> > -$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/%
> > - $(call cmd,shipped)
> > +OBJS += $(libfdt_objs)
> >
> > -$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \
> > - $(addprefix $(obj)/,$(libfdt_hdrs))
> > +# -fstack-protector-strong triggers protection checks in this code,
> > +# but it is being used too early to link to meaningful stack_chk logic.
> > +nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> > +$(foreach o, $(libfdt_objs), \
> > + $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> Is there a real reason this is only applied to a subset of the C object
> files, and not to all of them? Or have we been lucky so far, by not
> triggering the issue in decompressed.c, misc.c, and string.c (yet)?
I don't remember the details. See commit 7f66cd3f5420, which came from
Kees which introduced this.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
2020-04-22 7:58 ` Russell King - ARM Linux admin
@ 2020-04-23 17:52 ` Kees Cook
-1 siblings, 0 replies; 13+ messages in thread
From: Kees Cook @ 2020-04-23 17:52 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Geert Uytterhoeven, Masahiro Yamada, Ard Biesheuvel, Linux ARM,
Linux Kernel Mailing List
On Wed, Apr 22, 2020 at 08:58:54AM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Apr 22, 2020 at 09:44:38AM +0200, Geert Uytterhoeven wrote:
> > Is there a real reason this is only applied to a subset of the C object
> > files, and not to all of them? Or have we been lucky so far, by not
> > triggering the issue in decompressed.c, misc.c, and string.c (yet)?
>
> I don't remember the details. See commit 7f66cd3f5420, which came from
> Kees which introduced this.
Just to clarify: the original change was just removing it where it was
detected in the then-current build. I was going for the least invasive
change to the build system.
--
Kees Cook
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-23 17:52 ` Kees Cook
0 siblings, 0 replies; 13+ messages in thread
From: Kees Cook @ 2020-04-23 17:52 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Masahiro Yamada, Geert Uytterhoeven, Linux Kernel Mailing List,
Linux ARM, Ard Biesheuvel
On Wed, Apr 22, 2020 at 08:58:54AM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Apr 22, 2020 at 09:44:38AM +0200, Geert Uytterhoeven wrote:
> > Is there a real reason this is only applied to a subset of the C object
> > files, and not to all of them? Or have we been lucky so far, by not
> > triggering the issue in decompressed.c, misc.c, and string.c (yet)?
>
> I don't remember the details. See commit 7f66cd3f5420, which came from
> Kees which introduced this.
Just to clarify: the original change was just removing it where it was
detected in the then-current build. I was going for the least invasive
change to the build system.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
2020-04-22 7:44 ` Geert Uytterhoeven
@ 2020-04-22 12:30 ` Masahiro Yamada
-1 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2020-04-22 12:30 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kees Cook, Russell King, Ard Biesheuvel, Linux ARM,
Linux Kernel Mailing List
On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Yamada-san, Kees, Russell,
>
> -CC RMK's patch system
> +CC lakml
>
> On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > Copying source files during the build time may not end up with
> > as clean code as expected.
> >
> > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > nicely. Let's follow this approach for the arm decompressor, too.
> >
> > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > the Makefile messes. Another nice thing is we no longer need to
> > maintain the own libfdt_env.h because the decompressor can include
> > <linux/libfdt_env.h>.
> >
> > There is a subtle problem when generated files are turned into
> > check-in files.
> >
> > When you are doing a rebuild of an existing object tree with O=
> > option, there exists stale "shipped" copies that the old Makefile
> > implementation created. The build system ends up with compiling the
> > stale generated files because Make searches for prerequisites in the
> > current directory, i.e. $(objtree) first, and then the directory
> > listed in VPATH, i.e. $(srctree).
> >
> > To mend this issue, I added the following code:
> >
> > ifdef building_out_of_srctree
> > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > endif
> >
> > This will need to stay for a while because "git bisect" crossing this
> > commit, otherwise, would result in a build error.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> simplify libfdt builds") in arm/for-next.
>
> In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> memory from DTB"[1] on top of this, which would conditionally add
> another source file to libfdt_objs, I have a few questions.
>
> > --- a/arch/arm/boot/compressed/Makefile
> > +++ b/arch/arm/boot/compressed/Makefile
> > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > compress-$(CONFIG_KERNEL_LZ4) = lz4
> >
> > -# Borrowed libfdt files for the ATAG compatibility mode
> > -
> > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > -
> > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> >
>
> I guess the code below can be moved out of the ifeq block, as it doesn't
> really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> becomes empty?
> If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
Right. We can narrow the ifeq block.
I did not know your on-going work.
If I had known your work adding one more file here,
I would have written this part as follows:
------------------------------>8----------------------------------
libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
OBJS += $(libfdt_objs) atags_to_fdt.o
endif
# -fstack-protector-strong triggers protection checks in this code,
# but it is being used too early to link to meaningful stack_chk logic.
nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
$(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
# These were previously generated C files. When you are building the kernel
# with O=, make sure to remove the stale files in the output tree. Otherwise,
# the build system wrongly compiles the stale ones.
ifdef building_out_of_srctree
$(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
endif
-------------------------------------->8-----------------------------
So, how shall we move forward?
Leave the necessary Makefile change to Geert?
If Geert and Russell want to replace my patch,
I can send v5 with the code above.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-22 12:30 ` Masahiro Yamada
0 siblings, 0 replies; 13+ messages in thread
From: Masahiro Yamada @ 2020-04-22 12:30 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linux Kernel Mailing List, Ard Biesheuvel, Kees Cook, Linux ARM,
Russell King
On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Yamada-san, Kees, Russell,
>
> -CC RMK's patch system
> +CC lakml
>
> On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > Copying source files during the build time may not end up with
> > as clean code as expected.
> >
> > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > nicely. Let's follow this approach for the arm decompressor, too.
> >
> > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > the Makefile messes. Another nice thing is we no longer need to
> > maintain the own libfdt_env.h because the decompressor can include
> > <linux/libfdt_env.h>.
> >
> > There is a subtle problem when generated files are turned into
> > check-in files.
> >
> > When you are doing a rebuild of an existing object tree with O=
> > option, there exists stale "shipped" copies that the old Makefile
> > implementation created. The build system ends up with compiling the
> > stale generated files because Make searches for prerequisites in the
> > current directory, i.e. $(objtree) first, and then the directory
> > listed in VPATH, i.e. $(srctree).
> >
> > To mend this issue, I added the following code:
> >
> > ifdef building_out_of_srctree
> > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > endif
> >
> > This will need to stay for a while because "git bisect" crossing this
> > commit, otherwise, would result in a build error.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>
> This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> simplify libfdt builds") in arm/for-next.
>
> In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> memory from DTB"[1] on top of this, which would conditionally add
> another source file to libfdt_objs, I have a few questions.
>
> > --- a/arch/arm/boot/compressed/Makefile
> > +++ b/arch/arm/boot/compressed/Makefile
> > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > compress-$(CONFIG_KERNEL_LZ4) = lz4
> >
> > -# Borrowed libfdt files for the ATAG compatibility mode
> > -
> > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > -
> > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> >
>
> I guess the code below can be moved out of the ifeq block, as it doesn't
> really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> becomes empty?
> If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
Right. We can narrow the ifeq block.
I did not know your on-going work.
If I had known your work adding one more file here,
I would have written this part as follows:
------------------------------>8----------------------------------
libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
OBJS += $(libfdt_objs) atags_to_fdt.o
endif
# -fstack-protector-strong triggers protection checks in this code,
# but it is being used too early to link to meaningful stack_chk logic.
nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
$(foreach o, $(libfdt_objs) atags_to_fdt.o, \
$(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
# These were previously generated C files. When you are building the kernel
# with O=, make sure to remove the stale files in the output tree. Otherwise,
# the build system wrongly compiles the stale ones.
ifdef building_out_of_srctree
$(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
endif
-------------------------------------->8-----------------------------
So, how shall we move forward?
Leave the necessary Makefile change to Geert?
If Geert and Russell want to replace my patch,
I can send v5 with the code above.
--
Best Regards
Masahiro Yamada
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
2020-04-22 12:30 ` Masahiro Yamada
@ 2020-04-22 12:38 ` Russell King - ARM Linux admin
-1 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-22 12:38 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Geert Uytterhoeven, Linux Kernel Mailing List, Ard Biesheuvel,
Kees Cook, Linux ARM
On Wed, Apr 22, 2020 at 09:30:58PM +0900, Masahiro Yamada wrote:
> On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Yamada-san, Kees, Russell,
> >
> > -CC RMK's patch system
> > +CC lakml
> >
> > On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > Copying source files during the build time may not end up with
> > > as clean code as expected.
> > >
> > > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > > nicely. Let's follow this approach for the arm decompressor, too.
> > >
> > > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > > the Makefile messes. Another nice thing is we no longer need to
> > > maintain the own libfdt_env.h because the decompressor can include
> > > <linux/libfdt_env.h>.
> > >
> > > There is a subtle problem when generated files are turned into
> > > check-in files.
> > >
> > > When you are doing a rebuild of an existing object tree with O=
> > > option, there exists stale "shipped" copies that the old Makefile
> > > implementation created. The build system ends up with compiling the
> > > stale generated files because Make searches for prerequisites in the
> > > current directory, i.e. $(objtree) first, and then the directory
> > > listed in VPATH, i.e. $(srctree).
> > >
> > > To mend this issue, I added the following code:
> > >
> > > ifdef building_out_of_srctree
> > > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > > endif
> > >
> > > This will need to stay for a while because "git bisect" crossing this
> > > commit, otherwise, would result in a build error.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> > simplify libfdt builds") in arm/for-next.
> >
> > In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> > memory from DTB"[1] on top of this, which would conditionally add
> > another source file to libfdt_objs, I have a few questions.
> >
> > > --- a/arch/arm/boot/compressed/Makefile
> > > +++ b/arch/arm/boot/compressed/Makefile
> > > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > > compress-$(CONFIG_KERNEL_LZ4) = lz4
> > >
> > > -# Borrowed libfdt files for the ATAG compatibility mode
> > > -
> > > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > > -
> > > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> > >
> >
> > I guess the code below can be moved out of the ifeq block, as it doesn't
> > really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> > becomes empty?
> > If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> > to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
>
>
> Right. We can narrow the ifeq block.
> I did not know your on-going work.
>
>
> If I had known your work adding one more file here,
> I would have written this part as follows:
>
>
> ------------------------------>8----------------------------------
> libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
>
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> $(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> # These were previously generated C files. When you are building the kernel
> # with O=, make sure to remove the stale files in the output tree. Otherwise,
> # the build system wrongly compiles the stale ones.
> ifdef building_out_of_srctree
> $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> endif
> -------------------------------------->8-----------------------------
>
>
>
>
> So, how shall we move forward?
>
> Leave the necessary Makefile change to Geert?
>
> If Geert and Russell want to replace my patch,
> I can send v5 with the code above.
I've no problem with replacing your patch...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-22 12:38 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 13+ messages in thread
From: Russell King - ARM Linux admin @ 2020-04-22 12:38 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux ARM, Geert Uytterhoeven, Linux Kernel Mailing List,
Kees Cook, Ard Biesheuvel
On Wed, Apr 22, 2020 at 09:30:58PM +0900, Masahiro Yamada wrote:
> On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Yamada-san, Kees, Russell,
> >
> > -CC RMK's patch system
> > +CC lakml
> >
> > On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > Copying source files during the build time may not end up with
> > > as clean code as expected.
> > >
> > > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > > nicely. Let's follow this approach for the arm decompressor, too.
> > >
> > > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > > the Makefile messes. Another nice thing is we no longer need to
> > > maintain the own libfdt_env.h because the decompressor can include
> > > <linux/libfdt_env.h>.
> > >
> > > There is a subtle problem when generated files are turned into
> > > check-in files.
> > >
> > > When you are doing a rebuild of an existing object tree with O=
> > > option, there exists stale "shipped" copies that the old Makefile
> > > implementation created. The build system ends up with compiling the
> > > stale generated files because Make searches for prerequisites in the
> > > current directory, i.e. $(objtree) first, and then the directory
> > > listed in VPATH, i.e. $(srctree).
> > >
> > > To mend this issue, I added the following code:
> > >
> > > ifdef building_out_of_srctree
> > > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > > endif
> > >
> > > This will need to stay for a while because "git bisect" crossing this
> > > commit, otherwise, would result in a build error.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> > simplify libfdt builds") in arm/for-next.
> >
> > In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> > memory from DTB"[1] on top of this, which would conditionally add
> > another source file to libfdt_objs, I have a few questions.
> >
> > > --- a/arch/arm/boot/compressed/Makefile
> > > +++ b/arch/arm/boot/compressed/Makefile
> > > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > > compress-$(CONFIG_KERNEL_LZ4) = lz4
> > >
> > > -# Borrowed libfdt files for the ATAG compatibility mode
> > > -
> > > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > > -
> > > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> > >
> >
> > I guess the code below can be moved out of the ifeq block, as it doesn't
> > really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> > becomes empty?
> > If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> > to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
>
>
> Right. We can narrow the ifeq block.
> I did not know your on-going work.
>
>
> If I had known your work adding one more file here,
> I would have written this part as follows:
>
>
> ------------------------------>8----------------------------------
> libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
>
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> $(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> # These were previously generated C files. When you are building the kernel
> # with O=, make sure to remove the stale files in the output tree. Otherwise,
> # the build system wrongly compiles the stale ones.
> ifdef building_out_of_srctree
> $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> endif
> -------------------------------------->8-----------------------------
>
>
>
>
> So, how shall we move forward?
>
> Leave the necessary Makefile change to Geert?
>
> If Geert and Russell want to replace my patch,
> I can send v5 with the code above.
I've no problem with replacing your patch...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
2020-04-22 12:30 ` Masahiro Yamada
@ 2020-04-22 12:56 ` Geert Uytterhoeven
-1 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2020-04-22 12:56 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux Kernel Mailing List, Ard Biesheuvel, Kees Cook, Linux ARM,
Russell King
Hi Yamada-san,
On Wed, Apr 22, 2020 at 2:32 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > Copying source files during the build time may not end up with
> > > as clean code as expected.
> > >
> > > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > > nicely. Let's follow this approach for the arm decompressor, too.
> > >
> > > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > > the Makefile messes. Another nice thing is we no longer need to
> > > maintain the own libfdt_env.h because the decompressor can include
> > > <linux/libfdt_env.h>.
> > >
> > > There is a subtle problem when generated files are turned into
> > > check-in files.
> > >
> > > When you are doing a rebuild of an existing object tree with O=
> > > option, there exists stale "shipped" copies that the old Makefile
> > > implementation created. The build system ends up with compiling the
> > > stale generated files because Make searches for prerequisites in the
> > > current directory, i.e. $(objtree) first, and then the directory
> > > listed in VPATH, i.e. $(srctree).
> > >
> > > To mend this issue, I added the following code:
> > >
> > > ifdef building_out_of_srctree
> > > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > > endif
> > >
> > > This will need to stay for a while because "git bisect" crossing this
> > > commit, otherwise, would result in a build error.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> > simplify libfdt builds") in arm/for-next.
> >
> > In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> > memory from DTB"[1] on top of this, which would conditionally add
> > another source file to libfdt_objs, I have a few questions.
> >
> > > --- a/arch/arm/boot/compressed/Makefile
> > > +++ b/arch/arm/boot/compressed/Makefile
> > > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > > compress-$(CONFIG_KERNEL_LZ4) = lz4
> > >
> > > -# Borrowed libfdt files for the ATAG compatibility mode
> > > -
> > > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > > -
> > > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> > >
> >
> > I guess the code below can be moved out of the ifeq block, as it doesn't
> > really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> > becomes empty?
> > If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> > to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
>
>
> Right. We can narrow the ifeq block.
> I did not know your on-going work.
>
>
> If I had known your work adding one more file here,
> I would have written this part as follows:
>
>
> ------------------------------>8----------------------------------
> libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
>
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> $(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> # These were previously generated C files. When you are building the kernel
> # with O=, make sure to remove the stale files in the output tree. Otherwise,
> # the build system wrongly compiles the stale ones.
> ifdef building_out_of_srctree
> $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> endif
> -------------------------------------->8-----------------------------
Thanks, looks good to me!
> So, how shall we move forward?
>
> Leave the necessary Makefile change to Geert?
>
> If Geert and Russell want to replace my patch,
> I can send v5 with the code above.
I can fix it myself when rebasing my patch, unless you get to a v5 before me.
I just wanted to find a good approach, to avoid delaying my patch.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] ARM: decompressor: simplify libfdt builds
@ 2020-04-22 12:56 ` Geert Uytterhoeven
0 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2020-04-22 12:56 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Linux ARM, Russell King, Linux Kernel Mailing List, Kees Cook,
Ard Biesheuvel
Hi Yamada-san,
On Wed, Apr 22, 2020 at 2:32 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> On Wed, Apr 22, 2020 at 4:44 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Apr 19, 2020 at 9:26 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > Copying source files during the build time may not end up with
> > > as clean code as expected.
> > >
> > > lib/fdt*.c simply wrap scripts/dtc/libfdt/fdt*.c, and it works
> > > nicely. Let's follow this approach for the arm decompressor, too.
> > >
> > > Add four wrappers, arch/arm/boot/compressed/fdt*.c and remove
> > > the Makefile messes. Another nice thing is we no longer need to
> > > maintain the own libfdt_env.h because the decompressor can include
> > > <linux/libfdt_env.h>.
> > >
> > > There is a subtle problem when generated files are turned into
> > > check-in files.
> > >
> > > When you are doing a rebuild of an existing object tree with O=
> > > option, there exists stale "shipped" copies that the old Makefile
> > > implementation created. The build system ends up with compiling the
> > > stale generated files because Make searches for prerequisites in the
> > > current directory, i.e. $(objtree) first, and then the directory
> > > listed in VPATH, i.e. $(srctree).
> > >
> > > To mend this issue, I added the following code:
> > >
> > > ifdef building_out_of_srctree
> > > $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> > > endif
> > >
> > > This will need to stay for a while because "git bisect" crossing this
> > > commit, otherwise, would result in a build error.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > This is now commit 365a6327cd643eed ("ARM: 8968/1: decompressor:
> > simplify libfdt builds") in arm/for-next.
> >
> > In light of reworking "[PATCH v5] ARM: boot: Obtain start of physical
> > memory from DTB"[1] on top of this, which would conditionally add
> > another source file to libfdt_objs, I have a few questions.
> >
> > > --- a/arch/arm/boot/compressed/Makefile
> > > +++ b/arch/arm/boot/compressed/Makefile
> > > @@ -76,29 +76,31 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
> > > compress-$(CONFIG_KERNEL_XZ) = xzkern
> > > compress-$(CONFIG_KERNEL_LZ4) = lz4
> > >
> > > -# Borrowed libfdt files for the ATAG compatibility mode
> > > -
> > > -libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c
> > > -libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h
> > > -
> > > -libfdt_objs := $(addsuffix .o, $(basename $(libfdt)))
> > > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> > > +libfdt_objs = fdt_rw.o fdt_ro.o fdt_wip.o fdt.o atags_to_fdt.o
> > >
> >
> > I guess the code below can be moved out of the ifeq block, as it doesn't
> > really do anything if CONFIG_ARM_ATAG_DTB_COMPAT=n, and $(libfdt_objs)
> > becomes empty?
> > If not, I think I'll have to add a new Kconfig symbol ARM_BOOT_LIBFDT,
> > to be selected by ARM_ATAG_DTB_COMPAT and USE_OF.
>
>
>
> Right. We can narrow the ifeq block.
> I did not know your on-going work.
>
>
> If I had known your work adding one more file here,
> I would have written this part as follows:
>
>
> ------------------------------>8----------------------------------
> libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
>
> ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
> OBJS += $(libfdt_objs) atags_to_fdt.o
> endif
>
> # -fstack-protector-strong triggers protection checks in this code,
> # but it is being used too early to link to meaningful stack_chk logic.
> nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> $(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
>
> # These were previously generated C files. When you are building the kernel
> # with O=, make sure to remove the stale files in the output tree. Otherwise,
> # the build system wrongly compiles the stale ones.
> ifdef building_out_of_srctree
> $(shell rm -f $(addprefix $(obj)/, fdt_rw.c fdt_ro.c fdt_wip.c fdt.c))
> endif
> -------------------------------------->8-----------------------------
Thanks, looks good to me!
> So, how shall we move forward?
>
> Leave the necessary Makefile change to Geert?
>
> If Geert and Russell want to replace my patch,
> I can send v5 with the code above.
I can fix it myself when rebasing my patch, unless you get to a v5 before me.
I just wanted to find a good approach, to avoid delaying my patch.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 13+ messages in thread