| Message ID | 3683a542d4141cfcf9c2524a40a9ee75b657c1c2.1611904394.git.viresh.kumar@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | dt: build overlays | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Fri, Jan 29, 2021 at 12:54:10PM +0530, Viresh Kumar wrote: > Now that fdtoverlay is part of the kernel build, start using it to test > the unitest overlays we have by applying them statically. Create two new > base files static_base_1.dts and static_base_2.dts which includes other > .dtsi files. > > Some unittest overlays deliberately contain errors that unittest checks > for. These overlays will cause fdtoverlay to fail, and are thus not > included for static builds. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/of/unittest-data/Makefile | 56 ++++++++++++++++++++++ > drivers/of/unittest-data/static_base_1.dts | 4 ++ > drivers/of/unittest-data/static_base_2.dts | 4 ++ > 3 files changed, 64 insertions(+) > create mode 100644 drivers/of/unittest-data/static_base_1.dts > create mode 100644 drivers/of/unittest-data/static_base_2.dts The first 4 patches look good to me, no need to resend those if you don't want to. > > diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile > index 009f4045c8e4..fc286224b2d1 100644 > --- a/drivers/of/unittest-data/Makefile > +++ b/drivers/of/unittest-data/Makefile > @@ -34,7 +34,63 @@ DTC_FLAGS_overlay += -@ > DTC_FLAGS_overlay_bad_phandle += -@ > DTC_FLAGS_overlay_bad_symbol += -@ > DTC_FLAGS_overlay_base += -@ > +DTC_FLAGS_static_base_1 += -@ > +DTC_FLAGS_static_base_2 += -@ > DTC_FLAGS_testcases += -@ > > # suppress warnings about intentional errors > DTC_FLAGS_testcases += -Wno-interrupts_property > + > +# Apply overlays statically with fdtoverlay. This is a build time test that > +# the overlays can be applied successfully by fdtoverlay. This does not > +# guarantee that the overlays can be applied successfully at run time by > +# unittest, but it provides a bit of build time test coverage for those > +# who do not execute unittest. > +# > +# The overlays are applied on top of static_base_1.dtb and static_base_2.dtb to > +# create static_test_1.dtb and static_test_2.dtb. If fdtoverlay detects an > +# error than the kernel build will fail. static_test_1.dtb and > +# static_test_2.dtb are not consumed by unittest. > +# > +# Some unittest overlays deliberately contain errors that unittest checks for. > +# These overlays will cause fdtoverlay to fail, and are thus not included > +# in the static test: > +# overlay_bad_add_dup_node.dtb \ > +# overlay_bad_add_dup_prop.dtb \ > +# overlay_bad_phandle.dtb \ > +# overlay_bad_symbol.dtb \ > + > +apply_static_overlay_1 := overlay_0.dtb \ > + overlay_1.dtb \ > + overlay_2.dtb \ > + overlay_3.dtb \ > + overlay_4.dtb \ > + overlay_5.dtb \ > + overlay_6.dtb \ > + overlay_7.dtb \ > + overlay_8.dtb \ > + overlay_9.dtb \ > + overlay_10.dtb \ > + overlay_11.dtb \ > + overlay_12.dtb \ > + overlay_13.dtb \ > + overlay_15.dtb \ > + overlay_gpio_01.dtb \ > + overlay_gpio_02a.dtb \ > + overlay_gpio_02b.dtb \ > + overlay_gpio_03.dtb \ > + overlay_gpio_04a.dtb \ > + overlay_gpio_04b.dtb As these are overlays, .dtbo > + > +apply_static_overlay_2 := overlay.dtb > + > +quiet_cmd_fdtoverlay = FDTOVERLAY $@ > + cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^ > + > +$(obj)/static_test_1.dtb: $(obj)/static_base_1.dtb $(addprefix $(obj)/,$(apply_static_overlay_1)) > + $(call if_changed,fdtoverlay) > + > +$(obj)/static_test_2.dtb: $(obj)/static_base_2.dtb $(addprefix $(obj)/,$(apply_static_overlay_2)) > + $(call if_changed,fdtoverlay) > + > +always-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb We can't be leaving all this for ordinary folks to copy-n-paste in every directory with overlays. It needs to be simple for users like multi-file modules: test1-dtbs := static_base_1.dtb $(apply_static_overlay_1) test2-dtbs := static_base_2.dtb $(apply_static_overlay_2) dtb-$(CONFIG_OF_OVERLAY) += test1.dtb test2.dtb I've gotten something working with the patch below. Hopefully, Masahiro has some comments on it. I couldn't get multiple '-dtbs' to work without the '.SECONDEXPANSION'. Rob 8<------------------------------------------------------------------- From 3f3f1e478d0cc512050c70eda2e9e6f577bc3107 Mon Sep 17 00:00:00 2001 From: Rob Herring <robh@kernel.org> Date: Wed, 3 Feb 2021 19:22:47 -0600 Subject: [PATCH] rework dtb overlay building Signed-off-by: Rob Herring <robh@kernel.org> --- drivers/of/unittest-data/Makefile | 54 ++++++++++++++----------------- scripts/Makefile.lib | 12 +++++++ 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile index fc286224b2d1..749d04ee6dc3 100644 --- a/drivers/of/unittest-data/Makefile +++ b/drivers/of/unittest-data/Makefile @@ -60,37 +60,31 @@ DTC_FLAGS_testcases += -Wno-interrupts_property # overlay_bad_phandle.dtb \ # overlay_bad_symbol.dtb \ -apply_static_overlay_1 := overlay_0.dtb \ - overlay_1.dtb \ - overlay_2.dtb \ - overlay_3.dtb \ - overlay_4.dtb \ - overlay_5.dtb \ - overlay_6.dtb \ - overlay_7.dtb \ - overlay_8.dtb \ - overlay_9.dtb \ - overlay_10.dtb \ - overlay_11.dtb \ - overlay_12.dtb \ - overlay_13.dtb \ - overlay_15.dtb \ - overlay_gpio_01.dtb \ - overlay_gpio_02a.dtb \ - overlay_gpio_02b.dtb \ - overlay_gpio_03.dtb \ - overlay_gpio_04a.dtb \ - overlay_gpio_04b.dtb +apply_static_overlay_1 := overlay_0.dtbo \ + overlay_1.dtbo \ + overlay_2.dtbo \ + overlay_3.dtbo \ + overlay_4.dtbo \ + overlay_5.dtbo \ + overlay_6.dtbo \ + overlay_7.dtbo \ + overlay_8.dtbo \ + overlay_9.dtbo \ + overlay_10.dtbo \ + overlay_11.dtbo \ + overlay_12.dtbo \ + overlay_13.dtbo \ + overlay_15.dtbo \ + overlay_gpio_01.dtbo \ + overlay_gpio_02a.dtbo \ + overlay_gpio_02b.dtbo \ + overlay_gpio_03.dtbo \ + overlay_gpio_04a.dtbo \ + overlay_gpio_04b.dtbo apply_static_overlay_2 := overlay.dtb -quiet_cmd_fdtoverlay = FDTOVERLAY $@ - cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^ +test1-dtbs := static_base_1.dtb $(apply_static_overlay_1) +test2-dtbs := static_base_2.dtb $(apply_static_overlay_2) -$(obj)/static_test_1.dtb: $(obj)/static_base_1.dtb $(addprefix $(obj)/,$(apply_static_overlay_1)) - $(call if_changed,fdtoverlay) - -$(obj)/static_test_2.dtb: $(obj)/static_base_2.dtb $(addprefix $(obj)/,$(apply_static_overlay_2)) - $(call if_changed,fdtoverlay) - -always-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb +dtb-$(CONFIG_OF_OVERLAY) += test1.dtb test2.dtb diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index b00855b247e0..886d2e6c58f0 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -66,6 +66,9 @@ multi-used := $(multi-used-y) $(multi-used-m) real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m))) real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m))) +real-dtb-y := $(foreach m,$(dtb-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),)) +dtb-y += $(real-dtb-y) + always-y += $(always-m) # hostprogs-always-y += foo @@ -332,6 +335,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) + +quiet_cmd_fdtoverlay = DTOVL $@ + cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs) + +.SECONDEXPANSION: + +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE + $(call if_changed,fdtoverlay) + DT_CHECKER ?= dt-validate DT_BINDING_DIR := Documentation/devicetree/bindings # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile
On 03-02-21, 19:54, Rob Herring wrote: > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index b00855b247e0..886d2e6c58f0 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -66,6 +66,9 @@ multi-used := $(multi-used-y) $(multi-used-m) > real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m))) > real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m))) > > +real-dtb-y := $(foreach m,$(dtb-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),)) > +dtb-y += $(real-dtb-y) > + > always-y += $(always-m) > > # hostprogs-always-y += foo > @@ -332,6 +335,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) > > + > +quiet_cmd_fdtoverlay = DTOVL $@ > + cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs) > + > +.SECONDEXPANSION: > + > +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE > + $(call if_changed,fdtoverlay) > + > DT_CHECKER ?= dt-validate > DT_BINDING_DIR := Documentation/devicetree/bindings > # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile Thanks, this simplifies it greatly for platform guys.
On Mon, Feb 8, 2021 at 5:18 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 03-02-21, 19:54, Rob Herring wrote: > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index b00855b247e0..886d2e6c58f0 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -66,6 +66,9 @@ multi-used := $(multi-used-y) $(multi-used-m) > > real-obj-y := $(foreach m, $(obj-y), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)),$(m))) > > real-obj-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)) $($(m:.o=-))),$($(m:.o=-objs)) $($(m:.o=-y)) $($(m:.o=-m)),$(m))) > > > > +real-dtb-y := $(foreach m,$(dtb-y), $(if $(strip $($(m:.dtb=-dtbs))),$($(m:.dtb=-dtbs)),)) > > Sorry for my lack of knowledge, but what does (m:.dtb=-dtbs) do > exactly ? In string 'm' replace '.dtb' with '-dtbs'. Then we get the value of that variable. > > > +dtb-y += $(real-dtb-y) > > + > > always-y += $(always-m) > > > > # hostprogs-always-y += foo > > @@ -332,6 +335,15 @@ $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > $(obj)/%.dtbo: $(src)/%.dts $(DTC) FORCE > > $(call if_changed_dep,dtc) > > > > + > > +quiet_cmd_fdtoverlay = DTOVL $@ > > + cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $(real-prereqs) > > + > > +.SECONDEXPANSION: > > + > > +$(obj)/%.dtb: $$(addprefix $$(obj)/,$$(%-dtbs)) FORCE > > + $(call if_changed,fdtoverlay) > > + > > DT_CHECKER ?= dt-validate > > DT_BINDING_DIR := Documentation/devicetree/bindings > > # DT_TMP_SCHEMA may be overridden from Documentation/devicetree/bindings/Makefile > > I tried to test a dtbo from arch/ code like this: > > diff --git a/arch/arm64/boot/dts/hisilicon/Makefile b/arch/arm64/boot/dts/hisilicon/Makefile > index f4d68caeba83..0ee9d7dc8e07 100644 > --- a/arch/arm64/boot/dts/hisilicon/Makefile > +++ b/arch/arm64/boot/dts/hisilicon/Makefile > @@ -6,3 +6,8 @@ dtb-$(CONFIG_ARCH_HISI) += hi6220-hikey.dtb > dtb-$(CONFIG_ARCH_HISI) += hip05-d02.dtb > dtb-$(CONFIG_ARCH_HISI) += hip06-d03.dtb > dtb-$(CONFIG_ARCH_HISI) += hip07-d05.dtb > + > +DTC_FLAGS_hi3660-hikey960 += -@ > + > +test1-dtbs := hi3660-hikey960.dtb hi3660-hikey960-overlay.dtbo > +dtb-y += test1.dtb > diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960-overlay.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960-overlay.dts > new file mode 100644 > index 000000000000..1235a911caae > --- /dev/null > +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960-overlay.dts > @@ -0,0 +1,16 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/dts-v1/; > +/plugin/; > + > +&dwmmc2 { > + #address-cells = <0x1>; > + #size-cells = <0x0>; > + > + wlcore2: wlcore@5 { > + compatible = "ti,wl1837"; > + reg = <2>; > + interrupt-parent = <&gpio22>; > + interrupts = <3 1>; > + test = <1>; > + }; > +}; > > Even after your patch there are some issues I am facing: > > 1. dtbs_check doesn't test hi3660-hikey960-overlay.dts. I also tried > to add it to dtbo-y +=, but that didn't do anything as well. > > I expected this to work as we have this in scripts/Makefile.lib: > > ifneq ($(CHECK_DTBS),) > extra-y += $(patsubst %.dtb,%.dt.yaml, $(dtb-y)) > +extra-y += $(patsubst %.dtbo,%.dt.yaml, $(dtb-y)) > extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtb,%.dt.yaml, $(dtb-)) > +extra-$(CONFIG_OF_ALL_DTBS) += $(patsubst %.dtbo,%.dt.yaml, $(dtb-)) > endif I'll have to try that out. I think that should work. > 2. This fails dtbs_check as it tries to run it for the source file of > test1.dtb > > $ make ARCH=arm64 O=../barm64/ -j8 CROSS_COMPILE=aarch64-linux-gnu- dtbs_check > make[1]: Entering directory '/mnt/ssd/all/work/repos/devel/barm64' > make[3]: *** No rule to make target 'arch/arm64/boot/dts/hisilicon/test1.dt.yaml', needed by '__build'. Stop. > /mnt/ssd/all/work/repos/devel/linux/scripts/Makefile.build:496: recipe for target 'arch/arm64/boot/dts/hisilicon' failed > make[2]: *** [arch/arm64/boot/dts/hisilicon] Error 2 > make[2]: *** Waiting for unfinished jobs.... > /mnt/ssd/all/work/repos/devel/linux/Makefile:1345: recipe for target 'dtbs' failed > make[1]: *** [dtbs] Error 2 > make[1]: Leaving directory '/mnt/ssd/all/work/repos/devel/barm64' > Makefile:185: recipe for target '__sub-make' failed > make: *** [__sub-make] Error 2 > > I am not sure how to fix this. Even if we fixed the make rules, it's not going to work with validation. There's some information from source files that we maintain in yaml output, but is lost in dtb output. For example, the sizes of /bits/ syntax are maintained. For now, I think we'll want to just validate base and overlays separately. We may need to turn off checks in overlays for required properties as they may be incomplete. We already do that on disabled nodes. Rob
On 09-02-21, 15:40, Viresh Kumar wrote: > And after decent amount of effort understanding how to do this, I > finally did it in a not so efficient way, I am sure you can help > improving it :) Ping! Also, where do we send patches for dt-schema ? Which list ? > Author: Viresh Kumar <viresh.kumar@linaro.org> > Date: Tue Feb 9 12:19:50 2021 +0530 > > dt-validate: Skip "required property" checks for overlays > > The overlays may not carry the required properties and would depend on > the base dtb to carry those, there is no point raising those errors > here. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > tools/dt-validate | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/tools/dt-validate b/tools/dt-validate > index 410b0538ef47..c6117504f1d1 100755 > --- a/tools/dt-validate > +++ b/tools/dt-validate > @@ -80,6 +80,23 @@ show_unmatched = False > (filename, line, col, fullname, node['compatible']), file=sys.stderr) > continue > > + if nodename == '/': > + is_fragment = False > + for name in node.items(): > + if name[0] == 'fragment@0': > + is_fragment = True > + break; > + > + if is_fragment == True: > + if 'required property' in error.message: > + continue > + elif error.context: > + for e in error.context: > + if not 'required property' in e.message: > + break > + else: > + continue > + > print(dtschema.format_error(filename, error, nodename=nodename, verbose=verbose) + > '\n\tFrom schema: ' + schema['$filename'], > file=sys.stderr)
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile index 009f4045c8e4..fc286224b2d1 100644 --- a/drivers/of/unittest-data/Makefile +++ b/drivers/of/unittest-data/Makefile @@ -34,7 +34,63 @@ DTC_FLAGS_overlay += -@ DTC_FLAGS_overlay_bad_phandle += -@ DTC_FLAGS_overlay_bad_symbol += -@ DTC_FLAGS_overlay_base += -@ +DTC_FLAGS_static_base_1 += -@ +DTC_FLAGS_static_base_2 += -@ DTC_FLAGS_testcases += -@ # suppress warnings about intentional errors DTC_FLAGS_testcases += -Wno-interrupts_property + +# Apply overlays statically with fdtoverlay. This is a build time test that +# the overlays can be applied successfully by fdtoverlay. This does not +# guarantee that the overlays can be applied successfully at run time by +# unittest, but it provides a bit of build time test coverage for those +# who do not execute unittest. +# +# The overlays are applied on top of static_base_1.dtb and static_base_2.dtb to +# create static_test_1.dtb and static_test_2.dtb. If fdtoverlay detects an +# error than the kernel build will fail. static_test_1.dtb and +# static_test_2.dtb are not consumed by unittest. +# +# Some unittest overlays deliberately contain errors that unittest checks for. +# These overlays will cause fdtoverlay to fail, and are thus not included +# in the static test: +# overlay_bad_add_dup_node.dtb \ +# overlay_bad_add_dup_prop.dtb \ +# overlay_bad_phandle.dtb \ +# overlay_bad_symbol.dtb \ + +apply_static_overlay_1 := overlay_0.dtb \ + overlay_1.dtb \ + overlay_2.dtb \ + overlay_3.dtb \ + overlay_4.dtb \ + overlay_5.dtb \ + overlay_6.dtb \ + overlay_7.dtb \ + overlay_8.dtb \ + overlay_9.dtb \ + overlay_10.dtb \ + overlay_11.dtb \ + overlay_12.dtb \ + overlay_13.dtb \ + overlay_15.dtb \ + overlay_gpio_01.dtb \ + overlay_gpio_02a.dtb \ + overlay_gpio_02b.dtb \ + overlay_gpio_03.dtb \ + overlay_gpio_04a.dtb \ + overlay_gpio_04b.dtb + +apply_static_overlay_2 := overlay.dtb + +quiet_cmd_fdtoverlay = FDTOVERLAY $@ + cmd_fdtoverlay = $(objtree)/scripts/dtc/fdtoverlay -o $@ -i $^ + +$(obj)/static_test_1.dtb: $(obj)/static_base_1.dtb $(addprefix $(obj)/,$(apply_static_overlay_1)) + $(call if_changed,fdtoverlay) + +$(obj)/static_test_2.dtb: $(obj)/static_base_2.dtb $(addprefix $(obj)/,$(apply_static_overlay_2)) + $(call if_changed,fdtoverlay) + +always-$(CONFIG_OF_OVERLAY) += static_test_1.dtb static_test_2.dtb diff --git a/drivers/of/unittest-data/static_base_1.dts b/drivers/of/unittest-data/static_base_1.dts new file mode 100644 index 000000000000..10556cb3f01f --- /dev/null +++ b/drivers/of/unittest-data/static_base_1.dts @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; + +#include "testcases_common.dtsi" diff --git a/drivers/of/unittest-data/static_base_2.dts b/drivers/of/unittest-data/static_base_2.dts new file mode 100644 index 000000000000..b0ea9504d6f3 --- /dev/null +++ b/drivers/of/unittest-data/static_base_2.dts @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; + +#include "overlay_common.dtsi"
Now that fdtoverlay is part of the kernel build, start using it to test the unitest overlays we have by applying them statically. Create two new base files static_base_1.dts and static_base_2.dts which includes other .dtsi files. Some unittest overlays deliberately contain errors that unittest checks for. These overlays will cause fdtoverlay to fail, and are thus not included for static builds. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/of/unittest-data/Makefile | 56 ++++++++++++++++++++++ drivers/of/unittest-data/static_base_1.dts | 4 ++ drivers/of/unittest-data/static_base_2.dts | 4 ++ 3 files changed, 64 insertions(+) create mode 100644 drivers/of/unittest-data/static_base_1.dts create mode 100644 drivers/of/unittest-data/static_base_2.dts