Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Wed, 22 Apr 2020 14:56:40 +0200 | Subject | Re: [PATCH v4] ARM: decompressor: simplify libfdt builds |
| |
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
| |