| Message ID | 20240815110059.4912-1-tianyuanhao3@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] kbuild: Only build dtc if needed | expand |
Hi TIAN, On Thu, 15 Aug 2024 at 12:01, TIAN Yuanhao <tianyuanhao3@163.com> wrote: > > At present Linux always builds dtc if CONFIG_DTC is defined, even when > DTC is provided. The built dtc is not actually used, so this is a waste > of time. > > Update the Makefile logic to build dtc and fdtoverlay only if DTC or > FDTOVERLAY is not provided. > > Also, add an fdtoverlay wrapper to hide the actual path differences of > fdtoverlay from the make_fit.py script. > > Refs: > https://github.com/u-boot/u-boot/commit/93b196532254366f653b4d763f69e49ff193f06c > https://github.com/torvalds/linux/commit/6b22b3d1614af1a775f2ef006009f15077592c9c > > Signed-off-by: TIAN Yuanhao <tianyuanhao3@163.com> > Cc: Chen-Yu Tsai <wenst@chromium.org> > Cc: Rob Herring (Arm) <robh@kernel.org> > Cc: Simon Glass <sjg@chromium.org> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Cc: linux-kbuild@vger.kernel.org > --- > Makefile | 13 ++++++++++++- > scripts/Makefile.lib | 5 ++--- > scripts/fdtoverlay.sh | 7 +++++++ > scripts/make_fit.py | 2 +- > 4 files changed, 22 insertions(+), 5 deletions(-) > create mode 100755 scripts/fdtoverlay.sh > Reviewed-by: Simon Glass <sjg@chromium.org> > diff --git a/Makefile b/Makefile > index 0a364e34f50b..6e56696e85a1 100644 > --- a/Makefile > +++ b/Makefile > @@ -1419,9 +1419,20 @@ endif > > endif > > +# The dtc and fdtoverlay are automatically built unless DTC or FDTOVERLAY is > +# provided. > +DTC_INTREE := $(objtree)/scripts/dtc/dtc > +DTC ?= $(DTC_INTREE) > + > +FDTOVERLAY_INTREE := $(objtree)/scripts/dtc/fdtoverlay > +FDTOVERLAY ?= $(FDTOVERLAY_INTREE) > + > PHONY += scripts_dtc > scripts_dtc: scripts_basic > - $(Q)$(MAKE) $(build)=scripts/dtc > + $(Q)if [ "$(DTC)" = "$(DTC_INTREE)" ] || \ > + [ "$(FDTOVERLAY)" = "$(FDTOVERLAY_INTREE)" ]; then \ > + $(MAKE) $(build)=scripts/dtc; \ > + fi > > ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) > export CHECK_DTBS=y > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index fe3668dc4954..04ba30dadc8f 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -352,7 +352,6 @@ quiet_cmd_gzip = GZIP $@ > > # DTC > # --------------------------------------------------------------------------- > -DTC ?= $(objtree)/scripts/dtc/dtc > DTC_FLAGS += \ > -Wno-unique_unit_address > > @@ -415,10 +414,10 @@ DT_CHECK_CMD = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) > # recorded in the .*.cmd file. > ifneq ($(CHECK_DTBS),) > quiet_cmd_fdtoverlay = DTOVLCH $@ > - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true > + cmd_fdtoverlay = $(objtree)/scripts/fdtoverlay.sh -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true > else > quiet_cmd_fdtoverlay = DTOVL $@ > - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) > + cmd_fdtoverlay = $(objtree)/scripts/fdtoverlay.sh -o $@ -i $(filter %.dtb %.dtbo, $^) > endif > > $(multi-dtb-y): FORCE > diff --git a/scripts/fdtoverlay.sh b/scripts/fdtoverlay.sh > new file mode 100755 > index 000000000000..5bd07c47c22a > --- /dev/null > +++ b/scripts/fdtoverlay.sh > @@ -0,0 +1,7 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# > +# An fdtoverlay wrapper > +# scripts/make_fit.py uses the name of this script as a special marker. > + > +exec "${FDTOVERLAY}" "$@" > diff --git a/scripts/make_fit.py b/scripts/make_fit.py > index 4a1bb2f55861..37c4e1c8d5c6 100755 > --- a/scripts/make_fit.py > +++ b/scripts/make_fit.py > @@ -238,7 +238,7 @@ def process_dtb(fname, args): > with open(cmd_fname, 'r', encoding='ascii') as inf: > cmd = inf.read() > > - if 'scripts/dtc/fdtoverlay' in cmd: > + if '/scripts/fdtoverlay.sh ' in cmd: > # This depends on the structure of the composite DTB command > files = cmd.split() > files = files[files.index('-i') + 1:] > > base-commit: 6b0f8db921abf0520081d779876d3a41069dab95 > -- > 2.45.1 >
+Masahiro (use get_maintainers.pl in the future) On Thu, Aug 15, 2024 at 5:01 AM TIAN Yuanhao <tianyuanhao3@163.com> wrote: > > At present Linux always builds dtc if CONFIG_DTC is defined, even when > DTC is provided. The built dtc is not actually used, so this is a waste > of time. That's kind of a edge usecase, so I'm not sure it is worth the complexity. We could also just make CONFIG_DTC visible so it can be disabled. Or make it a path defaulting to the built-in one. Maybe Masahiro has some ideas. > Update the Makefile logic to build dtc and fdtoverlay only if DTC or > FDTOVERLAY is not provided. Overriding fdtoverlay is not even supported currently. Adding support for that should be a separate patch. > Also, add an fdtoverlay wrapper to hide the actual path differences of > fdtoverlay from the make_fit.py script. That's ugly. Rob
On Fri, Aug 16, 2024 at 7:41 AM Rob Herring <robh@kernel.org> wrote: > > +Masahiro > > (use get_maintainers.pl in the future) > > On Thu, Aug 15, 2024 at 5:01 AM TIAN Yuanhao <tianyuanhao3@163.com> wrote: > > > > At present Linux always builds dtc if CONFIG_DTC is defined, even when > > DTC is provided. The built dtc is not actually used, so this is a waste > > of time. > > That's kind of a edge usecase, so I'm not sure it is worth the > complexity. Indeed. Not worth the complexity. Overriding DTC is not a general use. It is a knob so that someone can test the latest dtc from upstream. > We could also just make CONFIG_DTC visible so it can be > disabled. Or make it a path defaulting to the built-in one. Maybe > Masahiro has some ideas. Let's not do anything. This patch is NACK. > > > Update the Makefile logic to build dtc and fdtoverlay only if DTC or > > FDTOVERLAY is not provided. > > Overriding fdtoverlay is not even supported currently. Adding support > for that should be a separate patch. > > > Also, add an fdtoverlay wrapper to hide the actual path differences of > > fdtoverlay from the make_fit.py script. > > That's ugly. > > Rob -- Best Regards Masahiro Yamada
At 2024-08-16 09:52:57, "Masahiro Yamada" <masahiroy@kernel.org> wrote: >On Fri, Aug 16, 2024 at 7:41 AM Rob Herring <robh@kernel.org> wrote: >> >> +Masahiro >> >> (use get_maintainers.pl in the future) >> >> On Thu, Aug 15, 2024 at 5:01 AM TIAN Yuanhao <tianyuanhao3@163.com> wrote: >> > >> > At present Linux always builds dtc if CONFIG_DTC is defined, even when >> > DTC is provided. The built dtc is not actually used, so this is a waste >> > of time. >> >> That's kind of a edge usecase, so I'm not sure it is worth the >> complexity. > > >Indeed. Not worth the complexity. > > >Overriding DTC is not a general use. >It is a knob so that someone can test the latest >dtc from upstream. > Some meta distributions such as Yocto and Buildroot compile a common host dtc, while ATF, U-Boot, and Linux can share the same host dtc. > >> We could also just make CONFIG_DTC visible so it can be >> disabled. Or make it a path defaulting to the built-in one. Maybe >> Masahiro has some ideas. > > >Let's not do anything. > > >This patch is NACK. > > If you think there is still room for discussion on this topic, I can split this patch into two as Rob suggested. > > > >> >> > Update the Makefile logic to build dtc and fdtoverlay only if DTC or >> > FDTOVERLAY is not provided. >> >> Overriding fdtoverlay is not even supported currently. Adding support >> for that should be a separate patch. >> >> > Also, add an fdtoverlay wrapper to hide the actual path differences of >> > fdtoverlay from the make_fit.py script. >> >> That's ugly. >> >> Rob > > > >-- >Best Regards > >Masahiro Yamada -- Best Regards TIAN Yuanhao
On Fri, Aug 16, 2024 at 11:20 AM TIAN Yuanhao <tianyuanhao3@163.com> wrote: > > At 2024-08-16 09:52:57, "Masahiro Yamada" <masahiroy@kernel.org> wrote: > >On Fri, Aug 16, 2024 at 7:41 AM Rob Herring <robh@kernel.org> wrote: > >> > >> +Masahiro > >> > >> (use get_maintainers.pl in the future) > >> > >> On Thu, Aug 15, 2024 at 5:01 AM TIAN Yuanhao <tianyuanhao3@163.com> wrote: > >> > > >> > At present Linux always builds dtc if CONFIG_DTC is defined, even when > >> > DTC is provided. The built dtc is not actually used, so this is a waste > >> > of time. > >> > >> That's kind of a edge usecase, so I'm not sure it is worth the > >> complexity. > > > > > >Indeed. Not worth the complexity. > > > > > >Overriding DTC is not a general use. > >It is a knob so that someone can test the latest > >dtc from upstream. > > > Some meta distributions such as Yocto and Buildroot compile a common host dtc, while ATF, U-Boot, and Linux can share the same host dtc. If distro's dtc works for the kernel, we should remove scripts/dtc/. Instead, we need to start caring about the minimal supported dtc version. And, it will make it difficult to add a new warning flag. We can use external dtc only when it is the same version or newer than the internal one. If you use an older external dtc, some of the dtc warning options may not be supported. > >> We could also just make CONFIG_DTC visible so it can be > >> disabled. Or make it a path defaulting to the built-in one. Maybe > >> Masahiro has some ideas. > > > > > >Let's not do anything. > > > > > >This patch is NACK. > > > > > If you think there is still room for discussion on this topic, I can split this patch into two as Rob suggested. No, splitting the patch does not help anything. > > > > > > > >> > >> > Update the Makefile logic to build dtc and fdtoverlay only if DTC or > >> > FDTOVERLAY is not provided. > >> > >> Overriding fdtoverlay is not even supported currently. Adding support > >> for that should be a separate patch. > >> > >> > Also, add an fdtoverlay wrapper to hide the actual path differences of > >> > fdtoverlay from the make_fit.py script. > >> > >> That's ugly. > >> > >> Rob > > > > > > > >-- > >Best Regards > > > >Masahiro Yamada > -- > Best Regards > TIAN Yuanhao >
diff --git a/Makefile b/Makefile index 0a364e34f50b..6e56696e85a1 100644 --- a/Makefile +++ b/Makefile @@ -1419,9 +1419,20 @@ endif endif +# The dtc and fdtoverlay are automatically built unless DTC or FDTOVERLAY is +# provided. +DTC_INTREE := $(objtree)/scripts/dtc/dtc +DTC ?= $(DTC_INTREE) + +FDTOVERLAY_INTREE := $(objtree)/scripts/dtc/fdtoverlay +FDTOVERLAY ?= $(FDTOVERLAY_INTREE) + PHONY += scripts_dtc scripts_dtc: scripts_basic - $(Q)$(MAKE) $(build)=scripts/dtc + $(Q)if [ "$(DTC)" = "$(DTC_INTREE)" ] || \ + [ "$(FDTOVERLAY)" = "$(FDTOVERLAY_INTREE)" ]; then \ + $(MAKE) $(build)=scripts/dtc; \ + fi ifneq ($(filter dt_binding_check, $(MAKECMDGOALS)),) export CHECK_DTBS=y diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index fe3668dc4954..04ba30dadc8f 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -352,7 +352,6 @@ quiet_cmd_gzip = GZIP $@ # DTC # --------------------------------------------------------------------------- -DTC ?= $(objtree)/scripts/dtc/dtc DTC_FLAGS += \ -Wno-unique_unit_address @@ -415,10 +414,10 @@ DT_CHECK_CMD = $(DT_CHECKER) $(DT_CHECKER_FLAGS) -u $(srctree)/$(DT_BINDING_DIR) # recorded in the .*.cmd file. ifneq ($(CHECK_DTBS),) quiet_cmd_fdtoverlay = DTOVLCH $@ - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true + cmd_fdtoverlay = $(objtree)/scripts/fdtoverlay.sh -o $@ -i $(filter %.dtb %.dtbo, $^) ; $(DT_CHECK_CMD) $@ || true else quiet_cmd_fdtoverlay = DTOVL $@ - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(filter %.dtb %.dtbo, $^) + cmd_fdtoverlay = $(objtree)/scripts/fdtoverlay.sh -o $@ -i $(filter %.dtb %.dtbo, $^) endif $(multi-dtb-y): FORCE diff --git a/scripts/fdtoverlay.sh b/scripts/fdtoverlay.sh new file mode 100755 index 000000000000..5bd07c47c22a --- /dev/null +++ b/scripts/fdtoverlay.sh @@ -0,0 +1,7 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# +# An fdtoverlay wrapper +# scripts/make_fit.py uses the name of this script as a special marker. + +exec "${FDTOVERLAY}" "$@" diff --git a/scripts/make_fit.py b/scripts/make_fit.py index 4a1bb2f55861..37c4e1c8d5c6 100755 --- a/scripts/make_fit.py +++ b/scripts/make_fit.py @@ -238,7 +238,7 @@ def process_dtb(fname, args): with open(cmd_fname, 'r', encoding='ascii') as inf: cmd = inf.read() - if 'scripts/dtc/fdtoverlay' in cmd: + if '/scripts/fdtoverlay.sh ' in cmd: # This depends on the structure of the composite DTB command files = cmd.split() files = files[files.index('-i') + 1:]
At present Linux always builds dtc if CONFIG_DTC is defined, even when DTC is provided. The built dtc is not actually used, so this is a waste of time. Update the Makefile logic to build dtc and fdtoverlay only if DTC or FDTOVERLAY is not provided. Also, add an fdtoverlay wrapper to hide the actual path differences of fdtoverlay from the make_fit.py script. Refs: https://github.com/u-boot/u-boot/commit/93b196532254366f653b4d763f69e49ff193f06c https://github.com/torvalds/linux/commit/6b22b3d1614af1a775f2ef006009f15077592c9c Signed-off-by: TIAN Yuanhao <tianyuanhao3@163.com> Cc: Chen-Yu Tsai <wenst@chromium.org> Cc: Rob Herring (Arm) <robh@kernel.org> Cc: Simon Glass <sjg@chromium.org> Cc: Viresh Kumar <viresh.kumar@linaro.org> Cc: linux-kbuild@vger.kernel.org --- Makefile | 13 ++++++++++++- scripts/Makefile.lib | 5 ++--- scripts/fdtoverlay.sh | 7 +++++++ scripts/make_fit.py | 2 +- 4 files changed, 22 insertions(+), 5 deletions(-) create mode 100755 scripts/fdtoverlay.sh base-commit: 6b0f8db921abf0520081d779876d3a41069dab95